linux-kernel - Re: [PATCH] let setup_irq reenable a shared irq

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.LFD.1.10.0804281443090.3261@apollo.tec.linutronix.de>
Date:	Mon, 28 Apr 2008 15:03:29 +0200 (CEST)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Uwe Kleine-König <Uwe.Kleine-Koenig@...i.com>
cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>
Subject: Re: [PATCH] let setup_irq reenable a shared irq

On Mon, 28 Apr 2008, Uwe Kleine-König wrote:
> Consider two devices A and B sharing an irq and B already asserts the irq on
> a booting machine.
> If the driver for A is loaded first the irq starts triggering and gets
> disabled after some time by note_interrupt().  Later when the driver for B
> is loaded the interrupt should be reenabled---other wise both A and B don't
> work properly.

Oh no. There is lots of code in drivers, which does:

   disable_irq();
   do_some_protected_stuff();
   enable_irq();

So when the second driver is loaded on another CPU it would see the
IRQ_DISABLED bit set and unconditionally reenable the interrupt. 

This unprotects the protected operation and definitely triggers the
WARN_ON() in enable_irq() where we check for desc->depth == 0.

I can see the rationale for your patch, as we have no way to silence
stupid hardware or hardware which was left in that state by the BIOS
other than disabling the interrupt line completely.

Waht kind of scenario/devices do you have which trigger this ?

Thanks,
	tglx

> Signed-off-by: Uwe Kleine-König <Uwe.Kleine-Koenig@...i.com>
> Cc: Ingo Molnar <mingo@...e.hu>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> ---
>  kernel/irq/manage.c |    9 ++++++++-
>  1 files changed, 8 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 438a014..831ebb2 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -365,11 +365,18 @@ int setup_irq(unsigned int irq, struct irqaction *new)
>  
>  		desc->status &= ~(IRQ_AUTODETECT | IRQ_WAITING |
>  				  IRQ_INPROGRESS);
> +	}
> +
> +	/* in the shared case the interrupt might be disabled if a device
> +	 * asserts the irq but the corresponding driver wasn't the first to be
> +	 * loaded.  So retry enabling the irq.
> +	 */
> +	if (!shared || desc->status & IRQ_DISABLED) {
>  
>  		if (!(desc->status & IRQ_NOAUTOEN)) {
>  			desc->depth = 0;
>  			desc->status &= ~IRQ_DISABLED;
> -			if (desc->chip->startup)
> +			if (!shared && desc->chip->startup)
>  				desc->chip->startup(irq);
>  			else
>  				desc->chip->enable(irq);
> -- 
> 1.5.5.1
>