[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAJgzZorQ0=LTVZgcRwjgmN-+Ax4UtSLMeDYPw2vRHCHJtLjCGA@mail.gmail.com>
Date: Tue, 23 Feb 2021 09:52:50 -0800
From: enh <enh@...gle.com>
To: libc-coord@...ts.openwall.com
Subject: Re: Lifetime of object returned by readdir
On Tue, Feb 23, 2021 at 9:16 AM Rich Felker <dalias@...c.org> wrote:
> As a consequence of Austin Group issue 696 (which still does not seem
> to be resolved properly), readdir_r is unusable and readdir should be
> required to be thread-safe. Moreover, it's already disallowed for the
> implementation to use a common static buffer for the result of
> readdir since operations on different streams cannot overwrite each
> other's results:
>
> The returned pointer, and pointers within the structure, might be
> invalidated or the structure or the storage areas might be
> overwritten by a subsequent call to readdir() on the same directory
> stream. They shall not be affected by a call to readdir() on a
> different directory stream. The returned pointer, and pointers
> within the structure, might also be invalidated if the calling
> thread is terminated.
>
> This leaves "part of the storage associated with the DIR stream
> object" as the only reasonable place for the dirent to live, and of
> course that's the natural (for zerocopy) place for it to live anyway,
> and where it does live in existing implementations (at least glibc and
> musl).
>
(and in Android's bionic libc too. we also, like glibc, mark readdir_r()
__attribute__((__deprecated__)) and suggest readdir() instead.)
> However, as part of resolving an application UAF bug where the dirent
> was used after closedir, I realized that the specification fails to
> mention closedir of the directory stream as a condition that can end
> the lifetime of the dirent object. This seems like an omission, and
> like it does not admit any implementation without severe memory leaks
> -- the last dirent returned for each stream would have to be preserved
> indefinitely unless the thread that called readdir exited.
>
> I'd like to push to have this fixed (adding closedir as a condition
> that ends the lifetime) as part of making readdir thread-safe, but
> before opening a new Austin Group issue or following up on the
> existing one there I'd like to make sure we're on the same page.
>
+1. on Android we're pushing pretty hard on "UAF checking all the time,
even in the field" with the switch to the scudo allocator and things like
probabilistic gwp-asan and so on, so we're probably helping find any such
issues already. (plus we run hwasan builds internally.)
fwiw, it looks like we've only seen one issue that was a UA(closedir),
though it's always hard to know whether the tools are just catching these
bugs "privately" before they're checked in vs people just aren't making
this particular mistake very often.
(annoyingly given that the bug was in a C++ file and i try to bang the "use
std::unique_ptr for your FILE* and DIR*s too" drum, the bug would have been
avoided by letting std::unique_ptr call closedir :-( )
> Rich
>
Content of type "text/html" skipped
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
