linux-kernel - Re: [RFC PATCH v2 01/11] time: Add do_settimeofday64() safe version(using timespec64)

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]

Date:	Thu, 30 Oct 2014 14:24:41 +0100 (CET)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	"pang.xunlei" <pang.xunlei@...aro.org>
cc:	linux-kernel@...r.kernel.org, rtc-linux@...glegroups.com,
	xen-devel@...ts.xenproject.org,
	John Stultz <john.stultz@...aro.org>,
	Alessandro Zummo <a.zummo@...ertech.it>,
	Stefano Stabellini <stefano.stabellini@...citrix.com>
Subject: Re: [RFC PATCH v2 01/11] time: Add do_settimeofday64() safe
 version(using timespec64)

On Thu, 30 Oct 2014, pang.xunlei wrote:

Your subject line

> time: Add do_settimeofday64() safe version(using timespec64)

is confusing at best.

time: Provide y2038 safe do_settimeofday() replacement

That explains what the patch is doing in a very precice way.

>  /**
> - * do_settimeofday - Sets the time of day
> - * @tv:		pointer to the timespec variable containing the new time
> + * do_settimeofday64 - Sets the time of day
> + * @ts:     pointer to the timespec64 variable containing the new time
>   *
>   * Sets the time of day to the new time and update NTP and notify hrtimers
> + * Safe version for 2038 safety.

I rather put this into the function description above:

+ * do_settimeofday64 - Sets the time of day. 2038 safe.

Way simpler to grep for.

>   */
> -int do_settimeofday(const struct timespec *tv)
> +int do_settimeofday64(const struct timespec64 *ts)
>  {
>  	struct timekeeper *tk = &tk_core.timekeeper;
> -	struct timespec64 ts_delta, xt, tmp;
> +	struct timespec64 ts_delta, xt;
>  	unsigned long flags;
>  
> -	if (!timespec_valid_strict(tv))
> +	if (!timespec64_valid_strict(ts))
>  		return -EINVAL;
>  
>  	raw_spin_lock_irqsave(&timekeeper_lock, flags);
> @@ -723,13 +724,12 @@ int do_settimeofday(const struct timespec *tv)
>  	timekeeping_forward_now(tk);
>  
>  	xt = tk_xtime(tk);
> -	ts_delta.tv_sec = tv->tv_sec - xt.tv_sec;
> -	ts_delta.tv_nsec = tv->tv_nsec - xt.tv_nsec;
> +	ts_delta.tv_sec = ts->tv_sec - xt.tv_sec;
> +	ts_delta.tv_nsec = ts->tv_nsec - xt.tv_nsec;
>  
>  	tk_set_wall_to_mono(tk, timespec64_sub(tk->wall_to_monotonic, ts_delta));
>  
> -	tmp = timespec_to_timespec64(*tv);
> -	tk_set_xtime(tk, &tmp);
> +	tk_set_xtime(tk, ts);
>  
>  	timekeeping_update(tk, TK_CLEAR_NTP | TK_MIRROR | TK_CLOCK_WAS_SET);
>  
> @@ -741,6 +741,22 @@ int do_settimeofday(const struct timespec *tv)
>  
>  	return 0;
>  }
> +EXPORT_SYMBOL(do_settimeofday64);

EXPORT_SYMBOL_GPL please.

> +
> +/**
> + * do_settimeofday - Sets the time of day
> + * @tv:		pointer to the timespec variable containing the new time
> + *
> + * Sets the time of day to the new time and update NTP and notify hrtimers
> + * TODO: [2038 safety] should be replaced by do_settimeofday64().

The TODO is not to replace do_settimeofday() by do_settimeofday64(). The TODO
is to fix the call sites.

So instead of adding this you can simply say:

+ * do_settimeofday - Sets the time of day. Deprecated. Use do_settimeofday64()

We want things like this easy to find and not be burried in some
cryptic comment.

Thanks,

	tglx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/