linux-kernel - Re: [RFC] genirq: Flush the irq thread on synchronization

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:	Sat, 3 Dec 2011 00:21:46 +0100 (CET)
From:	Thomas Gleixner <tglx@...utronix.de>
To:	Ido Yariv <ido@...ery.com>
cc:	linux-kernel@...r.kernel.org
Subject: Re: [RFC] genirq: Flush the irq thread on synchronization

On Fri, 2 Dec 2011, Ido Yariv wrote:

> The current implementation does not always flush the threaded handler
> when disabling the irq. In case the irq handler was called, but the
> threaded handler hasn't started running yet, the interrupt will be
> flagged as pending, and the handler will not run. This implementation
> has some issues:
> 
> First, if the interrupt is a wake source and flagged as pending, the
> system will not be able to suspend.
> 
> Second, when quickly disabling and re-enabling the irq, the threaded
> handler might continue to run after the irq is re-enabled without the
> irq handler being called first. This might be an unexpected behavior.

I'd wish people would stop calling disable/enable_irq() in loops and
circles for no reason.

> In addition, it might be counter-intuitive that the threaded handler
> will not be called even though the irq handler was called and returned
> IRQ_WAKE_THREAD.
> 
> Fix this by always waiting for the threaded handler to complete in
> synchronize_irq().

I can see your problem, but this might lead to threads_active leaks
under certain conditions. desc->threads_active was only meant to deal
with shared interrupts.

We explicitely allow a design where the primary handler can leave the
device interrupt enabled and allow further interrupts to occur while
the handler is running. We only have a single bit to note that the
thread should run, but your wakeup would up the threads_active count
in that scenario several times w/o a counterpart which decrements it.

The solution for this is to keep the current threads_active semantics
and make the wait function different. Instead of waiting for
threads_active to become 0 it should wait for threads_active == 0 and
the IRQTF_RUNTHREAD for all actions to be cleared. To avoid looping
over the actions, we can take a similar approach as we take with the
desc->threads_oneshot bitfield.

Thanks,

	tglx

--
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/