Bug 312989

Summary:	[PATCH] ioctl handling needs to do POST handling on generic ioctls and needs to handle BPF ioctls
Product:	[Developer tools] valgrind	Reporter:	Guy Harris <gharris>
Component:	general	Assignee:	Rhys Kidd <rhyskidd>
Status:	RESOLVED FIXED
Severity:	normal	CC:	rhyskidd
Priority:	NOR
Version First Reported In:	3.9.0.SVN
Target Milestone:	---
Platform:	Compiled Sources
OS:	macOS
Latest Commit:		Version Fixed In:
Sentry Crash Report:
Attachments:	Patch to fix the problems in question Slightly improved patch to fix the problems in question An improved-a-little-more patch

Description Guy Harris 2013-01-10 03:12:40 UTC

The ioctl handling for Darwin wasn't calling the POST_unknown_ioctl routine for unknown ioctls in the POST routine, so a bunch of ioctls that return values weren't being handled, and a number of BPF ioctls needed special handling so as not to report bogus "uninitialized data" for locations the kernel doesn't look at (because they're not used at all, because they're written to but not read, or because they're padding) and to handle ioctls where there are pointers passed into the structure (so that we check validity of data read through those pointers or note that data written through those pointers is initialized).

Reproducible: Always

Steps to Reproduce:
1. Run tcpdump under Valgrind
Actual Results:  
A bunch of warnings that don't reflect actual problems.

Expected Results:  
No such warnings.

Comment 1 Guy Harris 2013-01-10 03:14:27 UTC

Created attachment 76356 [details]
Patch to fix the problems in question

This should work regardless of whether bug 312980 is fixed before this is applied or not, and if this is fixed first, the patches for bug 312980 should apply without a problem.

Comment 2 Guy Harris 2013-04-10 23:38:39 UTC

Created attachment 78785 [details]
Slightly improved patch to fix the problems in question

This patch is made from top-of-trunk at the time the patch was made, and makes the error message printed if the argument to the BIOCSETF ioctl points to an initialized structure, but the pointer in that structure points to uninitialized (or otherwise invalid) memory, a bit more detailed and hopefully a bit clearer.

Comment 3 Guy Harris 2013-04-10 23:57:08 UTC

Created attachment 78786 [details]
An improved-a-little-more patch

Improved another error message, checked that the pointer member of the argument to BIOCGDLTLIST is initialized, and made the check that the length member of that argument is initialized happen only if the pointer member is non-NULL (matching what happens in the OS; if the pointer member is NULL, that means the caller wants to know how big a buffer to allocate, and doesn't know the buffer length, so it might not have bothered to set it).

Comment 4 Rhys Kidd 2015-07-13 02:00:33 UTC

Thanks for the research and patch Guy. Is it still your most up to date version?

I'll take a look at this patch and whether it can be merged.

Comment 5 Rhys Kidd 2015-07-25 07:58:28 UTC

Committed in r15451.
Thank you for the patch.