Re: [PATCH 17/23] y2038: time: avoid timespec usage in settimeofday()

[Thomas Gleixner]

On Fri, 8 Nov 2019, Arnd Bergmann wrote:
> -SYSCALL_DEFINE2(settimeofday, struct timeval __user *, tv,
> +SYSCALL_DEFINE2(settimeofday, struct __kernel_old_timeval __user *, tv,
>  		struct timezone __user *, tz)
>  {
>  	struct timespec64 new_ts;
> -	struct timeval user_tv;
>  	struct timezone new_tz;
>  
>  	if (tv) {
> -		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;

How is that supposed to be correct on a 32bit kernel?

>  
> -		if (!timeval_valid(&user_tv))
> +		if (tv->tv_usec > USEC_PER_SEC)
>  			return -EINVAL;

That's incomplete:

static inline bool timeval_valid(const struct timeval *tv)
{
        /* Dates before 1970 are bogus */
        if (tv->tv_sec < 0)
                return false;

	/* Can't have more microseconds then a second */
        if (tv->tv_usec < 0 || tv->tv_usec >= USEC_PER_SEC)
                return false;

        return true;
}


>  
> -		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;
>  	}
>  	if (tz) {
>  		if (copy_from_user(&new_tz, tz, sizeof(*tz)))
> @@ -245,18 +244,17 @@ COMPAT_SYSCALL_DEFINE2(settimeofday, struct old_timeval32 __user *, tv,
>  		       struct timezone __user *, tz)
>  {
>  	struct timespec64 new_ts;
> -	struct timeval user_tv;
>  	struct timezone new_tz;
>  
>  	if (tv) {
> -		if (compat_get_timeval(&user_tv, 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;

Ditto.

Thanks,

	tglx