Re: [musl] Bug in src/signal/block.c

[Rich Felker <dalias@libc.org>]

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 <dalias@libc.org> 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
> >