[musl] [PATCH] set EINVAL for sigismember when sig is invalid

[musl] [PATCH] set EINVAL for sigismember when sig is invalid

[lihua.zhao.cn]

From: Lihua Zhao <lihua.zhao.cn@windriver.com>

---
 src/signal/sigismember.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/signal/sigismember.c b/src/signal/sigismember.c
index ab87d622..0b33888c 100644
--- a/src/signal/sigismember.c
+++ b/src/signal/sigismember.c
@@ -1,8 +1,12 @@
 #include <signal.h>
+#include <errno.h>
 
 int sigismember(const sigset_t *set, int sig)
 {
 	unsigned s = sig-1;
-	if (s >= _NSIG-1) return 0;
+	if (s < 0 || s >= _NSIG-1) {
+		errno = EINVAL;
+		return -1;
+	}
 	return !!(set->__bits[s/8/sizeof *set->__bits] & 1UL<<(s&8*sizeof *set->__bits-1));
 }
-- 
2.34.1

Re: [musl] [PATCH] set EINVAL for sigismember when sig is invalid

[Thorsten Glaser]

On Tue, 29 Oct 2024, lihua.zhao.cn@windriver.com wrote:

> int sigismember(const sigset_t *set, int sig)
> {
> 	unsigned s = sig-1;
>-	if (s >= _NSIG-1) return 0;
>+	if (s < 0 || s >= _NSIG-1) {

unsigned s can never be 0, and assignment from int will wrap around,
so the >= is enough.

There’s UB if sig == INT_MIN though.

- 	unsigned s = sig-1;
+ 	unsigned s = sig;
+ 	++s;

Or:

- 	unsigned s = sig-1;
+ 	unsigned s = (unsigned)sig - 1U;

bye,
//mirabilos
-- 
In traditional syntax ' is ignored, but in c99 everything between two ' is
handled as character constant.  Therefore you cannot use ' in a preproces-
sing file in c99 mode.	-- Ragge
No faith left in ISO C99, undefined behaviour, etc.

Re: [musl] [PATCH] set EINVAL for sigismember when sig is invalid

[Rich Felker]

On Tue, Oct 29, 2024 at 02:12:49PM +0100, Thorsten Glaser wrote:
> On Tue, 29 Oct 2024, lihua.zhao.cn@windriver.com wrote:
> 
> > int sigismember(const sigset_t *set, int sig)
> > {
> > 	unsigned s = sig-1;
> >-	if (s >= _NSIG-1) return 0;
> >+	if (s < 0 || s >= _NSIG-1) {
> 
> unsigned s can never be 0, and assignment from int will wrap around,
> so the >= is enough.
> 
> There’s UB if sig == INT_MIN though.

The UB concern exists for all the existing files, so it should be
fixed there and the same idiom copied to sigismember. Note that the
above patch for sigismember does not catch signal numbers that are
invalid because they're implementation-internal, like the other sigset
functions do. That needs to be fixed, but then the internal usage in
posix_spawn would need to be fixed to match.

> 
> - 	unsigned s = sig-1;
> + 	unsigned s = sig;
> + 	++s;
> 
> Or:
> 
> - 	unsigned s = sig-1;
> + 	unsigned s = (unsigned)sig - 1U;

sig-1U is the idiomatic way we do this.

Rich

[musl] [PATCH v2] signal: set EINVAL for sigismember when sig is invalid

[lihua.zhao.cn]

From: Lihua Zhao <lihua.zhao.cn@windriver.com>

sigismember() forget to set errno when input sig is invalid
---
 src/signal/sigismember.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/src/signal/sigismember.c b/src/signal/sigismember.c
index ab87d622..3ecd300f 100644
--- a/src/signal/sigismember.c
+++ b/src/signal/sigismember.c
@@ -1,8 +1,12 @@
 #include <signal.h>
+#include <errno.h>

 int sigismember(const sigset_t *set, int sig)
 {
-	unsigned s = sig-1;
-	if (s >= _NSIG-1) return 0;
+	unsigned s = sig-1U;
+	if (s >= _NSIG-1) {
+		errno = EINVAL;
+		return -1;
+	}
 	return !!(set->__bits[s/8/sizeof *set->__bits] & 1UL<<(s&8*sizeof *set->__bits-1));
 }
--
2.34.1