linux-kernel - Re: [PATCH v5 2/4] time: Fix a bug in timekeeping

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:	Wed, 18 Mar 2015 10:17:15 -0700
From:	John Stultz <john.stultz@...aro.org>
To:	Xunlei Pang <xlpang@....com>
Cc:	lkml <linux-kernel@...r.kernel.org>,
	"rtc-linux@...glegroups.com" <rtc-linux@...glegroups.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Alessandro Zummo <a.zummo@...ertech.it>,
	Arnd Bergmann <arnd.bergmann@...aro.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Xunlei Pang <pang.xunlei@...aro.org>
Subject: Re: [PATCH v5 2/4] time: Fix a bug in timekeeping_suspend() with no
 persistent clock

On Sun, Mar 8, 2015 at 11:27 PM, Xunlei Pang <xlpang@....com> wrote:
> From: Xunlei Pang <pang.xunlei@...aro.org>
>
> When there's no persistent clock, normally timekeeping_suspend_time
> should always be zero, but this can break in timekeeping_suspend().
>
> At T1, there was a system suspend, so old_delta was assigned T1.
> After some time, one time adjustment happened, and xtime got the
> value of T1-dt(0s<dt<2s). Then, there comes another system suspend
> soon after this adjustment, obviously we will get a small negative
> delta_delta, resulting in a negative timekeeping_suspend_time.
>
> This is problematic, when doing timekeeping_resume() if there is
> no nonstop clocksource for example, it will hit the else leg and
> inject the improper sleeptime which is the wrong logic.
>
> So, we can solve this problem by only doing delta related code when
> the persistent clock is existent. Actually the code only makes sense
> for persistent clock cases.
>
> This patch also improves the name of timekeeping_suspend_time.

So re-reviewing this here. I really think the renaming here isn't an
improvement. It does clarify that its related only to the persistent
clock, but the resulting code is much uglier due to the 80col
reformatting you've done.

Can we drop the rename, which should make the fix in this code more clear?

thanks
-john
--
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/