linux-kernel - Re: [GIT pull] irq updates for 4.13

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]

Message-ID: <alpine.DEB.2.20.1707110815440.1799@nanos>
Date:   Tue, 11 Jul 2017 08:55:58 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
cc:     Sebastian Reichel <sebastian.reichel@...labora.co.uk>,
        LKML <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Ingo Molnar <mingo@...nel.org>,
        "H. Peter Anvin" <hpa@...or.com>, Tony Lindgren <tony@...mide.com>,
        Pavel Machek <pavel@....cz>
Subject: Re: [GIT pull] irq updates for 4.13

On Mon, 10 Jul 2017, Linus Torvalds wrote:
> On Mon, Jul 10, 2017 at 6:35 AM, Sebastian Reichel
> <sebastian.reichel@...labora.co.uk> wrote:
> >
> > This patch apparently breaks OMAP platform:
> >
> > 46e48e257360f0845fe17089713cbad4db611e70 is the first bad commit
> > commit 46e48e257360f0845fe17089713cbad4db611e70
> > Author: Thomas Gleixner <tglx@...utronix.de>
> > Date:   Thu Jun 29 23:33:38 2017 +0200
> >
> >     genirq: Move irq resource handling out of spinlocked region
> >
> > Boot failure log from Droid 4:
> > [ ... snip snip ..]
> >
> > Droid 4 boots current master again after applying the patch below
> > (which is git revet of above patch, but I provide the patch, since
> > it did not revet cleanly).
> 
> Hmm. Do you actually need the full revert?
> 
> I think it's only the __setup_irq() part that looks like it may be garbage.
> 
> For example, I think it releases the resources twice if the
> __irq_set_trigger() call fails.

Yes, I missed that. Sorry.

> But it looks questionably in other ways too - notably, the change to
> make the request call be in the same context as the freeing is done is
> apparently done entirely for symmetry reasons, not for any actual
> *reason* reasons.

There is a reasons reason. The whole purpose was to move out the
request/free resources call from the spinlocked and irq disabled reason.
I noticed the free ordering issue, when I was working on that.

The fact that the patch breaks the OMAP boot, points to something else.

The only user of the irq_request_resources() callback at the moment is the
GPIO subsystem and some pinctrl drivers, which are not involved in the OMAP
case. In case of OMAP it uses the gpiolib generic implementation which
does:

      try_module_get(chip->gpiodev->owner);
      gpiochip_lock_as_irq(chip, d->hwirq);

I have no idea at the moment why this would break anything. The double
release in the __irq_set_trigger() error path is the only issue I can find
there.

Sebastian, can you please provide a .config and a full boot log, preferably
with initcall_debug on the kernel command line?

Thanks,

	tglx