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  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:   Thu, 21 Mar 2019 10:26:26 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Stephen Boyd <swboyd@...omium.org>
cc:     linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-gpio@...r.kernel.org, Lina Iyer <ilina@...eaurora.org>,
        Marc Zyngier <marc.zyngier@....com>
Subject: Re: [PATCH] genirq: Respect IRQCHIP_SKIP_SET_WAKE in
 irq_chip_set_wake_parent()

On Fri, 15 Mar 2019, Stephen Boyd wrote:

> This function returns an error if a child interrupt controller calls
> irq_chip_set_wake_parent() but that parent interrupt controller has the
> IRQCHIP_SKIP_SET_WAKE flag. Let's return 0 for success instead because
> there isn't anything to do.
> 
> There's also the possibility that a parent indicates that we should skip
> it, but the grandparent has an .irq_set_wake callback. Let's iterate
> through the parent chain as long as the IRQCHIP_SKIP_SET_WAKE flag isn't
> set so we can find the first parent that needs to handle the wake
> configuration. This fixes a problem on my Qualcomm sdm845 device where
> I'm trying to enable wake on an irq from the gpio controller that's a
> child of the qcom pdc interrupt controller. The qcom pdc interrupt
> controller has the IRQCHIP_SKIP_SET_WAKE flag set, and so does the
> grandparent (ARM GIC), causing this function to return a failure because
> the parent controller doesn't have the .irq_set_wake callback set.

It took me some time to distangle that changelog.... and I don't think that
this is the right thing to do.

set_irq_wake_real() returns 0 when the topmost chip has SKIP_SET_WAKE set.

So let's assume we have the following chains:

  chip A -> chip B 

  chip A -> chip B -> chip C

chip A has SKIP_SET_WAKE not set
chip B has SKIP_SET_WAKE set
chip C has SKIP_SET_WAKE not set and invokes irq_chip_set_wake_parent()

Now assume we have interrupt X connected to chip B and interrupt Y
connected to chip C.

If irq_set_wake() is called for interrupt X, then the function returns
without trying to invoke the set_wake() callback of chip A.

If irq_set_wake() is called for interrupt Y, irq_chip_set_wake_parent() is
invoked from chip C which then skips chip B, but tries to invoke the
callback on chip A.

That's inconsistent and changes the existing behaviour. So IMO, the right
thing to do is to return 0 from irq_chip_set_wake_parent() when the parent
has SKIP_SET_WAKE set and not to try to follow the whole chain. That should
fix your problem nicely w/o changing behaviour.

Thanks,

	tglx

----
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index 3faef4a77f71..51128bea3846 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -1449,6 +1449,10 @@ int irq_chip_set_vcpu_affinity_parent(struct irq_data *data, void *vcpu_info)
 int irq_chip_set_wake_parent(struct irq_data *data, unsigned int on)
 {
 	data = data->parent_data;
+
+	if (data->chip->flags & IRQCHIP_SKIP_SET_WAKE)
+		return 0;
+
 	if (data->chip->irq_set_wake)
 		return data->chip->irq_set_wake(data, on);
 


Powered by blists - more mailing lists