Re: [PATCH] Catch errors with sigaddset(3) in sigaddset (sighold)

[Kostik Belousov]

On Sun, Jul 18, 2010 at 10:46:25PM -0700, Garrett Cooper wrote:
>     sighold(3) doesn't determine whether or not the signal added is
> valid today (and sigprocmask doesn't verify that either). This fixes
> that.
> Thanks,
> -Garrett
> 
> Index: sigcompat.c
> ===================================================================
> --- sigcompat.c       (revision 210226)
> +++ sigcompat.c       (working copy)
> @@ -131,7 +131,8 @@
>       sigset_t set;
> 
>       sigemptyset(&set);
> -     sigaddset(&set, sig);
> +     if (sigaddset(&set, sig) == -1)
> +             return (-1);
>       return (_sigprocmask(SIG_BLOCK, &set, NULL));
>  }

I added checks for failures of sig{add,del}set to the sigcompat.c,
and unified style to not assign intermediate error to local variable.

This is what I am going to commit shortly.

diff --git a/lib/libc/compat-43/sigcompat.c b/lib/libc/compat-43/sigcompat.c
index 6841eeb..199846f 100644
--- a/lib/libc/compat-43/sigcompat.c
+++ b/lib/libc/compat-43/sigcompat.c
@@ -112,16 +112,11 @@ int
 xsi_sigpause(int sig)
 {
        sigset_t set;
-       int error;
 
-       if (!_SIG_VALID(sig)) {
-               errno = EINVAL;
+       if (_sigprocmask(SIG_BLOCK, NULL, &set) == -1)
+               return (-1);
+       if (sigdelset(&set, sig) == -1)
                return (-1);
-       }
-       error = _sigprocmask(SIG_BLOCK, NULL, &set);
-       if (error != 0)
-               return (error);
-       sigdelset(&set, sig);
        return (_sigsuspend(&set));
 }
 
@@ -131,7 +126,8 @@ sighold(int sig)
        sigset_t set;
 
        sigemptyset(&set);
-       sigaddset(&set, sig);
+       if (sigaddset(&set, sig) == -1)
+               return (-1);
        return (_sigprocmask(SIG_BLOCK, &set, NULL));
 }
 
@@ -151,7 +147,8 @@ sigrelse(int sig)
        sigset_t set;
 
        sigemptyset(&set);
-       sigaddset(&set, sig);
+       if (sigaddset(&set, sig) == -1)
+               return (-1);
        return (_sigprocmask(SIG_UNBLOCK, &set, NULL));
 }
 
@@ -160,35 +157,30 @@ void
 {
        sigset_t set, pset;
        struct sigaction sa, psa;
-       int error;
 
        sigemptyset(&set);
-       sigaddset(&set, sig);
-       error = _sigprocmask(SIG_BLOCK, NULL, &pset);
-       if (error == -1)
+       if (sigaddset(&set, sig) == -1)
+               return (SIG_ERR);
+       if (_sigprocmask(SIG_BLOCK, NULL, &pset) == -1)
                return (SIG_ERR);
        if ((__sighandler_t *)disp == SIG_HOLD) {
-               error = _sigprocmask(SIG_BLOCK, &set, &pset);
-               if (error == -1)
+               if (_sigprocmask(SIG_BLOCK, &set, &pset) == -1)
                        return (SIG_ERR);
                if (sigismember(&pset, sig))
                        return (SIG_HOLD);
                else {
-                       error = _sigaction(sig, NULL, &psa);
-                       if (error == -1)
+                       if (_sigaction(sig, NULL, &psa) == -1)
                                return (SIG_ERR);
                        return (psa.sa_handler);
                }
        } else {
-               error = _sigprocmask(SIG_UNBLOCK, &set, &pset);
-               if (error == -1)
+               if (_sigprocmask(SIG_UNBLOCK, &set, &pset) == -1)
                        return (SIG_ERR);
        }
 
        bzero(&sa, sizeof(sa));
        sa.sa_handler = disp;
-       error = _sigaction(sig, &sa, &psa);
-       if (error == -1)
+       if (_sigaction(sig, &sa, &psa) == -1)
                return (SIG_ERR);
        if (sigismember(&pset, sig))
                return (SIG_HOLD);

pgp46SckIWt6s.pgp
Description: PGP signature