linux-kernel - Re: [PATCH] time: fix typo that makes sync_cmos

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] [day] [month] [year] [list]

Message-ID: <alpine.LFD.0.9999.0711140850210.3265@localhost.localdomain>
Date:	Wed, 14 Nov 2007 08:57:10 +0100 (CET)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	"David P. Reed" <dpreed@...d.com>
cc:	linux-kernel@...r.kernel.org,
	Allessandro Zummo <a.zummo@...ertech.it>
Subject: Re: [PATCH] time: fix typo that makes sync_cmos_clock erratic

David,

On Mon, 12 Nov 2007, David P. Reed wrote:

> From: David P. Reed
> 
> Fix a typo in ntp.c that has caused updating of the persistent (RTC) clock
> when synced to NTP to behave erratically.

Good catch.

> Signed-off-by: David P. Reed

Whitespace damaged as well. Please fix and resend

> ---
> When debugging a freeze that arises on my AMD64 machines when I run the 
> ntpd service, I added a number of printk's to monitor the sync_cmos_clock
> procedure.  I discovered that it was not syncing to cmos RTC every 11 minutes
> as documented, but instead would keep trying every second for hours at a time.
> The reason turned out to be a typo in sync_cmos_clock, where it attempts to
> ensure that update_persistent_clock is called very close to 500 msec. after a
> 1 second boundary (required by the PC RTC's spec). That typo referred to
> "xtime" in one spot, rather than "now", which is derived from "xtime" but not
> equal to it.  This makes the test erratic, creating a "coin-flip" that decides
> when update_persistent_clock is called - when it is called, which is rarely,
> it may be at any time during the one second period, rather than close to 500
> msec, so the value written is needlessly incorrect, too.

Please put the explanation into the changelog.

> The patch works in 2.6.24rc2, but it should also work in 2.6.23.

Yup
 
> @@ -205,7 +206,10 @@ static void sync_cmos_clock(unsigned lon
>         return;
> 
>     getnstimeofday(&now);
> -    if (abs(xtime.tv_nsec - (NSEC_PER_SEC / 2)) <= tick_nsec / 2)

Can we just s/xtime/now/ ? There is no value in an extra variable.

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/