linux-kernel - Re: Kernel Bug? Use of IRQF_SHARED + IRQF

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] [day] [month] [year] [list]

Message-Id: <20080213003547.e8113b9a.akpm@linux-foundation.org>
Date:	Wed, 13 Feb 2008 00:35:47 -0800
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	"Rajat Jain" <rajat.noida.india@...il.com>
Cc:	linux-kernel@...r.kernel.org, Alan Cox <alan@...rguk.ukuu.org.uk>
Subject: Re: Kernel Bug? Use of IRQF_SHARED + IRQF_DISABLED

On Tue, 12 Feb 2008 11:19:03 +0530 "Rajat Jain" <rajat.noida.india@...il.com> wrote:

> Hi,
> 
> Based on suggestion from Thomas Petazzoni, I'm moving this to LKML.
> 
> This is regarding the following code in kernel/irq/handle.c. Consider
> the case of a shared IRQ line, where two handlers are registered such
> that first handler does not specify IRQF_DISABLED, but the second one
> does. But it seems both the handlers will be called with IRQs ENABLED
> (which is certainly not what the second handler expects).
> 
> I also checked but could not find anything that stops me from
> registering two shared ISRs - one with IRQF_DISABLED & another without
> this flag. Am I missing something here?
> 
> irqreturn_t handle_IRQ_event(unsigned int irq, struct irqaction *action)
> {
>    irqreturn_t ret, retval = IRQ_NONE;
>    unsigned int status = 0;
> 
>    handle_dynamic_tick(action);
> 
>    if (!(action->flags & IRQF_DISABLED))
>        local_irq_enable_in_hardirq();
> 
>    do {
>        ret = action->handler(irq, action->dev_id);
>        if (ret == IRQ_HANDLED)
>            status |= action->flags;
>        retval |= ret;
>        action = action->next;
>    } while (action);
> 
>    if (status & IRQF_SAMPLE_RANDOM)
>        add_interrupt_randomness(irq);
>    local_irq_disable();
> 
>    return retval;
> }
> 
> I'd like to submit a patch but was wondering which of the following is
> the correct startegy to deal with above situation (I personally think
> (1) below is more appropriate):
> 
> 1) IN the above code while calling shared ISRs, check for each ISR
> whether it specified IRQF_DISABLED or not. Enable IRQs only for ISR
> that did not specify IRQF_DISABLED.
> 
> 2) While installing ISR, check that all the ISRs for that IRQ should
> have consistent use of IRQF_DISABLED. Don't allow insonsistent use of
> IRQF_DISABLED on a shared IRQ.

Well.. the question is "what are the semantics of IRQF_DISABLED?".

If it is "interrupts should be disabled while calling the ->action then
fine, 1) is good.

But that sounds fairly pointless - the handler can do local_irq_disable()
if it wants.  I guess IRQF_DISABLED was deswigned to hold off interrupts
for the entire duration of the hard IRQ - I don't really recall.  Alan
might though?

I'm pretty sure this came up within the last year, and it looks like we
decided to leave the code in the state which you see there.  I wonder why. 
How god is your googling?

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/