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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 12 Oct 2017 22:53:09 +0200 (CEST)
From:   David Kozub <zub@...ux.fjfi.cvut.cz>
To:     Thomas Gleixner <tglx@...utronix.de>,
        Daniel Lezcano <daniel.lezcano@...aro.org>
cc:     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, Thomas Gleixner wrote:

> 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()?

I tried one, the other, both. Still, the unexpected handler call happens. 
I (again) added a msleep(1000) after the disable (and the barriers). If 
the trouble was a missing barrier, the sleep would most likely also make 
the isaue go away (even if in a hacky way).

> 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.

It is, but only after clockevents_config_and_register. But mfgpt_tick is 
called before that (just after setup_irq).

Best regards,
David