linux-kernel - [RFC PATCH] irqchip: arm-gic: take gic

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:   Thu, 22 Mar 2018 10:28:59 +0000
From:   Aniruddha Banerjee <aniruddhab@...dia.com>
To:     Marc Zyngier <marc.zyngier@....com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-tegra@...r.kernel.org" <linux-tegra@...r.kernel.org>,
        Jonathan Hunter <jonathanh@...dia.com>,
        Stephen Warren <swarren@...dia.com>,
        Thierry Reding <treding@...dia.com>,
        Vipin Kumar <vipink@...dia.com>
Subject: [RFC PATCH] irqchip: arm-gic: take gic_lock when updating irq type

The kernel documentation states that the irq-chip driver should
handle the locking of the irq-chip registers. In the irq-gic,
the accesses to the irqchip are seemingly not protected and multiple
writes to SPIs from different irq descriptors do RMW requests without
taking the irq-chip lock. When multiple irqs call the request_irq at
the same time, there can be a simultaneous write at the gic
distributor, leading to a race. Acquire the irq_controller lock when the
irq_type is updated.

This patch is only for GICv2; however, I have noticed a similar
implementation in GICv3. This patch is sent as an RFC in case I am
missing anything.

Signed-off-by: Aniruddha Banerjee <aniruddhab@...dia.com>
---
 drivers/irqchip/irq-gic.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/irqchip/irq-gic.c b/drivers/irqchip/irq-gic.c
index 4c797b43614d..61380f5a2254 100644
--- a/drivers/irqchip/irq-gic.c
+++ b/drivers/irqchip/irq-gic.c
@@ -67,6 +67,8 @@ static void gic_check_cpu_features(void)
 #define gic_check_cpu_features()       do { } while(0)
 #endif

+static DEFINE_RAW_SPINLOCK(irq_controller_lock);
+
 union gic_base {
        void __iomem *common_base;
        void __percpu * __iomem *percpu_base;
@@ -529,6 +531,7 @@ static int gic_set_type(struct irq_data *d, unsigned int type)
 {
        void __iomem *base = gic_dist_base(d);
        unsigned int gicirq = gic_irq(d);
+       int ret;

        /* Interrupt configuration for SGIs can't be changed */
        if (gicirq < 16)
@@ -539,7 +542,11 @@ static int gic_set_type(struct irq_data *d, unsigned int type)
                            type != IRQ_TYPE_EDGE_RISING)
                return -EINVAL;

-       return gic_configure_irq(gicirq, type, base, NULL);
+       raw_spin_lock(&irq_controller_lock);
+       ret = gic_configure_irq(gicirq, type, base, NULL);
+       raw_spin_unlock(&irq_controller_lock);
+
+       return ret;
 }

 static int gic_irq_set_vcpu_affinity(struct irq_data *d, void *vcpu)
--
2.15.1