From mboxrd@z Thu Jan 1 00:00:00 1970 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on inbox.vuxu.org X-Spam-Level: X-Spam-Status: No, score=-3.3 required=5.0 tests=MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL autolearn=ham autolearn_force=no version=3.4.4 Received: (qmail 22424 invoked from network); 28 Jul 2021 22:05:05 -0000 Received: from mother.openwall.net (195.42.179.200) by inbox.vuxu.org with ESMTPUTF8; 28 Jul 2021 22:05:05 -0000 Received: (qmail 21639 invoked by uid 550); 28 Jul 2021 22:05:03 -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 21618 invoked from network); 28 Jul 2021 22:05:02 -0000 Date: Wed, 28 Jul 2021 18:04:50 -0400 From: Rich Felker To: Jasper Hugunin Cc: musl@lists.openwall.com Message-ID: <20210728220450.GT13220@brightrain.aerifal.cx> References: <20210728155340.GQ13220@brightrain.aerifal.cx> <20210728193628.GS13220@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] Bug in src/signal/block.c On Wed, Jul 28, 2021 at 12:52:23PM -0700, Jasper Hugunin wrote: > It looks ok to me, I think that will work for all the architectures > currently around. > > I do worry about what would happen on an architecture setting `_NSIG` to > something other than 65 or 128, say 256 or 230 or something like that, but > that is a larger scale concern for the whole musl codebase. > > (Note: I am not an expert on signal handling, I was just poking around the > code when I spotted this inconsistency.) Thanks for your review. It's a historical mistake that MIPS had 128 (later changed to 127) signals rather than the 64 everywhere else. It would be very unexpected for any new arch to deviate like this, since nowadays everyone realizes trying to mimic the old proprietary unices that once existed 'natively' on each arch was a mistake. If it does happen, generalizing to support it would be part of the work of adding that arch. Rich > On Wed, Jul 28, 2021 at 12:36 PM Rich Felker wrote: > > > On Wed, Jul 28, 2021 at 11:53:41AM -0400, Rich Felker wrote: > > > On Wed, Jul 28, 2021 at 08:00:00AM -0700, Jasper Hugunin wrote: > > > > Hello, > > > > > > > > In musl, as far as I can tell, `_NSIG` is always defined as either 65, > > or > > > > 128 (for all three MIPS architectures) at the bottom of > > > > `${arch}/bits/signal.h`. Meanwhile, in `src/signal/block.c`, there is a > > > > test `#if ULONG_MAX == 0xffffffff && _NSIG == 129`, which will never > > > > succeed since _NSIG will be 128 instead of 129. This seems likely to be > > > > left over from Commit: fix _NSIG and SIGRTMAX on mips > > > > < > > https://git.musl-libc.org/cgit/musl/commit/arch?id=7c440977db9444d7e6b1c3dcb1fdf4ee49ca4158 > > > > > > > .. > > > > > > > > I have not demonstrated the bug, I found it by inspection of the > > source. My > > > > guess is that this bug causes __block_all_sigs to fail to block high > > real > > > > time signals on MIPS. At best, however, this test seems to be dead > > code. > > > > > > > > (I am not subscribed to the mailing list; please cc me directly on any > > > > responses I need to see.) > > > > My apologies if I have misunderstood the situation. > > > > > > Thanks! This is a real bug that will prevent signal blocking from > > > working correctly on mips, resulting in application code being able to > > > run in contexts where it is unsafe for that to happen if the > > > application installs signal handlers on high signal numbers. > > > > Does the attached patch look ok? > > > > Rich > >