On 08/05/18(Tue) 14:57, Scott Cheloha wrote:
> Hi,
>
> futex(2) doesn't do any range checking for timeout for FUTEX_WAIT,
> though recent Linux does so. I assume we'd also want to validate
> timeout before waiting.
Could you use timespecfix() for that? Look how pselect(2) or ppoll(2)
do it. Btw pirofti@ had a similar diff.
> The attached returns EINVAL if timeout's fields are non-normal,
> documents it, and adds a test case.
Nice!
> Index: lib/libc/sys/futex.2
> ===================================================================
> RCS file: /cvs/src/lib/libc/sys/futex.2,v
> retrieving revision 1.4
> diff -u -p -r1.4 futex.2
> --- lib/libc/sys/futex.2 24 Apr 2018 17:19:35 -0000 1.4
> +++ lib/libc/sys/futex.2 8 May 2018 19:49:27 -0000
> @@ -116,6 +116,10 @@ The value pointed to by
> .Fa uaddr
> is not the same as the expected value
> .Fa val .
> +.It Bq Er EINVAL
> +.Fa timeout
> +specified a second value less than zero,
> +or a nanosecond value less than zero or greater than or equal to 1000
> million.
> .It Bq Er ETIMEDOUT
> The
> .Fa timeout
> Index: sys/kern/sys_futex.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/sys_futex.c,v
> retrieving revision 1.7
> diff -u -p -r1.7 sys_futex.c
> --- sys/kern/sys_futex.c 24 Apr 2018 17:19:35 -0000 1.7
> +++ sys/kern/sys_futex.c 8 May 2018 19:49:27 -0000
> @@ -214,6 +214,8 @@ futex_wait(uint32_t *uaddr, uint32_t val
> if (KTRPOINT(p, KTR_STRUCT))
> ktrabstimespec(p, &ts);
> #endif
> + if (ts.tv_sec < 0 || ts.tv_nsec < 0 || ts.tv_nsec >= 1000000000)
> + return EINVAL;
> to_ticks = (uint64_t)hz * ts.tv_sec +
> (ts.tv_nsec + tick * 1000 - 1) / (tick * 1000) + 1;
> if (to_ticks > INT_MAX)
> Index: regress/sys/kern/futex/futex.c
> ===================================================================
> RCS file: /cvs/src/regress/sys/kern/futex/futex.c,v
> retrieving revision 1.2
> diff -u -p -r1.2 futex.c
> --- regress/sys/kern/futex/futex.c 30 Apr 2017 10:11:03 -0000 1.2
> +++ regress/sys/kern/futex/futex.c 8 May 2018 19:49:27 -0000
> @@ -37,7 +37,7 @@ int
> main(int argc, char *argv[])
> {
> struct sigaction sa;
> - struct timespec abs = { 0, 5000 };
> + struct timespec abs = { 0, 5000 }, bogus;
> pthread_t thread;
>
> /* Invalid operation */
> @@ -48,6 +48,17 @@ main(int argc, char *argv[])
>
> /* If (lock != 1) return EAGAIN */
> assert(futex_twait(&lock, 1, 0, NULL) == EAGAIN);
> +
> + /* Invalid timeout */
> + bogus.tv_sec = -1;
> + bogus.tv_nsec = 0;
> + assert(futex(&lock, FUTEX_WAIT, lock, &bogus, NULL) == EINVAL);
> + bogus.tv_sec = 0;
> + bogus.tv_nsec = -1;
> + assert(futex(&lock, FUTEX_WAIT, lock, &bogus, NULL) == EINVAL);
> + bogus.tv_sec = 0;
> + bogus.tv_nsec = 1000000001;
> + assert(futex(&lock, FUTEX_WAIT, lock, &bogus, NULL) == EINVAL);
>
> /* Deadlock for 5000ns */
> assert(futex_twait(&lock, 0, CLOCK_REALTIME, &abs) == ETIMEDOUT);
