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

[Jan Kara]

On Sat 31-10-26 10:31:00, Avi Weiss wrote:
> do_compat_select() normalizes old_timeval32 before validating the
> timeout. Negative seconds and microseconds can cancel each other,
> causing compat select() and old_select() to accept invalid timeouts.

Yes, but who cares? You can pass various strange values (like making sec +
usec / USEC_PER_SEC overflow) resulting in odd behavior but you are only
shooting yourself in the foot... So if it doesn't hurt integrity of the
code flow, I'd just ignore this.

								Honza

> 
> Reject negative components before normalization (as kern_select()
> already does) - and preserve the handling of oversized positive
> tv_usec values.
> 
> A 32-bit syscall reproducer confirmed four incorrect results on the
> unpatched kernel. With the fix applied, all my test cases passed on
> both compat select interfaces.
> 
> Fixes: 4d36a9e65d49 ("select: deal with math overflow from borderline valid userland data")
> Signed-off-by: Avi Weiss <thnkslprpt@xxxxxxxxx>
> ---
>  fs/select.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/fs/select.c b/fs/select.c
> index 95d76531015aa..8b6d1dd2977c6 100644
> --- a/fs/select.c
> +++ b/fs/select.c
> @@ -1265,6 +1265,8 @@ static int do_compat_select(int n, compat_ulong_t __user *inp,
>  		if (copy_from_user(&tv, tvp, sizeof(tv)))
>  			return -EFAULT;
>  
> +		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: 36aad21d50d1af3f6954bb0c988257a908202477
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR