linux-kernel - Re: PROBLEM: Kernel BUG in mfgpt_tick (cs5535-clockevt.c) on ALIX 2c3

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, 12 Oct 2017 17:29:23 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Daniel Lezcano <daniel.lezcano@...aro.org>
cc:     David Kozub <zub@...ux.fjfi.cvut.cz>, linux-kernel@...r.kernel.org
Subject: Re: PROBLEM: Kernel BUG in mfgpt_tick (cs5535-clockevt.c) on ALIX
 2c3 - null call

On Thu, 12 Oct 2017, Daniel Lezcano wrote:

> On 11/10/2017 22:48, David Kozub wrote:
> 
> [ ... ]
> 
> >>
> >> +       disable_timer(timer);
> >> +       cs5535_mfgpt_write(timer, MFGPT_REG_COUNTER, 0);
> >> +
> >>        /* Set up the IRQ on the MFGPT side */
> >>        if (cs5535_mfgpt_setup_irq(timer, MFGPT_CMP2, &timer_irq)) {
> >>                printk(KERN_ERR DRV_NAME ": Could not set up IRQ %d\n",
> > 
> > I tried that and the handler is still called. So I did some more random
> > experiments and I found out that if I call disable_timer(timer) twice,
> > then the issue is resolved (the handler is not called before the
> > registration is finished.) And I don't have to set MFGPT_REG_COUNTER to 0.
> 
> Aha! we are close to a fix.
> 
> > I have no idea why do I have to call disable_timer twice.
> 
> For testing purpose, can you try by adding mmiowb() and/or wmb() after
> disable_timer()?

The real question is why

        /* Set the clock scale and enable the event mode for CMP2 */
        val = MFGPT_SCALE | (3 << 8);

        cs5535_mfgpt_write(cs5535_event_clock, MFGPT_REG_SETUP, val);

is in the setup code at all.

The obvious place for this is in mfgpt_set_periodic() which gets called
when the clock event and the handler is set up in the core code. Up to that
point the interrupt handler is protected against shared interrupts via the
is_shutdown() check.

Thanks,

	tglx