Follow @Openwall on Twitter for new release announcements and other news
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240125155653.GI4163@brightrain.aerifal.cx>
Date: Thu, 25 Jan 2024 10:56:54 -0500
From: Rich Felker <dalias@...c.org>
To: Ismael Luceno <ismael@...ev.co.uk>
Cc: musl@...ts.openwall.com
Subject: Re: [PATCH] fix avoidable segfault in catclose

On Thu, Jan 25, 2024 at 04:28:47PM +0100, Ismael Luceno wrote:
> <...>
> > Generally in musl, we prefer to trap on UB rather than allowing
> > forward progress, especially when the natural default action without
> > special casing it is to trap.
> 
> POSIX says: "The catclose() function may fail if: [EBADF] The catalog
> descriptor is not valid. ..."
> 
> Implying a known invalid descriptor like -1, or an invalidated
> descriptor should be handled.
> 
> Glibc manual says:
> "...  Errors can occur if the catalog descriptor catalog_desc is not
> valid in which case errno is set to EBADF."
> 
> The linux manpage also says that once closed, the descriptor gets
> invalidated, which isn't what we're doing here.

This is not a "should be handled". "May fail" is always optional and
is a redundant allowance we explicitly do not use when the behavior is
UB. (It's redundant because anything can happen in the event of UB;
you don't need an allowance for that.)

Here, the API is designed around the possibility that cat_t may be
implemented as some sort of descriptor where it's possible/tractable
to identify valid vs invalid values, and presumably that's where the
idea of EBADF came from. But even EBADF in general is a bad idea, and
not something we'd want to implement except where required. EBADF
basically always indicates a dangerous UAF or DF type bug, where
trapping is the preferred behavior.

The philosophy behind this is explained in the *glibc* wiki, where the
relevant text (10.4.1 NULL pointers) was copied from an answer of mine
on Stack Overflow long ago:

https://sourceware.org/glibc/wiki/Style_and_Conventions#Bugs_in_the_user_program

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.