seekdir dereferences null

Jeff Johnston jjohnstn@redhat.com
Fri Dec 5 13:07:00 GMT 2008


Eric Blake wrote:
> Howland Craig D (Craig <howland <at> LGSInnovations.com> writes:
>
>   
>>> Another bug picked up by the RTEMS test suite.
>>>
>>> seekdir(NULL, 0) core dumps.
>>>       
>
> Where do the standards mandate that this is required to be supported?  Aren't 
> you just adding bloat?  This is no different than calling strlen(NULL), and 
> expecting a sensible result - the bug is in the RTEMS test suite for not 
> passing a valid DIR* in the first place, and not in newlib for crashing on 
> invalid input - garbage in, garbage out.  I also think idea of returning EBADF 
> is wrong here - a NULL DIR* should trigger EFAULT (bad pointer), not EBADF 
> (good pointer, but to an unopened or otherwise bad DIR stream).
>
> In other words, since a valid program should never be passing NULL in the first 
> place, this proposed patch is just bloating things for the single case of NULL, 
> without helping for the more generic QoI issue of detecting _all_ invalid 
> pointers.  The best solution, if you are going for QoI, is the much bigger task 
> of figuring out how to write a SIGBUS/SIGSEGV handler that decides whether the 
> fault occurred inside a newlib function, in which case execution is resumed 
> with that library function failing with EFAULT (cygwin has managed to do 
> something like this), at which point filtering out for just NULL actually slows 
> things down (it is more efficient to let a signal/exception handler deal with 
> the corner case of bad code than to add a branch to the common case of good 
> code).
>
>   
I have been thinking along these same lines.  It is not specified in any 
of the documentation I found.  So I looked at glibc.  The non-stubbed 
versions of these functions do not make the check.  The stubbed version 
in dirent returns ENOSYS, makes the check and sets errno to EINVAL if NULL.

The one reason to add such a bonus check like this is to allow nested 
calls.  For example, opendir(), the supplier of DIRP, can return NULL.  
If code does a nested call whereby opendir() is used to supply the DIRP 
for seekdir(), then checking for NULL is providing convenience.  
However, since the norm appears to be no checking, code out there won't 
be doing this and Eric is correct that this is then just bloat.

If anybody has evidence to the contrary, feel free to comment.

-- Jeff J.



More information about the Newlib mailing list