linux-kernel - [PATCH v2 5/7] irqchip/gic-v3-its: Only emit SYNC if targetting a valid collection

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20180622095254.5906-6-marc.zyngier@arm.com>
Date:   Fri, 22 Jun 2018 10:52:52 +0100
From:   Marc Zyngier <marc.zyngier@....com>
To:     Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>
Cc:     linux-kernel@...r.kernel.org,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        Yang Yingliang <yangyingliang@...wei.com>,
        Sumit Garg <sumit.garg@...aro.org>
Subject: [PATCH v2 5/7] irqchip/gic-v3-its: Only emit SYNC if targetting a valid collection

It is possible, under obscure circumstances, to convince the ITS
driver to emit a SYNC operation that targets a collection that is
not bound to any redistributor (and the target_address field is
zero) because the corresponding CPU has not been seen yet (the
system has been booted with max_cpus="something small").

If the ITS is using the linear CPU number as the target, this is
not a big deal, as we just end-up issuing a SYNC to CPU0. But if
the ITS requires the physical address of the redistributor (with
GITS_TYPER.PTA==1), we end-up asking the ITS to write to the
physical address zero, which is not exactly a good idea (there
has been report of the ITS locking up). This should of course
never happen, but hey, this is SW...

In order to avoid the above disaster, let's track which collections
have been actually initialized, and let's not generate a SYNC
if the collection hasn't been properly bound to a redistributor.
We take this opportunity to spit our a warning, in the hope that
someone may report the issue if it arrises again.

Reported-by: Yang Yingliang <yangyingliang@...wei.com>
Signed-off-by: Marc Zyngier <marc.zyngier@....com>
---
 drivers/irqchip/irq-gic-v3-its.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
index cae53937feeb..fcfc96f8e0de 100644
--- a/drivers/irqchip/irq-gic-v3-its.c
+++ b/drivers/irqchip/irq-gic-v3-its.c
@@ -182,6 +182,14 @@ static struct its_collection *dev_event_to_col(struct its_device *its_dev,
 	return its->collections + its_dev->event_map.col_map[event];
 }
 
+static struct its_collection *valid_col(struct its_collection *col)
+{
+	if (WARN_ON_ONCE(col->target_address & GENMASK_ULL(0, 15)))
+		return NULL;
+
+	return col;
+}
+
 /*
  * ITS command descriptors - parameters to be encoded in a command
  * block.
@@ -439,7 +447,7 @@ static struct its_collection *its_build_mapti_cmd(struct its_node *its,
 
 	its_fixup_cmd(cmd);
 
-	return col;
+	return valid_col(col);
 }
 
 static struct its_collection *its_build_movi_cmd(struct its_node *its,
@@ -458,7 +466,7 @@ static struct its_collection *its_build_movi_cmd(struct its_node *its,
 
 	its_fixup_cmd(cmd);
 
-	return col;
+	return valid_col(col);
 }
 
 static struct its_collection *its_build_discard_cmd(struct its_node *its,
@@ -476,7 +484,7 @@ static struct its_collection *its_build_discard_cmd(struct its_node *its,
 
 	its_fixup_cmd(cmd);
 
-	return col;
+	return valid_col(col);
 }
 
 static struct its_collection *its_build_inv_cmd(struct its_node *its,
@@ -494,7 +502,7 @@ static struct its_collection *its_build_inv_cmd(struct its_node *its,
 
 	its_fixup_cmd(cmd);
 
-	return col;
+	return valid_col(col);
 }
 
 static struct its_collection *its_build_int_cmd(struct its_node *its,
@@ -512,7 +520,7 @@ static struct its_collection *its_build_int_cmd(struct its_node *its,
 
 	its_fixup_cmd(cmd);
 
-	return col;
+	return valid_col(col);
 }
 
 static struct its_collection *its_build_clear_cmd(struct its_node *its,
@@ -530,7 +538,7 @@ static struct its_collection *its_build_clear_cmd(struct its_node *its,
 
 	its_fixup_cmd(cmd);
 
-	return col;
+	return valid_col(col);
 }
 
 static struct its_collection *its_build_invall_cmd(struct its_node *its,
@@ -1824,11 +1832,16 @@ static int its_alloc_tables(struct its_node *its)
 
 static int its_alloc_collections(struct its_node *its)
 {
+	int i;
+
 	its->collections = kcalloc(nr_cpu_ids, sizeof(*its->collections),
 				   GFP_KERNEL);
 	if (!its->collections)
 		return -ENOMEM;
 
+	for (i = 0; i < nr_cpu_ids; i++)
+		its->collections[i].target_address = ~0ULL;
+
 	return 0;
 }
 
-- 
2.17.1