linux-kernel - re-send: gpio-pca953x interrupt feature unreliable!!

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

Message-Id: <D108B135-F9CF-46C5-9E42-9705DB0B040C@gmail.com>
Date:	Tue, 18 Oct 2011 10:34:43 -0400
From:	Jean-Francois Dagenais <jeff.dagenais@...il.com>
To:	maz@...terjones.org, grant.likely@...retlab.ca
Cc:	open list <linux-kernel@...r.kernel.org>,
	broonie@...nsource.wolfsonmicro.com, haojian.zhuang@...vell.com
Subject: re-send: gpio-pca953x interrupt feature unreliable!!

Does Anyone care?
===============Re-Send==================
Hi all,

(Using 3.1.0-rc4)

I have detected a flaw in the interrupt support of the gpio-pca953x driver. This is how I discovered the issue:

I have a interrupt client device which has it's interrupt signal connected to one of the GPIOs of the pca9555. The client is an ad714x device. It's interrupt routine is falling edge triggered (so says the driver, but according to the AD714x specs, it should be level,low, however this is another topic). The pca9555 is interrupting the CPU through INTA of an ethernet PCI card for which I have only done "echo 1 > /bus/pci/devices/00xyz/enable". It is "(level, low)" which matches the pca953x request_Isr call (IRQF_TRIGGER_LOW | IRQF_ONESHOT). I used two GPOs on some other device to get the !ISR signals. These show the nested nature of the cli ISR called from the pca953x's ISR.

ASCII art of my probing (use a monospace font)
             ____          _______
client !INT      |________|       |________(lost)___
             ___________                   _________
cli !ISR                |____|____________|
             ____      ___         _________________
pca953x !INT     |____|   |_______|
             ______                             ____
pca953x !ISR       |___________________________|

(note: the little spike in the cli !ISR marks the end of reading the client's registers, the rest of the ISR is updating driver state machines, etc.)

Summary: The client enters interrupt, which immediately puts the pca953x in interrupt as well. When the pca953x enters it's isr thread, it reads the input status register, this immediately clears the pca953x interrupt. The pca953x ISR then figure's out it should invoke the nested ISR for the client. The client ISR starts by reading the client's status register, which clears the client interrupt. This immediately puts the pca953x back into interrupt since it's a different state that what was last read in the pca953x. This should't be a problem. The real problem is that the client goes back in interrupt before the client ISR even has time to finish. At that moment, the pca953x's input are back to the way they where when the pca953x ISR began and it read the input status register. When all of the ISR routines are done, the client has it's INT line asserted and no more interrupt will come from the pca953x.

The pca953x specs clearly state this: "Resetting the interrupt circuit is achieved when data on the port is changed to the original setting, data is read from the port that generated the interrupt."

So in essence, although the pca953x's design and doc says only edges are detected and supported, the chip design makes it impossible to guarantee that all edges will be caught. In fact, since the chip only has an input state register and no latching interrupt status register, the driver could be slightly modified to support only level based interrupt clients.

I would propose a patch, but I am missing knowledge on the internals of the kernel's irq code, so here's a description, hoping to get some feedback.  In the pca953x_irq_handler (threaded), just after reading the pca953x's input status register (which clears the pca953x INT), just before calling the nested handlers, we could re-enable the IRQ so that the pca953x_irq_handler would be re-scheduled right after if needed. Now this would always happen since the client's clearing of it's INT constitute a change. This is why a hard IRQ handler should be added to re-disable the interrupt to avoid the storm caused by the level,low interrupt. In addition to all this, the driver should be changed to support only LEVEL based clients, since this is what it can reliably detect.

Any thoughts? Hints on the precise APIs to use?

/jfd

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