From owner-freebsd-hackers@FreeBSD.ORG Tue Jul 20 19:00:46 2010 Return-Path: Delivered-To: hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 1D2C31065670 for ; Tue, 20 Jul 2010 19:00:46 +0000 (UTC) (envelope-from yanegomi@gmail.com) Received: from mail-iw0-f182.google.com (mail-iw0-f182.google.com [209.85.214.182]) by mx1.freebsd.org (Postfix) with ESMTP id D63EA8FC08 for ; Tue, 20 Jul 2010 19:00:45 +0000 (UTC) Received: by iwn35 with SMTP id 35so7483758iwn.13 for ; Tue, 20 Jul 2010 12:00:45 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=yqkzPvz/67n/P7rPm78riSo1GCjy7mVJFfDAU/8m8QA=; b=qHCA87N5E8DtEL+NM12CnETIy3RyFdK6S/07RY+iJX2nhaPeSQTGYzmiKPq04oHasm 5MrQ2lF3eIP86hELhMukpsNcF6NLfdbiA2Z1miS2XNpqpKQem4cuWngxNXtaxPxJsAsX VuQuf2jj7NU3hCnFPPQTfEsEQgPI4YZybUvgw= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; b=ahKrrGORmAWIFcS066PAl0qUnRDXQ2ax/ETCZEIsD1DZvsbn13eFoqJp6ac84BtunE OA5DtfdOf9SOMzQ2O/WQv9oWTZWJ3QmqjaZBByaW7Iqp+0GVGlwKXHKItdgYfqUBQVrX zgdBN4A6i8wiRD5J1sd/XdzT5+CxwrQEXiYE4= MIME-Version: 1.0 Received: by 10.231.172.83 with SMTP id k19mr8253552ibz.114.1279652444654; Tue, 20 Jul 2010 12:00:44 -0700 (PDT) Received: by 10.231.169.18 with HTTP; Tue, 20 Jul 2010 12:00:44 -0700 (PDT) In-Reply-To: <20100719120916.GX2381@deviant.kiev.zoral.com.ua> References: <20100719120916.GX2381@deviant.kiev.zoral.com.ua> Date: Tue, 20 Jul 2010 12:00:44 -0700 Message-ID: From: Garrett Cooper To: Kostik Belousov Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: hackers@freebsd.org Subject: Re: [PATCH] Catch errors with sigaddset(3) in sigaddset (sighold) X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 20 Jul 2010 19:00:46 -0000 On Mon, Jul 19, 2010 at 5:09 AM, Kostik Belousov 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