From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.1 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 Received: from second.openwall.net (second.openwall.net [193.110.157.125]) by inbox.vuxu.org (Postfix) with SMTP id 38EF624076 for ; Thu, 25 Jan 2024 16:56:51 +0100 (CET) Received: (qmail 18002 invoked by uid 550); 25 Jan 2024 15:54:39 -0000 Mailing-List: contact musl-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Reply-To: musl@lists.openwall.com Received: (qmail 17970 invoked from network); 25 Jan 2024 15:54:38 -0000 Date: Thu, 25 Jan 2024 10:56:54 -0500 From: Rich Felker To: Ismael Luceno Cc: musl@lists.openwall.com Message-ID: <20240125155653.GI4163@brightrain.aerifal.cx> References: <20240125070950.28673-1-ismael@iodev.co.uk> <20240125140503.GG4163@brightrain.aerifal.cx> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Subject: Re: [musl] [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