netdev - Re: [PATCH 11/11] k_clock:Remove the 32bit methods with timespec type

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150420084229.GA4608@localhost.localdomain>
Date:	Mon, 20 Apr 2015 10:42:30 +0200
From:	Richard Cochran <richardcochran@...il.com>
To:	Baolin Wang <baolin.wang@...aro.org>
Cc:	tglx@...utronix.de, arnd@...db.de, john.stultz@...aro.org,
	heenasirwani@...il.com, pang.xunlei@...aro.org,
	peterz@...radead.org, rafael.j.wysocki@...el.com,
	gregkh@...uxfoundation.org, benh@...nel.crashing.org,
	paulus@...ba.org, mpe@...erman.id.au, schwidefsky@...ibm.com,
	heiko.carstens@...ibm.com, linux390@...ibm.com, rth@...ddle.net,
	riel@...hat.com, cl@...ux.com, tj@...nel.org, fweisbec@...il.com,
	ahh@...gle.com, pjt@...gle.com, linuxppc-dev@...ts.ozlabs.org,
	linux-s390@...r.kernel.org, linux-arch@...r.kernel.org,
	linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
	y2038@...ts.linaro.org
Subject: Re: [PATCH 11/11] k_clock:Remove the 32bit methods with timespec type

On Mon, Apr 20, 2015 at 01:57:39PM +0800, Baolin Wang wrote:

> @@ -911,18 +907,14 @@ retry:
>  		return -EINVAL;
>  
>  	kc = clockid_to_kclock(timr->it_clock);
> -	if (WARN_ON_ONCE(!kc || (!kc->timer_set && !kc->timer_set64))) {
> +	if (WARN_ON_ONCE(!kc || !kc->timer_set64)) {
>  		error = -EINVAL;
>  	} else {
> -		if (kc->timer_set64) {
> -			new_spec64 = itimerspec_to_itimerspec64(new_spec);
> -			error = kc->timer_set64(timr, flags, &new_spec64,
> -						&old_spec64);
> -			if (old_setting)
> -				old_spec = itimerspec64_to_itimerspec(old_spec64);
> -		} else {
> -			error = kc->timer_set(timr, flags, &new_spec, rtn);
> -		}
> +		new_spec64 = itimerspec_to_itimerspec64(new_spec);
> +		error = kc->timer_set64(timr, flags, &new_spec64,
> +					&old_spec64);

This statement can fit on one line.

> +		if (old_setting)
> +			old_spec = itimerspec64_to_itimerspec(old_spec64);
>  	}
>  
>  	unlock_timer(timr, flag);

> @@ -1057,14 +1045,13 @@ SYSCALL_DEFINE2(clock_gettime, const clockid_t, which_clock,
>  	if (!kc)
>  		return -EINVAL;
>  
> -	if (kc->clock_get64) {
> -		error = kc->clock_get64(which_clock, &kernel_tp64);
> -		kernel_tp = timespec64_to_timespec(kernel_tp64);
> -	} else {
> -		error = kc->clock_get(which_clock, &kernel_tp);
> -	}
> +	error = kc->clock_get64(which_clock, &kernel_tp64);
> +	if (!error)
> +		return error;

Wrong test, should be: if (error) ...

> +
> +	kernel_tp = timespec64_to_timespec(kernel_tp64);
>  
> -	if (!error && copy_to_user(tp, &kernel_tp, sizeof (kernel_tp)))

The (!error && ...) was correct here!

> +	if (copy_to_user(tp, &kernel_tp, sizeof (kernel_tp)))
>  		error = -EFAULT;
>  
>  	return error;

You can simplify this like so:

	return copy_to_user(tp, &kernel_tp, sizeof(kernel_tp)) ? -EFAULT : 0;

> @@ -1104,14 +1091,13 @@ SYSCALL_DEFINE2(clock_getres, const clockid_t, which_clock,
>  	if (!kc)
>  		return -EINVAL;
>  
> -	if (kc->clock_getres64) {
> -		error = kc->clock_getres64(which_clock, &rtn_tp64);
> -		rtn_tp = timespec64_to_timespec(rtn_tp64);
> -	} else {
> -		error = kc->clock_getres(which_clock, &rtn_tp);
> -	}
> +	error = kc->clock_getres64(which_clock, &rtn_tp64);
> +	if (!error)
> +		return error;

Also wrong.

> +
> +	rtn_tp = timespec64_to_timespec(rtn_tp64);
>  
> -	if (!error && tp && copy_to_user(tp, &rtn_tp, sizeof (rtn_tp)))
> +	if (tp && copy_to_user(tp, &rtn_tp, sizeof (rtn_tp)))
>  		error = -EFAULT;
>  
>  	return error;
> -- 
> 1.7.9.5
> 

Thanks,
Richard
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html