linux-kernel - Re: [tip:irq/urgent] genirq: Make legacy autoprobing work again

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, 1 Feb 2018 09:15:12 +0100
From:   Geert Uytterhoeven <geert@...ux-m68k.org>
To:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "H. Peter Anvin" <hpa@...or.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Meelis Roos <mroos@...ux.ee>, Ingo Molnar <mingo@...nel.org>
Cc:     linux-tip-commits@...r.kernel.org,
        kbuild test robot <fengguang.wu@...el.com>,
        Arnd Bergmann <arnd@...db.de>
Subject: Re: [tip:irq/urgent] genirq: Make legacy autoprobing work again

On Wed, Jan 31, 2018 at 10:54 AM, tip-bot for Thomas Gleixner
<tipbot@...or.com> wrote:
> Commit-ID:  55595980acc3232b018ba30df8ee6e0ac40ad184
> Gitweb:     https://git.kernel.org/tip/55595980acc3232b018ba30df8ee6e0ac40ad184
> Author:     Thomas Gleixner <tglx@...utronix.de>
> AuthorDate: Tue, 30 Jan 2018 19:36:32 +0100
> Committer:  Thomas Gleixner <tglx@...utronix.de>
> CommitDate: Wed, 31 Jan 2018 10:52:06 +0100
>
> genirq: Make legacy autoprobing work again
>
> Meelis reported the following warning on a quad P3 HP NetServer museum piece:
>
> WARNING: CPU: 3 PID: 258 at kernel/irq/chip.c:244 __irq_startup+0x80/0x100
> EIP: __irq_startup+0x80/0x100
> irq_startup+0x7e/0x170
> probe_irq_on+0x128/0x2b0
> parport_irq_probe.constprop.18+0x8d/0x1af [parport_pc]
> parport_pc_probe_port+0xf11/0x1260 [parport_pc]
> parport_pc_init+0x78a/0xf10 [parport_pc]
> parport_parse_param.constprop.16+0xf0/0xf0 [parport_pc]
> do_one_initcall+0x45/0x1e0
>
> This is caused by the rewrite of the irq activation/startup sequence which
> missed to convert a callsite in the irq legacy auto probing code.
>
> To fix this irq_activate_and_startup() needs to gain a return value so the
> pending logic can work proper.
>
> Fixes: c942cee46bba ("genirq: Separate activation and startup")
> Reported-by: Meelis Roos <mroos@...ux.ee>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> Tested-by: Meelis Roos <mroos@...ux.ee>
> Cc: stable@...r.kernel.org
> Link: https://lkml.kernel.org/r/alpine.DEB.2.20.1801301935410.1797@nanos

> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -294,11 +294,11 @@ int irq_activate(struct irq_desc *desc)
>         return 0;
>  }
>
> -void irq_activate_and_startup(struct irq_desc *desc, bool resend)
> +int irq_activate_and_startup(struct irq_desc *desc, bool resend)
>  {
>         if (WARN_ON(irq_activate(desc)))
>                 return;

As reported by 0day, this should return something.

if CONFIG_IRQ_DOMAIN=n or CONFIG_IRQ_DOMAIN_HIERARCHY=n,
irq_activate() will never return 1, and the code path can be eliminated.

BTW, is gcc becoming too smart, and already eliminating code before it
generates warnings for it? Recently I've seen some other cases where buggy
code was not warned about, as it was considered not having any impact.

> -       irq_startup(desc, resend, IRQ_START_FORCE);
> +       return irq_startup(desc, resend, IRQ_START_FORCE);
>  }

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds