kern/149980: [patch] negative value integer to nanosleep(2) should fail with EINVAL

[Garrett Cooper]

The following reply was made to PR kern/149980; it has been noted by GNATS.

From: Garrett Cooper <gcooper at FreeBSD.org>
To: Garrett Cooper <gcooper at freebsd.org>
Cc: vwe at freebsd.org, bug-followup <bug-followup at freebsd.org>
Subject: Re: kern/149980: [patch] negative value integer to nanosleep(2)
 should fail with EINVAL
Date: Sun, 29 Aug 2010 13:17:44 -0700

 On Sun, Aug 29, 2010 at 1:16 PM, Garrett Cooper <gcooper at freebsd.org> wrote=
 :
 > On Sun, Aug 29, 2010 at 1:03 PM, =A0<vwe at freebsd.org> wrote:
 >> Old Synopsis: [PATCH] negative value integer to nanosleep(2) should fail=
  with EINVAL
 >> New Synopsis: [patch] negative value integer to nanosleep(2) should fail=
  with EINVAL
 >>
 >> State-Changed-From-To: open->analyzed
 >> State-Changed-By: vwe
 >> State-Changed-When: Sun Aug 29 20:00:22 UTC 2010
 >> State-Changed-Why:
 >> double checked that and it's looking reasonable
 >> I think the checks for 'tv_nsec < 0' and 'tv_sec < 0' can be made in one=
  go,
 >> but IMO it should not make a difference (assembler wise):
 >>
 >> Index: sys/kern/kern_time.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
 >> --- sys/kern/kern_time.c =A0 =A0 =A0 =A0(revision 211522)
 >> +++ sys/kern/kern_time.c =A0 =A0 =A0 =A0(working copy)
 >> @@ -362,9 +362,9 @@
 >> =A0 =A0 =A0 =A0struct timeval tv;
 >> =A0 =A0 =A0 =A0int error;
 >>
 >> - =A0 =A0 =A0 if (rqt->tv_nsec < 0 || rqt->tv_nsec >=3D 1000000000)
 >> + =A0 =A0 =A0 if (rqt->tv_nsec < 0 || rqt->tv_nsec >=3D 1000000000 || rq=
 t->tv_sec < 0)
 >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return (EINVAL);
 >> - =A0 =A0 =A0 if (rqt->tv_sec < 0 || (rqt->tv_sec =3D=3D 0 && rqt->tv_ns=
 ec =3D=3D 0))
 >> + =A0 =A0 =A0 if (rqt->tv_sec =3D=3D 0 && rqt->tv_nsec =3D=3D 0)
 >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0return (0);
 >> =A0 =A0 =A0 =A0getnanouptime(&ts);
 >> =A0 =A0 =A0 =A0timespecadd(&ts, rqt);
 >
 > Same thing that bde@ asked me to create, so it naturally looks good :).
 >
 > The reason why I hadn't posted anything earlier about this bug is that
 > bde@ brought it to my attention that there are additional issues with
 > the timer code, mostly dealing with the fact that itimerfix isn't used
 > when checking the bounds of the tv argument. There are other
 > associated issues with using this though (itimerfix checks tv_msec,
 > and nanosleep doesn't check the tv_msec field because nanosleep uses
 > nanosecond granularity, not millisecond granularity).
 
 One other thing that I failed to mention. itimerfix is used as a
 one-size fit-all solution in a lot of of pieces of code, s.t. it would
 affect other items like select(2), etc.
 -Garrett