* [musl] Bug in src/signal/block.c @ 2021-07-28 15:00 Jasper Hugunin 2021-07-28 15:53 ` Rich Felker 0 siblings, 1 reply; 8+ messages in thread From: Jasper Hugunin @ 2021-07-28 15:00 UTC (permalink / raw) To: musl [-- Attachment #1: Type: text/plain, Size: 904 bytes --] 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. Sincerely, - Jasper Hugunin [-- Attachment #2: Type: text/html, Size: 1063 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [musl] Bug in src/signal/block.c 2021-07-28 15:00 [musl] Bug in src/signal/block.c Jasper Hugunin @ 2021-07-28 15:53 ` Rich Felker 2021-07-28 17:11 ` Laurent Bercot 2021-07-28 19:36 ` Rich Felker 0 siblings, 2 replies; 8+ messages in thread From: Rich Felker @ 2021-07-28 15:53 UTC (permalink / raw) To: Jasper Hugunin; +Cc: musl 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. Rich ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [musl] Bug in src/signal/block.c 2021-07-28 15:53 ` Rich Felker @ 2021-07-28 17:11 ` Laurent Bercot 2021-07-28 18:43 ` Michael Forney 2021-07-28 19:30 ` Rich Felker 2021-07-28 19:36 ` Rich Felker 1 sibling, 2 replies; 8+ messages in thread From: Laurent Bercot @ 2021-07-28 17:11 UTC (permalink / raw) To: musl >> succeed since _NSIG will be 128 instead of 129. I happen to be in the process of updating my programming library performing workarounds for badly-specified parts of POSIX and related functions. NSIG is one of those parts. It is not specified by POSIX, but it is useful to have a walkable (as in, not 8*sizeof(sigset_t)) upper bound for the number of signals on a system. But NSIG is badly specified even across the systems where it exists. On glibc, it is 1 + the highest signal number. On FreeBSD and OpenBSD at least, it is the highest signal number. musl appears to align on glibc; I suppose the value for MIPS will be updated to 129, for consistency. Can I assume that if NSIG is defined, it is strictly greater than the highest signal number, except on the BSDs which are an unfortunate exception, as they too often are? -- Laurent ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [musl] Bug in src/signal/block.c 2021-07-28 17:11 ` Laurent Bercot @ 2021-07-28 18:43 ` Michael Forney 2021-07-28 19:30 ` Rich Felker 1 sibling, 0 replies; 8+ messages in thread From: Michael Forney @ 2021-07-28 18:43 UTC (permalink / raw) To: musl "Laurent Bercot" <ska-dietlibc@skarnet.org> wrote: > I happen to be in the process of updating my programming library > performing workarounds for badly-specified parts of POSIX and related > functions. > > NSIG is one of those parts. It is not specified by POSIX, but it is > useful to have a walkable (as in, not 8*sizeof(sigset_t)) upper bound > for the number of signals on a system. > > But NSIG is badly specified even across the systems where it exists. > On glibc, it is 1 + the highest signal number. On FreeBSD and OpenBSD > at least, it is the highest signal number. The current draft of POSIX issue 8 includes a resolution for https://www.austingroupbugs.net/view.php?id=741 (Add a NSIG constant (or, alternatively, SIGMAX)) However, rather than adding NSIG, they introduced a new sysconf variable _SC_NSIG, defined as the highest supported signal number + 1, and NSIG_MAX, which is defined as follows: Maximum possible return value of sysconf(_SC_NSIG). See XSH sysconf(). The value of {NSIG_MAX} shall be no greater than the number of signals that the sigset_t type (see <signal.h>) is capable of representing, ignoring any restrictions imposed by sigfillset() or sigaddset(). They probably went with this route instead of specifying NSIG due to those inconsistencies you mentioned. Unfortunately, as far as I'm aware, both _SC_NSIG and NSIG_MAX are not yet present in any libc. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [musl] Bug in src/signal/block.c 2021-07-28 17:11 ` Laurent Bercot 2021-07-28 18:43 ` Michael Forney @ 2021-07-28 19:30 ` Rich Felker 1 sibling, 0 replies; 8+ messages in thread From: Rich Felker @ 2021-07-28 19:30 UTC (permalink / raw) To: Laurent Bercot; +Cc: musl On Wed, Jul 28, 2021 at 05:11:11PM +0000, Laurent Bercot wrote: > >> succeed since _NSIG will be 128 instead of 129. > > I happen to be in the process of updating my programming library > performing workarounds for badly-specified parts of POSIX and related > functions. > > NSIG is one of those parts. It is not specified by POSIX, but it is > useful to have a walkable (as in, not 8*sizeof(sigset_t)) upper bound > for the number of signals on a system. I thought it was going to be POSIX-future, but maybe they did the ugly _SC_NSIG-only approach instead (which partly defeats the purpose, although you can use 8*sizeof(sigset_t) as an ugly upper bound on NSIG where you need a constant expression). > But NSIG is badly specified even across the systems where it exists. > On glibc, it is 1 + the highest signal number. On FreeBSD and OpenBSD > at least, it is the highest signal number. That's unfortunate. > musl appears to align on glibc; I suppose the value for MIPS will be > updated to 129, for consistency. No. The greatest signal number on mips is 127, not 128. There's a long story behind this and it involves the kernel doing stupid stuff. Inspect sys/wait.h if you want to try to figure it out yourself. :-) Rich ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [musl] Bug in src/signal/block.c 2021-07-28 15:53 ` Rich Felker 2021-07-28 17:11 ` Laurent Bercot @ 2021-07-28 19:36 ` Rich Felker 2021-07-28 19:52 ` Jasper Hugunin 1 sibling, 1 reply; 8+ messages in thread From: Rich Felker @ 2021-07-28 19:36 UTC (permalink / raw) To: Jasper Hugunin; +Cc: musl [-- Attachment #1: Type: text/plain, Size: 1385 bytes --] 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 [-- Attachment #2: sigblock-mips.diff --] [-- Type: text/plain, Size: 433 bytes --] diff --git a/src/signal/block.c b/src/signal/block.c index d7f61001..cc8698f0 100644 --- a/src/signal/block.c +++ b/src/signal/block.c @@ -3,9 +3,9 @@ #include <signal.h> static const unsigned long all_mask[] = { -#if ULONG_MAX == 0xffffffff && _NSIG == 129 +#if ULONG_MAX == 0xffffffff && _NSIG > 65 -1UL, -1UL, -1UL, -1UL -#elif ULONG_MAX == 0xffffffff +#elif ULONG_MAX == 0xffffffff || _NSIG > 65 -1UL, -1UL #else -1UL ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [musl] Bug in src/signal/block.c 2021-07-28 19:36 ` Rich Felker @ 2021-07-28 19:52 ` Jasper Hugunin 2021-07-28 22:04 ` Rich Felker 0 siblings, 1 reply; 8+ messages in thread From: Jasper Hugunin @ 2021-07-28 19:52 UTC (permalink / raw) To: Rich Felker; +Cc: musl [-- Attachment #1: Type: text/plain, Size: 1966 bytes --] 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.) Sincerely, - Jasper Hugunin 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 > [-- Attachment #2: Type: text/html, Size: 2680 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [musl] Bug in src/signal/block.c 2021-07-28 19:52 ` Jasper Hugunin @ 2021-07-28 22:04 ` Rich Felker 0 siblings, 0 replies; 8+ messages in thread From: Rich Felker @ 2021-07-28 22:04 UTC (permalink / raw) To: Jasper Hugunin; +Cc: musl 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 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-07-28 22:05 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-07-28 15:00 [musl] Bug in src/signal/block.c Jasper Hugunin 2021-07-28 15:53 ` Rich Felker 2021-07-28 17:11 ` Laurent Bercot 2021-07-28 18:43 ` Michael Forney 2021-07-28 19:30 ` Rich Felker 2021-07-28 19:36 ` Rich Felker 2021-07-28 19:52 ` Jasper Hugunin 2021-07-28 22:04 ` Rich Felker
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/musl/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).