Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250712144456.GB1827@brightrain.aerifal.cx>
Date: Sat, 12 Jul 2025 10:44:56 -0400
From: Rich Felker <dalias@...c.org>
To: Luca Kellermann <mailto.luca.kellermann@...il.com>
Cc: Markus Wichmann <nullplan@....net>, musl@...ts.openwall.com
Subject: Re: [PATCH 3/4] scandir: fix leaks caused by cancellation

On Sat, Jul 12, 2025 at 07:48:53AM +0200, Luca Kellermann wrote:
> On Thu, Jul 10, 2025 at 11:57:11PM -0400, Rich Felker wrote:
> > On Thu, Jul 10, 2025 at 09:19:24PM +0200, Markus Wichmann wrote:
> > > Am Thu, Jul 10, 2025 at 08:51:30PM +0200 schrieb Luca Kellermann:
> > > > opendir(), sel(), closedir(), or cmp() might act upon a cancellation
> > > > request. because scandir() did not install a cancellation cleanup
> > > > handler, this could lead to memory and file descriptor leaks.
> > >
> > > scandir() is on the "may be cancel-point" list in POSIX-2024, so it
> > > would also be acceptable to just disable cancellation for the duration
> > > of the function (since it's not on the "shall be cancel-point" list).
> >
> > Yes, generally we do not support cancellation for the "may be a
> > cancellation point" functions except in some trivial cases. Here it's
> > complicated by the fact that we're calling back to application code,
> > and scandir is underspecified in that it doesn't actually specify what
> > happens if the sel or compar function does not return in the normative
> > text. However, the APPLICATION USAGE section mentions this possibility
> > and advises applications not to do it, but doesn't seem to forbid it.
> 
> So should I update the patch to disable cancellation? It would
> definitely simplify the code. If so, should it be disabled before or
> after the call to opendir()? There is no need for a cancellation
> cleanup handler if opendir() acts upon a cancellation request.

If the function is not a cancellation point, it shouldn't act on
cancellation at any point during it, so I think it should be blocked
across the open and close.

I'm not sure if it's strictly conforming for the blocking of
cancellation to be exposed to the callbacks to application-provided
functions though. While it's arguably *nicer* to leave it blocked,
POSIX grants us the choice of whether scandir should be a cancellation
point, not of whether it calls the callbacks with altered thread
state. So I think we probably need to just block around the opendir
and closedir calls, but otherwise leave it unblocked.

Rich

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.