Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAFGcq_59MC9yvhWTkwz4U9hMRvn-X9PwTcMQa3ydoo1Eubp10g@mail.gmail.com>
Date: Sat, 12 Jul 2025 07:48:53 +0200
From: Luca Kellermann <mailto.luca.kellermann@...il.com>
To: Rich Felker <dalias@...c.org>
Cc: Markus Wichmann <nullplan@....net>, musl@...ts.openwall.com
Subject: Re: [PATCH 3/4] scandir: fix leaks caused by cancellation

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.

Would I use pthread_setcancelstate() or __pthread_setcancelstate() to
disable cancellation? Some functions in musl use
pthread_setcancelstate() (e.g. sem_open()), others use
__pthread_setcancelstate() (e.g. pthread_join()). I think I don't
really understand hidden and weak_alias and how/why musl uses them.

Luca

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.