linux-kernel - regression in irq sharing caused by genirq: Use irqd_get_trigger_type to compare the trigger type for shared IRQs

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

Date:   Wed, 8 Nov 2017 00:41:13 +0100
From:   Petr Cvek <petrcvekcz@...il.com>
To:     hdegoede@...hat.com, tglx@...utronix.de, marc.zyngier@....com,
        tglx@...utronix.de
Cc:     linux-kernel@...r.kernel.org, philipp.zabel@...il.com
Subject: regression in irq sharing caused by genirq: Use irqd_get_trigger_type
 to compare the trigger type for shared IRQs

Hello,

Commit 382bd4de61827 ("genirq: Use irqd_get_trigger_type to compare the 
trigger type for shared IRQs") causes a regression for pda-power driver 
and Magician machine (mach-pxa/magician.c).

   unsigned int oldtype = irqd_get_trigger_type(&desc->irq_data);

   ... assert:
	oldtype == 0	//new code
	old->flags == 0x83	//old code
	new->flags & IRQF_TRIGGER_MASK == 3

   if (!((old->flags & new->flags) & IRQF_SHARED) ||
	(oldtype != (new->flags & IRQF_TRIGGER_MASK)) ||
	((old->flags ^ new->flags) & IRQF_ONESHOT))
		goto mismatch;

The assert shows the new code will trigger the jump for "mismatch" error 
the old variant of code works fine.

The case for Magician machine is specific as the same interrupt line is 
requested twice from the same driver (pda-power). But it still could 
point to some hidden problem in the IRQ setup code.

I wasn't able to trace the code when desc->irq_data is getting set. The 
flags values should be (as with old->flags):

	IRQF_SHARED | IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING

It could be a problem with a weird IRQ sharing in magician code, but 
it's still failing the driver and the start of the charging system.

IRQ definition in arch/arm/mach-pxa/magician.c looks like this:

static struct resource power_supply_resources[] = {
	[0] = {
		.name	= "ac",
		.flags	= IORESOURCE_IRQ | IORESOURCE_IRQ_HIGHEDGE |
			IORESOURCE_IRQ_LOWEDGE,
		.start	= IRQ_MAGICIAN_VBUS,
		.end	= IRQ_MAGICIAN_VBUS,
	},
	[1] = {
		.name	= "usb",
		.flags	= IORESOURCE_IRQ | IORESOURCE_IRQ_HIGHEDGE |
			IORESOURCE_IRQ_LOWEDGE,
		.start	= IRQ_MAGICIAN_VBUS,
		.end	= IRQ_MAGICIAN_VBUS,
	},
};

and IRQ requests from drivers/power/supply/pda_power.c look like this:

if (ac_irq) {
	ret = request_irq(ac_irq->start, power_changed_isr,
			  get_irq_flags(ac_irq), ac_irq->name,
			  pda_psy_ac);
...
if (usb_irq) {
	ret = request_irq(usb_irq->start, power_changed_isr,
			  get_irq_flags(usb_irq),
			  usb_irq->name, pda_psy_usb);

I could rewrite the part in the magician code so it would use only one 
interrupt, but it doesn't solve why oldtype == 0 problem.

Best regards,
Petr