Date: Tue, 20 Jul 2010 12:00:44 -0700 From: Garrett Cooper <yanegomi@gmail.com> To: Kostik Belousov <kostikbel@gmail.com> Cc: hackers@freebsd.org Subject: Re: [PATCH] Catch errors with sigaddset(3) in sigaddset (sighold) Message-ID: <AANLkTin_zxwyPPbnQWSCMtix9iHFo7nVRg4g1-Z_UTX1@mail.gmail.com> In-Reply-To: <20100719120916.GX2381@deviant.kiev.zoral.com.ua> References: <AANLkTik4fPb1WwHZzrFop_nIR6xxl_Ygw8Av3Z5cIb4Q@mail.gmail.com> <20100719120916.GX2381@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Jul 19, 2010 at 5:09 AM, Kostik Belousov <kostikbel@gmail.com> wrot=
e:
> On Sun, Jul 18, 2010 at 10:46:25PM -0700, Garrett Cooper wrote:
>> =A0 =A0 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
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>> --- sigcompat.c =A0 =A0 =A0 (revision 210226)
>> +++ sigcompat.c =A0 =A0 =A0 (working copy)
>> @@ -131,7 +131,8 @@
>> =A0 =A0 =A0 sigset_t set;
>>
>> =A0 =A0 =A0 sigemptyset(&set);
>> - =A0 =A0 sigaddset(&set, sig);
>> + =A0 =A0 if (sigaddset(&set, sig) =3D=3D -1)
>> + =A0 =A0 =A0 =A0 =A0 =A0 return (-1);
>> =A0 =A0 =A0 return (_sigprocmask(SIG_BLOCK, &set, NULL));
>> =A0}
>
> 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/sigcompa=
t.c
> index 6841eeb..199846f 100644
> --- a/lib/libc/compat-43/sigcompat.c
> +++ b/lib/libc/compat-43/sigcompat.c
> @@ -112,16 +112,11 @@ int
> =A0xsi_sigpause(int sig)
> =A0{
> =A0 =A0 =A0 =A0sigset_t set;
> - =A0 =A0 =A0 int error;
>
> - =A0 =A0 =A0 if (!_SIG_VALID(sig)) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 errno =3D EINVAL;
> + =A0 =A0 =A0 if (_sigprocmask(SIG_BLOCK, NULL, &set) =3D=3D -1)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (-1);
> + =A0 =A0 =A0 if (sigdelset(&set, sig) =3D=3D -1)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return (-1);
> - =A0 =A0 =A0 }
> - =A0 =A0 =A0 error =3D _sigprocmask(SIG_BLOCK, NULL, &set);
> - =A0 =A0 =A0 if (error !=3D 0)
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (error);
> - =A0 =A0 =A0 sigdelset(&set, sig);
> =A0 =A0 =A0 =A0return (_sigsuspend(&set));
> =A0}
>
> @@ -131,7 +126,8 @@ sighold(int sig)
> =A0 =A0 =A0 =A0sigset_t set;
>
> =A0 =A0 =A0 =A0sigemptyset(&set);
> - =A0 =A0 =A0 sigaddset(&set, sig);
> + =A0 =A0 =A0 if (sigaddset(&set, sig) =3D=3D -1)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (-1);
> =A0 =A0 =A0 =A0return (_sigprocmask(SIG_BLOCK, &set, NULL));
> =A0}
>
> @@ -151,7 +147,8 @@ sigrelse(int sig)
> =A0 =A0 =A0 =A0sigset_t set;
>
> =A0 =A0 =A0 =A0sigemptyset(&set);
> - =A0 =A0 =A0 sigaddset(&set, sig);
> + =A0 =A0 =A0 if (sigaddset(&set, sig) =3D=3D -1)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (-1);
> =A0 =A0 =A0 =A0return (_sigprocmask(SIG_UNBLOCK, &set, NULL));
> =A0}
>
> @@ -160,35 +157,30 @@ void
> =A0{
> =A0 =A0 =A0 =A0sigset_t set, pset;
> =A0 =A0 =A0 =A0struct sigaction sa, psa;
> - =A0 =A0 =A0 int error;
>
> =A0 =A0 =A0 =A0sigemptyset(&set);
> - =A0 =A0 =A0 sigaddset(&set, sig);
> - =A0 =A0 =A0 error =3D _sigprocmask(SIG_BLOCK, NULL, &pset);
> - =A0 =A0 =A0 if (error =3D=3D -1)
> + =A0 =A0 =A0 if (sigaddset(&set, sig) =3D=3D -1)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 return (SIG_ERR);
> + =A0 =A0 =A0 if (_sigprocmask(SIG_BLOCK, NULL, &pset) =3D=3D -1)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return (SIG_ERR);
> =A0 =A0 =A0 =A0if ((__sighandler_t *)disp =3D=3D SIG_HOLD) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 error =3D _sigprocmask(SIG_BLOCK, &set, &ps=
et);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (error =3D=3D -1)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (_sigprocmask(SIG_BLOCK, &set, &pset) =
=3D=3D -1)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return (SIG_ERR);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (sigismember(&pset, sig))
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return (SIG_HOLD);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0else {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 error =3D _sigaction(sig, N=
ULL, &psa);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (error =3D=3D -1)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (_sigaction(sig, NULL, &=
psa) =3D=3D -1)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return (SI=
G_ERR);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return (psa.sa_handler);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0} else {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 error =3D _sigprocmask(SIG_UNBLOCK, &set, &=
pset);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (error =3D=3D -1)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (_sigprocmask(SIG_UNBLOCK, &set, &pset) =
=3D=3D -1)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return (SIG_ERR);
> =A0 =A0 =A0 =A0}
>
> =A0 =A0 =A0 =A0bzero(&sa, sizeof(sa));
> =A0 =A0 =A0 =A0sa.sa_handler =3D disp;
> - =A0 =A0 =A0 error =3D _sigaction(sig, &sa, &psa);
> - =A0 =A0 =A0 if (error =3D=3D -1)
> + =A0 =A0 =A0 if (_sigaction(sig, &sa, &psa) =3D=3D -1)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return (SIG_ERR);
> =A0 =A0 =A0 =A0if (sigismember(&pset, sig))
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return (SIG_HOLD);
The results when I ran the tests manually outside of the shell
script were ok. I need to track down why they're failing in the script
itself.
Thanks,
-Garrett
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTin_zxwyPPbnQWSCMtix9iHFo7nVRg4g1-Z_UTX1>