[Bug classpath/33751] Classpath fails to build on Solaris 2.9 (SPARC)

[robilad at kaffe dot org]

------- Comment #3 from robilad at kaffe dot org  2008-01-13 19:14 -------
>From looking at the readdir spec at
http://www.opengroup.org/onlinepubs/009695399/functions/readdir.html I'd say
that the current code is broken in several ways:

* does not set errno to 0 prior to calling readdir. violates 'Applications
wishing to check for error situations should set errno to 0 before calling
readdir(). If errno is set to non-zero on return, an error occurred.'

* mistreats end of directory by setting errno. violates 'Upon successful
return, the pointer returned at *result shall have the same value as the
argument entry. Upon reaching the end of the directory stream, this pointer
shall have the value NULL.'

* allows buffer overflows via a directory path longer than FILENAME_MAX, as on
systems without a path length limit such paths are possible. I'd suggest that
the d_name is strdup'ed and that value returned, rather than copying it around
into an array of a fixed size, due to this footnote in the ISO C 9989:1999
draft at  http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf, and due to
the POSIXy NAME_MAX being -1 for implementations not having a limit on path
length:

'255) If the implementation imposes no practical limit on the length of file
name strings, the value of FILENAME_MAX should instead be the recommended size
of an array intended to hold a file name string. Of course, file name string
contents are subject to other system-specific constraints; therefore all
possible strings of length FILENAME_MAX cannot be expected to be opened
successfully.'

* doesn't use return value or readdir_r to determine if readdir_r succeeded,
uses a check if dBuf is NULL instead. While that's a valid strategy for
readdir, it's invalid for readdir_r, since dBuf can be set to NULL upon
reaching the end of the directory stream after a successful readdir_r call.

* Use of readdir_r seems unwarranted to me, as readdir is threadsafe as long as
it used from a single thread, which can be ensured by making the only user of
cpio_readDir, the method list in the VMFile interface synchronized. That would
have the added benefit of fixing any readdir_r related portability issues, and
avoiding potential race issues outlined in
http://womble.decadentplace.org.uk/readdir_r-advisory.html , which specifically
lists gcj as vulnerable (though it uses a somewhat different implementation).

In closing, I'd suggest rewriting the list method's native code to simply use
glib's GDir interface to open a directory, read the entries and close it, and
to make the method synchronized to make it thread safe.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=33751