[PATCH] fs/select: reject negative timeval components in kern_select()

[PATCH] fs/select: reject negative timeval components in kern_select()

[Breno Leitao]

kern_select() normalises the user-supplied struct __kernel_old_timeval
with

	tv.tv_sec + (tv.tv_usec / USEC_PER_SEC)
	(tv.tv_usec % USEC_PER_SEC) * NSEC_PER_USEC

before calling poll_select_set_timeout() -> timespec64_valid().  Both
operands of the seconds sum are unbounded user-controlled signed long.
A crafted pair where tv_usec is a negative multiple of USEC_PER_SEC
drives the sum across the wrap boundary - e.g.

	{ .tv_sec = LONG_MIN, .tv_usec = -1000000 }

yields sec = LONG_MAX, nsec = 0, which passes timespec64_valid() and
then flows through timespec64_add_safe(), which saturates the absolute
deadline to TIME64_MAX (clamped further to KTIME_MAX downstream).
select(2) therefore blocks effectively forever instead of returning
-EINVAL as POSIX requires for a negative timeout.

Only the legacy __NR_select syscall takes this path.  pselect6, ppoll,
poll and epoll_pwait2 all hand the user's two fields directly to
poll_select_set_timeout(), which validates *before* doing any
arithmetic:

	/* fs/select.c:271 -- the validator */
	int poll_select_set_timeout(struct timespec64 *to, time64_t sec, long nsec)
	{
		struct timespec64 ts = {.tv_sec = sec, .tv_nsec = nsec};
		if (!timespec64_valid(&ts))
			return -EINVAL;
		...
	}

	/* include/linux/time64.h:97 -- timespec64_valid */
	if (ts->tv_sec < 0)                              return false;
	if ((unsigned long)ts->tv_nsec >= NSEC_PER_SEC)  return false;

	/* fs/select.c:744  do_pselect() (pselect6, pselect6_time32) */
	if (get_timespec64(&ts, tsp)) return -EFAULT;
	if (poll_select_set_timeout(to, ts.tv_sec, ts.tv_nsec)) return -EINVAL;

	/* fs/select.c:1097 ppoll */
	if (get_timespec64(&ts, tsp)) return -EFAULT;
	if (poll_select_set_timeout(to, ts.tv_sec, ts.tv_nsec)) return -EINVAL;

	/* fs/select.c:1065 poll -- timeout_msecs is int; >= 0 gates the math */
	if (timeout_msecs >= 0)
		poll_select_set_timeout(to, timeout_msecs / MSEC_PER_SEC,
		                        NSEC_PER_MSEC * (timeout_msecs % MSEC_PER_SEC));

	/* fs/eventpoll.c:2512 epoll_pwait2 */
	if (get_timespec64(&ts, timeout)) return -EFAULT;
	if (poll_select_set_timeout(to, ts.tv_sec, ts.tv_nsec)) return -EINVAL;

In every one of these the wrap-prone arithmetic from kern_select()
simply does not exist; the user fields reach timespec64_valid()
unmodified.  glibc routes the C-library select() through pselect6,
so the bug is reachable only via a direct syscall(__NR_select, ...).

The pre-validation negative check that used to live here was lost
when the syscall was switched to the poll_select_set_timeout() helper.
Restore it: reject tv_sec < 0 || tv_usec < 0 up front, mirroring what
glibc does in userspace.  do_compat_select() has the same arithmetic
pattern but is only reachable on 32-bit compat and from a different
syscall entry; left for a follow-up so this change stays minimal.

Reproducer (returns -1/EINVAL on a fixed kernel; blocks indefinitely
on an unfixed one):

	struct timeval tv = { .tv_sec = LONG_MIN, .tv_usec = -1000000 };
	fd_set r;
	int pfd[2];
	pipe(pfd);
	FD_ZERO(&r);
	FD_SET(pfd[0], &r);
	syscall(__NR_select, pfd[0] + 1, &r, NULL, NULL, &tv);

Fixes: 4d36a9e65d49 ("select: deal with math overflow from borderline valid userland data")
Signed-off-by: Breno Leitao <leitao@debian.org>
---
 fs/select.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/fs/select.c b/fs/select.c
index 75978b18f48f8..bf71c9838dfe1 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -708,6 +708,17 @@ static int kern_select(int n, fd_set __user *inp, fd_set __user *outp,
 		if (copy_from_user(&tv, tvp, sizeof(tv)))
 			return -EFAULT;
 
+		/*
+		 * Reject negative components before normalisation. The seconds
+		 * sum below is performed in signed long and a crafted negative
+		 * timeval can wrap to a positive value that passes
+		 * timespec64_valid() and turns into an effectively-infinite
+		 * deadline via timespec64_add_safe()'s saturation, instead of
+		 * the -EINVAL POSIX requires for negative timeouts.
+		 */
+		if (tv.tv_sec < 0 || tv.tv_usec < 0)
+			return -EINVAL;
+
 		to = &end_time;
 		if (poll_select_set_timeout(to,
 				tv.tv_sec + (tv.tv_usec / USEC_PER_SEC),

---
base-commit: 9974969c14031a097d6b45bcb7a06bb4aa525c40
change-id: 20260429-timeval-8a3498dde479

Best regards,
--  
Breno Leitao <leitao@debian.org>

Re: [PATCH] fs/select: reject negative timeval components in kern_select()

[Jan Kara]

On Wed 29-04-26 06:09:37, Breno Leitao wrote:
> kern_select() normalises the user-supplied struct __kernel_old_timeval
> with
> 
> 	tv.tv_sec + (tv.tv_usec / USEC_PER_SEC)
> 	(tv.tv_usec % USEC_PER_SEC) * NSEC_PER_USEC
> 
> before calling poll_select_set_timeout() -> timespec64_valid().  Both
> operands of the seconds sum are unbounded user-controlled signed long.
> A crafted pair where tv_usec is a negative multiple of USEC_PER_SEC
> drives the sum across the wrap boundary - e.g.
> 
> 	{ .tv_sec = LONG_MIN, .tv_usec = -1000000 }
> 
> yields sec = LONG_MAX, nsec = 0, which passes timespec64_valid() and
> then flows through timespec64_add_safe(), which saturates the absolute
> deadline to TIME64_MAX (clamped further to KTIME_MAX downstream).
> select(2) therefore blocks effectively forever instead of returning
> -EINVAL as POSIX requires for a negative timeout.
> 
> Only the legacy __NR_select syscall takes this path.  pselect6, ppoll,
> poll and epoll_pwait2 all hand the user's two fields directly to
> poll_select_set_timeout(), which validates *before* doing any
> arithmetic:
> 
> 	/* fs/select.c:271 -- the validator */
> 	int poll_select_set_timeout(struct timespec64 *to, time64_t sec, long nsec)
> 	{
> 		struct timespec64 ts = {.tv_sec = sec, .tv_nsec = nsec};
> 		if (!timespec64_valid(&ts))
> 			return -EINVAL;
> 		...
> 	}
> 
> 	/* include/linux/time64.h:97 -- timespec64_valid */
> 	if (ts->tv_sec < 0)                              return false;
> 	if ((unsigned long)ts->tv_nsec >= NSEC_PER_SEC)  return false;
> 
> 	/* fs/select.c:744  do_pselect() (pselect6, pselect6_time32) */
> 	if (get_timespec64(&ts, tsp)) return -EFAULT;
> 	if (poll_select_set_timeout(to, ts.tv_sec, ts.tv_nsec)) return -EINVAL;
> 
> 	/* fs/select.c:1097 ppoll */
> 	if (get_timespec64(&ts, tsp)) return -EFAULT;
> 	if (poll_select_set_timeout(to, ts.tv_sec, ts.tv_nsec)) return -EINVAL;
> 
> 	/* fs/select.c:1065 poll -- timeout_msecs is int; >= 0 gates the math */
> 	if (timeout_msecs >= 0)
> 		poll_select_set_timeout(to, timeout_msecs / MSEC_PER_SEC,
> 		                        NSEC_PER_MSEC * (timeout_msecs % MSEC_PER_SEC));
> 
> 	/* fs/eventpoll.c:2512 epoll_pwait2 */
> 	if (get_timespec64(&ts, timeout)) return -EFAULT;
> 	if (poll_select_set_timeout(to, ts.tv_sec, ts.tv_nsec)) return -EINVAL;
> 
> In every one of these the wrap-prone arithmetic from kern_select()
> simply does not exist; the user fields reach timespec64_valid()
> unmodified.  glibc routes the C-library select() through pselect6,
> so the bug is reachable only via a direct syscall(__NR_select, ...).
> 
> The pre-validation negative check that used to live here was lost
> when the syscall was switched to the poll_select_set_timeout() helper.
> Restore it: reject tv_sec < 0 || tv_usec < 0 up front, mirroring what
> glibc does in userspace.  do_compat_select() has the same arithmetic
> pattern but is only reachable on 32-bit compat and from a different
> syscall entry; left for a follow-up so this change stays minimal.
> 
> Reproducer (returns -1/EINVAL on a fixed kernel; blocks indefinitely
> on an unfixed one):
> 
> 	struct timeval tv = { .tv_sec = LONG_MIN, .tv_usec = -1000000 };
> 	fd_set r;
> 	int pfd[2];
> 	pipe(pfd);
> 	FD_ZERO(&r);
> 	FD_SET(pfd[0], &r);
> 	syscall(__NR_select, pfd[0] + 1, &r, NULL, NULL, &tv);
> 
> Fixes: 4d36a9e65d49 ("select: deal with math overflow from borderline valid userland data")
> Signed-off-by: Breno Leitao <leitao@debian.org>

Looks good. I just wonder whether we shouldn't also check that tv.tv_usec <
USEC_PER_SEC. But in any case feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/select.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/fs/select.c b/fs/select.c
> index 75978b18f48f8..bf71c9838dfe1 100644
> --- a/fs/select.c
> +++ b/fs/select.c
> @@ -708,6 +708,17 @@ static int kern_select(int n, fd_set __user *inp, fd_set __user *outp,
>  		if (copy_from_user(&tv, tvp, sizeof(tv)))
>  			return -EFAULT;
>  
> +		/*
> +		 * Reject negative components before normalisation. The seconds
> +		 * sum below is performed in signed long and a crafted negative
> +		 * timeval can wrap to a positive value that passes
> +		 * timespec64_valid() and turns into an effectively-infinite
> +		 * deadline via timespec64_add_safe()'s saturation, instead of
> +		 * the -EINVAL POSIX requires for negative timeouts.
> +		 */
> +		if (tv.tv_sec < 0 || tv.tv_usec < 0)
> +			return -EINVAL;
> +
>  		to = &end_time;
>  		if (poll_select_set_timeout(to,
>  				tv.tv_sec + (tv.tv_usec / USEC_PER_SEC),
> 
> ---
> base-commit: 9974969c14031a097d6b45bcb7a06bb4aa525c40
> change-id: 20260429-timeval-8a3498dde479
> 
> Best regards,
> --  
> Breno Leitao <leitao@debian.org>
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] fs/select: reject negative timeval components in kern_select()

[Breno Leitao]

On Thu, Apr 30, 2026 at 09:33:01AM +0200, Jan Kara wrote:
> On Wed 29-04-26 06:09:37, Breno Leitao wrote:
> > kern_select() normalises the user-supplied struct __kernel_old_timeval
> > with
> >
> > 	tv.tv_sec + (tv.tv_usec / USEC_PER_SEC)
> > 	(tv.tv_usec % USEC_PER_SEC) * NSEC_PER_USEC
> >
> > before calling poll_select_set_timeout() -> timespec64_valid().  Both
> > operands of the seconds sum are unbounded user-controlled signed long.
> > A crafted pair where tv_usec is a negative multiple of USEC_PER_SEC
> > drives the sum across the wrap boundary - e.g.
> >
> > 	{ .tv_sec = LONG_MIN, .tv_usec = -1000000 }
> >
> > yields sec = LONG_MAX, nsec = 0, which passes timespec64_valid() and
> > then flows through timespec64_add_safe(), which saturates the absolute
> > deadline to TIME64_MAX (clamped further to KTIME_MAX downstream).
> > select(2) therefore blocks effectively forever instead of returning
> > -EINVAL as POSIX requires for a negative timeout.
> >
> > Only the legacy __NR_select syscall takes this path.  pselect6, ppoll,
> > poll and epoll_pwait2 all hand the user's two fields directly to
> > poll_select_set_timeout(), which validates *before* doing any
> > arithmetic:
> >
> > 	/* fs/select.c:271 -- the validator */
> > 	int poll_select_set_timeout(struct timespec64 *to, time64_t sec, long nsec)
> > 	{
> > 		struct timespec64 ts = {.tv_sec = sec, .tv_nsec = nsec};
> > 		if (!timespec64_valid(&ts))
> > 			return -EINVAL;
> > 		...
> > 	}
> >
> > 	/* include/linux/time64.h:97 -- timespec64_valid */
> > 	if (ts->tv_sec < 0)                              return false;
> > 	if ((unsigned long)ts->tv_nsec >= NSEC_PER_SEC)  return false;
> >
> > 	/* fs/select.c:744  do_pselect() (pselect6, pselect6_time32) */
> > 	if (get_timespec64(&ts, tsp)) return -EFAULT;
> > 	if (poll_select_set_timeout(to, ts.tv_sec, ts.tv_nsec)) return -EINVAL;
> >
> > 	/* fs/select.c:1097 ppoll */
> > 	if (get_timespec64(&ts, tsp)) return -EFAULT;
> > 	if (poll_select_set_timeout(to, ts.tv_sec, ts.tv_nsec)) return -EINVAL;
> >
> > 	/* fs/select.c:1065 poll -- timeout_msecs is int; >= 0 gates the math */
> > 	if (timeout_msecs >= 0)
> > 		poll_select_set_timeout(to, timeout_msecs / MSEC_PER_SEC,
> > 		                        NSEC_PER_MSEC * (timeout_msecs % MSEC_PER_SEC));
> >
> > 	/* fs/eventpoll.c:2512 epoll_pwait2 */
> > 	if (get_timespec64(&ts, timeout)) return -EFAULT;
> > 	if (poll_select_set_timeout(to, ts.tv_sec, ts.tv_nsec)) return -EINVAL;
> >
> > In every one of these the wrap-prone arithmetic from kern_select()
> > simply does not exist; the user fields reach timespec64_valid()
> > unmodified.  glibc routes the C-library select() through pselect6,
> > so the bug is reachable only via a direct syscall(__NR_select, ...).
> >
> > The pre-validation negative check that used to live here was lost
> > when the syscall was switched to the poll_select_set_timeout() helper.
> > Restore it: reject tv_sec < 0 || tv_usec < 0 up front, mirroring what
> > glibc does in userspace.  do_compat_select() has the same arithmetic
> > pattern but is only reachable on 32-bit compat and from a different
> > syscall entry; left for a follow-up so this change stays minimal.
> >
> > Reproducer (returns -1/EINVAL on a fixed kernel; blocks indefinitely
> > on an unfixed one):
> >
> > 	struct timeval tv = { .tv_sec = LONG_MIN, .tv_usec = -1000000 };
> > 	fd_set r;
> > 	int pfd[2];
> > 	pipe(pfd);
> > 	FD_ZERO(&r);
> > 	FD_SET(pfd[0], &r);
> > 	syscall(__NR_select, pfd[0] + 1, &r, NULL, NULL, &tv);
> >
> > Fixes: 4d36a9e65d49 ("select: deal with math overflow from borderline valid userland data")
> > Signed-off-by: Breno Leitao <leitao@debian.org>
>
> Looks good. I just wonder whether we shouldn't also check that tv.tv_usec <
> USEC_PER_SEC. But in any case feel free to add:

Good question. I opted not to add that check because it would represent
an ABI change—for example, it would start rejecting { .tv_sec = 0,
.tv_usec = 2000000 }.

That said, glibc already performs this check, so applications using libc
already have this constraint.

I'm comfortable with either approach.

Thanks for the review,
--breno