linux-kernel - [PATCH v2 3/3] genirq/irqdomain: Detect type race in irq_create_fwspec

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

Message-ID: <20190927150025.26481-4-alexander.sverdlin@nokia.com>
Date:   Fri, 27 Sep 2019 15:00:45 +0000
From:   "Sverdlin, Alexander (Nokia - DE/Ulm)" <alexander.sverdlin@...ia.com>
To:     Marc Zyngier <maz@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Grant Likely <grant.likely@...retlab.ca>
CC:     "Sverdlin, Alexander (Nokia - DE/Ulm)" <alexander.sverdlin@...ia.com>,
        Mark Brown <broonie@...nel.org>,
        Jon Hunter <jonathanh@...dia.com>,
        "Glavinic-Pecotic, Matija (EXT - DE/Ulm)" 
        <matija.glavinic-pecotic.ext@...ia.com>,
        "Adamski, Krzysztof (Nokia - PL/Wroclaw)" 
        <krzysztof.adamski@...ia.com>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: [PATCH v2 3/3] genirq/irqdomain: Detect type race in
 irq_create_fwspec_mapping()

From: Alexander Sverdlin <alexander.sverdlin@...ia.com>

irq_create_fwspec_mapping() can race with itself during IRQ trigger type
configuration. Possible scenarios include:

- Mapping exists, two irq_create_fwspec_mapping() running in parallel do
  not detect type mismatch, IRQ remains configured with one of the
  different trigger types randomly
- Second call to irq_create_fwspec_mapping() sees existing mapping just
  created by first call, but earlier irqd_set_trigger_type() call races
  with later irqd_set_trigger_type() => totally undetected, IRQ type
  is being set randomly to either one or another type

Introduce helper function to detect parallel changes to IRQ type.

Cc: stable@...r.kernel.org
Signed-off-by: Alexander Sverdlin <alexander.sverdlin@...ia.com>
---
 kernel/irq/irqdomain.c | 66 +++++++++++++++++++++++++++++---------------------
 1 file changed, 38 insertions(+), 28 deletions(-)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index ad62c08..4ff4073 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -764,10 +764,45 @@ static void of_phandle_args_to_fwspec(struct device_node *np, const u32 *args,
 		fwspec->param[i] = args[i];
 }
 
+/* Detect races during IRQ type setting */
+static int irq_set_trigger_type_locked(unsigned int virq, unsigned int type,
+				       irq_hw_number_t hwirq,
+				       const struct irq_fwspec *fwspec)
+{
+	struct irq_data *irq_data;
+	int ret = 0;
+
+	mutex_lock(&irq_domain_mutex);
+	/*
+	 * If the trigger type is not specified or matches the current trigger
+	 * type then we are done.
+	 */
+	if (type == IRQ_TYPE_NONE || type == irq_get_trigger_type(virq))
+		goto unlock;
+
+	/* If the trigger type has not been set yet, then set it now */
+	if (irq_get_trigger_type(virq) != IRQ_TYPE_NONE) {
+		pr_warn("type mismatch, failed to map hwirq-%lu for %s!\n",
+			hwirq, of_node_full_name(to_of_node(fwspec->fwnode)));
+		ret = -EINVAL;
+		goto unlock;
+	}
+
+	irq_data = irq_get_irq_data(virq);
+	if (!irq_data) {
+		ret = -ENOENT;
+		goto unlock;
+	}
+	irqd_set_trigger_type(irq_data, type);
+
+unlock:
+	mutex_unlock(&irq_domain_mutex);
+	return ret;
+}
+
 unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
 {
 	struct irq_domain *domain;
-	struct irq_data *irq_data;
 	irq_hw_number_t hwirq;
 	unsigned int type = IRQ_TYPE_NONE;
 	int virq;
@@ -802,29 +837,8 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
 	 */
 	virq = irq_find_mapping(domain, hwirq);
 	if (virq) {
-		/*
-		 * If the trigger type is not specified or matches the
-		 * current trigger type then we are done so return the
-		 * interrupt number.
-		 */
-		if (type == IRQ_TYPE_NONE || type == irq_get_trigger_type(virq))
-			return virq;
-
-		/*
-		 * If the trigger type has not been set yet, then set
-		 * it now and return the interrupt number.
-		 */
-		if (irq_get_trigger_type(virq) == IRQ_TYPE_NONE) {
-			irq_data = irq_get_irq_data(virq);
-			if (!irq_data)
-				return 0;
-
-			irqd_set_trigger_type(irq_data, type);
+		if (!irq_set_trigger_type_locked(virq, type, hwirq, fwspec))
 			return virq;
-		}
-
-		pr_warn("type mismatch, failed to map hwirq-%lu for %s!\n",
-			hwirq, of_node_full_name(to_of_node(fwspec->fwnode)));
 		return 0;
 	}
 
@@ -839,8 +853,7 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
 			return virq;
 	}
 
-	irq_data = irq_get_irq_data(virq);
-	if (!irq_data) {
+	if (irq_set_trigger_type_locked(virq, type, hwirq, fwspec)) {
 		if (irq_domain_is_hierarchy(domain))
 			irq_domain_free_irqs(virq, 1);
 		else
@@ -848,9 +861,6 @@ unsigned int irq_create_fwspec_mapping(struct irq_fwspec *fwspec)
 		return 0;
 	}
 
-	/* Store trigger type */
-	irqd_set_trigger_type(irq_data, type);
-
 	return virq;
 }
 EXPORT_SYMBOL_GPL(irq_create_fwspec_mapping);