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:   Fri, 12 May 2017 10:40:53 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Gregory Fong <gregory.0xf0@...il.com>
cc:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        linux@...linux.org.uk, Florian Fainelli <f.fainelli@...il.com>
Subject: Re: handle_bad_irq and locking

On Thu, 11 May 2017, Gregory Fong wrote:
> Hi Thomas,
> 
> I noticed that when you changed arm irq handling to use the generic
> implementation back in 2006 that you changed do_bad_IRQ() to the
> following:
> 
> +#define do_bad_IRQ(irq,desc,regs)                      \
> +do {                                                   \
> +       spin_lock(&desc->lock);                         \
> +       handle_bad_irq(irq, desc, regs);                \
> +       spin_unlock(&desc->lock);                       \
> +} while(0)
> 
> and it's mostly stayed the same since then.  As such, there are a few
> examples of this being open-coded in the kernel in various irqchip
> handlers such as that of drivers/irqchip/irq-brcmstb-l2.c, and even
> more cases where the lock isn't being grabbed before calling
> handle_bad_irq().
> 
> Since handle_bad_irq() is sort of a flow handler like
> handle_edge_irq() etc., do you think it would make sense to do the
> same as those and have it do the locking on its own?  A quick look
> through existing users of handle_bad_irq() at [1] suggests that either
> the locking isn't necessary or it's almost always done wrong.

Right. So the proper solution to this is to create a generic function

       handle_bad_irq_desc()

have proper locking in it and move all users (including do_bad_IRQ()) over
to it.

Thanks,

	tglx

Powered by blists - more mailing lists