On Fri, Nov 15, 2019 at 11:27 AM Rasmus Villemoes linux@rasmusvillemoes.dk wrote:
On 15/11/2019 08.58, Arnd Bergmann wrote:
On Fri, Nov 15, 2019 at 12:01 AM Abel Vesa abelvesa@linux.com wrote:
--- a/kernel/time/time.c +++ b/kernel/time/time.c @@ -207,7 +207,7 @@ SYSCALL_DEFINE2(settimeofday, struct __kernel_old_timeval __user *, tv, get_user(new_ts.tv_nsec, &tv->tv_usec)) return -EFAULT;
if (tv->tv_usec > USEC_PER_SEC)
if (new_ts->tv_usec > USEC_PER_SEC) return -EINVAL;Hopefully not :)
No, I misquoted from a fix that I had temporarily applied, not the version in linux-next.
new_ts.tv_nsec *= NSEC_PER_USEC;So the actual patch in next-20191115 does
if (copy_from_user(&user_tv, tv, sizeof(*tv)))
if (get_user(new_ts.tv_sec, &tv->tv_sec) ||get_user(new_ts.tv_nsec, &tv->tv_usec)) return -EFAULT;
if (!timeval_valid(&user_tv))
if (new_ts.tv_nsec > USEC_PER_SEC) return -EINVAL;
new_ts.tv_sec = user_tv.tv_sec;new_ts.tv_nsec = user_tv.tv_usec * NSEC_PER_USEC;
new_ts.tv_nsec *= NSEC_PER_USEC;But removing the "user value is < 0" check, relying on the timespec value being rejected later, is wrong
You are right of course, so many ways to get this one line wrong... Pushed more more update to the branch now.
Thanks for the careful review!
Arnd