This is the mail archive of the
newlib@sourceware.org
mailing list for the newlib project.
Re: seekdir dereferences null
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.