linux-kernel - Re: [PATCH][RFC v4] timekeeping: ignore the bogus sleep time if pm

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]

Date:   Sun, 28 Aug 2016 16:28:10 +0800
From:   Chen Yu <yu.c.chen@...el.com>
To:     xlpang@...hat.com
Cc:     linux-pm@...r.kernel.org, Ingo Molnar <mingo@...hat.com>,
        "H . Peter Anvin" <hpa@...or.com>, x86@...nel.org,
        "Rafael J . Wysocki" <rjw@...ysocki.net>,
        John Stultz <john.stultz@...aro.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Zhang Rui <rui.zhang@...el.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH][RFC v4] timekeeping: ignore the bogus sleep time if
 pm_trace is enabled

On Sat, Aug 27, 2016 at 03:08:56PM +0800, Xunlei Pang wrote:
> On 2016/08/18 at 18:43, Chen Yu wrote:
> > Previously we encountered some memory overflow issues due to
> > the bogus sleep time brought by inconsistent rtc, which is
> > triggered when pm_trace is enabled, please refer to:
> > https://patchwork.kernel.org/patch/9286365/
> > It's improper in the first place to call __timekeeping_inject_sleeptime()
> > in case that pm_trace is enabled simply because that "hash" time value
> > will wreckage the timekeeping subsystem.
> >
> > So this patch ignores the sleep time if pm_trace is enabled in
> > the following situation:
> > 1. rtc is used as persist clock to compensate for sleep time,
> >    (because system does not have a nonstop clocksource) or
> > 2. rtc is used to calculate the sleep time in rtc_resume.
> >
> > Cc: Rafael J. Wysocki <rjw@...ysocki.net>
> > Cc: John Stultz <john.stultz@...aro.org>
> > Cc: Thomas Gleixner <tglx@...utronix.de>
> > Cc: Xunlei Pang <xlpang@...hat.com>
> > Cc: Zhang Rui <rui.zhang@...el.com>
> > Cc: linux-kernel@...r.kernel.org
> > Cc: linux-pm@...r.kernel.org
> > Suggested-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> > Reported-by: Janek Kozicki <cosurgi@...il.com>
> > Signed-off-by: Chen Yu <yu.c.chen@...el.com>
> > ---
> I suddenly think of a way to avoid adding this ugly __weak auxiliary function.
> 
> Add a special treatment for read_persistent_clock() in arch/x86/kernel/rtc.c as follows,
> void read_persistent_clock(struct timespec *ts)
> {
>     x86_platform.get_wallclock(ts);
> 
>     /* Make rtc-based persistent clock unusable if pm_trace is enabled. */
>     if (pm_trace_is_enabled() &&
>         x86_platform.get_wallclock == mach_get_cmos_time) {
>         ts->tv_sec = 0;
>         ts->tv_nsec = 0;
> 
> In this way, we can avoid the touch of timekeeping core, after all ptrace is currently x86-specific.
> 
> What do you think?
>
OK, I have another question, if we do like this, as read_persistent_clock64
is invoked in timekeeping_suspend/timekeeping_resume/timekeeping_init,
then for timekeeping_init case, if pm_trace is enabled by command line,
we will never use rtc even if we do not suspend?