Re: [patch] libstdc++/67747 Allocate space for dirent::d_name

[Florian Weimer <fweimer at redhat dot com>]

On 10/02/2015 02:34 PM, Jonathan Wakely wrote:
> On 02/10/15 14:16 +0200, Florian Weimer wrote:
>> On 09/29/2015 01:37 PM, Jonathan Wakely wrote:
>>> POSIX says that dirent::d_name has an unspecified length, so calls to
>>> readdir_r must pass a buffer with enough trailing space for
>>> {NAME_MAX}+1 characters. I wasn't doing that, which works OK on
>>> GNU/Linux and BSD where d_name is a large array, but fails on Solaris
>>> 32-bit.
>>>
>>> This uses pathconf to get NAME_MAX and allocates a buffer.
>>
>> This still has a buffer overflow on certain file systems.
>>
>> You must not use readdir_r, it is deprecated and always insecure.  We
>> should probably mark it as such in the glibc headers.
> 
> OK, I'll just use readdir() then. The directory stream is private to
> the library type, so the only way to call readdir() concurrently on a
> single directory stream is to increment iterators concurrently, which
> is undefined anyway.

Right, that's the only case where readdir_r could be theoretically
useful.  But it's not a global structure, the callers have to coordinate
anyway, and so you could well use an external lock.

> So that will work as long as readdir() doesn't use a global static
> buffer shared between streams, i.e. it meets the POSIX requirement
> that "They shall not be affected by a call to readdir() on a different
> directory stream." I don't know if mingw meets that, but there is lots
> of work needed to make this stuff work in mingw.

If mingw has this flaw, it is worth fixing on its own, and mingw is
sufficiently alive that sticking workarounds into libstdc++ for its bugs
doesn't make sense (IMHO).

>> Have we already released code which uses readdir_r?
> 
> No, it's only on trunk and the gcc-5-branch, not in any release.

Good, no need to treat this as a security vulnerability. :)