linux-kernel - Re: [PATCH RFC v2] PM / sleep: Fix racing timers

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:	Mon, 28 Jul 2014 13:02:25 -0700
From:	Sören Brinkmann <soren.brinkmann@...inx.com>
To:	Stephen Boyd <sboyd@...eaurora.org>
CC:	John Stultz <john.stultz@...aro.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Pavel Machek <pavel@....cz>, Len Brown <len.brown@...el.com>,
	<linux-kernel@...r.kernel.org>, <linux-pm@...r.kernel.org>,
	Daniel Lezcano <daniel.lezcano@...aro.org>
Subject: Re: [PATCH RFC v2] PM / sleep: Fix racing timers

On Mon, 2014-07-28 at 12:38PM -0700, Stephen Boyd wrote:
> On 07/28/14 12:24, John Stultz wrote:
> > On 07/28/2014 12:19 PM, Stephen Boyd wrote:
> >> On 07/25/14 14:06, Soren Brinkmann wrote:
> >>> On platforms that do not power off during suspend, successfully entering
> >>> suspend races with timers.
> >>>
> >>> The race happening in a couple of location is:
> >>>
> >>>   1. disable IRQs   		(e.g. arch_suspend_disable_irqs())
> >>>      ...
> >>>   2. syscore_suspend()
> >>>       -> timekeeping_suspend()
> >>>        -> clockevents_notify(SUSPEND)
> >>>         -> tick_suspend()   	(timers are turned off here)
> >>>      ...
> >>>   3. wfi            		(wait for wake-IRQ here)
> >>>
> >>> Between steps 1 and 2 the timers can still generate interrupts that are
> >>> not handled and stay pending until step 3. That pending IRQ causes an
> >>> immediate - spurious - wake.
> >>>
> >>> The solution is to move the clockevents suspend/resume notification
> >>> out of the syscore_suspend step and explictly call them at the appropriate
> >>> time in the suspend/hibernation paths. I.e. timers are suspend _before_
> >>> IRQs get disabled. And accordingly in the resume path.
> >>>
> >>> Signed-off-by: Soren Brinkmann <soren.brinkmann@...inx.com>
> >> Do we know which timer_list or hrtimer wants to run while entering
> >> suspend? I'd suspect the scheduler tick but perhaps we just forgot to
> >> cancel some timer during suspend?
> > Though, canceling timers really shouldn't be necessary for
> > suspend/resume, no?
> >
> >
> 
> Agreed. Perhaps I put it the wrong way. I'm worried that some timer
> needs to run just when we go into suspend. As long as that timer is the
> scheduler tick we should be ok, but if it isn't the scheduler tick then
> it would be good to know what it is and why it's pending. Unless the
> idea is that if we get this far into suspend and there's a pending timer
> we should just ignore it and go to sleep anyway?

Well, that is pretty much what happens currently. The IRQs are disabled
and nobody cares about the pending timer. My problem with that is, that
"suspend" for Zynq is just waiting in WFI. Hence, the pending interrupts
causes an immediate resume.
So, it should hopefully be more or less fine since the current
implementation basically ignores the timer. With this patch we just shut
them down a little earlier to prevent this pending interrupt - at least
that is the intention.

	Thanks,
	Sören
--
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/