Date: Sun, 29 Aug 2010 20:20:04 GMT From: Garrett Cooper <gcooper@FreeBSD.org> To: freebsd-bugs@FreeBSD.org Subject: Re: kern/149980: [patch] negative value integer to nanosleep(2) should fail with EINVAL Message-ID: <201008292020.o7TKK4su056758@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
The following reply was made to PR kern/149980; it has been noted by GNATS. From: Garrett Cooper <gcooper@FreeBSD.org> To: Garrett Cooper <gcooper@freebsd.org> Cc: vwe@freebsd.org, bug-followup <bug-followup@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@freebsd.org> wrote= : > On Sun, Aug 29, 2010 at 1:03 PM, =A0<vwe@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201008292020.o7TKK4su056758>