devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] of/irq: Consider device address size in interrupt map walk
@ 2024-08-07 13:40 Stefan Wiehler
  2024-08-07 14:09 ` Rob Herring
  2024-08-09  0:20 ` kernel test robot
  0 siblings, 2 replies; 3+ messages in thread
From: Stefan Wiehler @ 2024-08-07 13:40 UTC (permalink / raw)
  To: Rob Herring, Saravana Kannan; +Cc: devicetree, linux-kernel, Stefan Wiehler

When of_irq_parse_raw() is invoked with a device address smaller than
the interrupt parent node (from #address-cells property), KASAN detects
the following out-of-bounds read when populating the initial match table
(dyndbg="func of_irq_parse_* +p"):

  OF: of_irq_parse_one: dev=/soc@0/picasso/watchdog, index=0
  OF:  parent=/soc@0/pci@878000000000/gpio0@17,0, intsize=2
  OF:  intspec=4
  OF: of_irq_parse_raw: ipar=/soc@0/pci@878000000000/gpio0@17,0, size=2
  OF:  -> addrsize=3
  ==================================================================
  BUG: KASAN: slab-out-of-bounds in of_irq_parse_raw+0x2b8/0x8d0
  Read of size 4 at addr ffffff81beca5608 by task bash/764

  CPU: 1 PID: 764 Comm: bash Tainted: G           O       6.1.67-484c613561-nokia_sm_arm64 #1
  Hardware name: Unknown Unknown Product/Unknown Product, BIOS 2023.01-12.24.03-dirty 01/01/2023
  Call trace:
   dump_backtrace+0xdc/0x130
   show_stack+0x1c/0x30
   dump_stack_lvl+0x6c/0x84
   print_report+0x150/0x448
   kasan_report+0x98/0x140
   __asan_load4+0x78/0xa0
   of_irq_parse_raw+0x2b8/0x8d0
   of_irq_parse_one+0x24c/0x270
   parse_interrupts+0xc0/0x120
   of_fwnode_add_links+0x100/0x2d0
   fw_devlink_parse_fwtree+0x64/0xc0
   device_add+0xb38/0xc30
   of_device_add+0x64/0x90
   of_platform_device_create_pdata+0xd0/0x170
   of_platform_bus_create+0x244/0x600
   of_platform_notify+0x1b0/0x254
   blocking_notifier_call_chain+0x9c/0xd0
   __of_changeset_entry_notify+0x1b8/0x230
   __of_changeset_apply_notify+0x54/0xe4
   of_overlay_fdt_apply+0xc04/0xd94
   ...

  The buggy address belongs to the object at ffffff81beca5600
   which belongs to the cache kmalloc-128 of size 128
  The buggy address is located 8 bytes inside of
   128-byte region [ffffff81beca5600, ffffff81beca5680)

  The buggy address belongs to the physical page:
  page:00000000230d3d03 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x1beca4
  head:00000000230d3d03 order:1 compound_mapcount:0 compound_pincount:0
  flags: 0x8000000000010200(slab|head|zone=2)
  raw: 8000000000010200 0000000000000000 dead000000000122 ffffff810000c300
  raw: 0000000000000000 0000000000200020 00000001ffffffff 0000000000000000
  page dumped because: kasan: bad access detected

  Memory state around the buggy address:
   ffffff81beca5500: 04 fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
   ffffff81beca5580: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
  >ffffff81beca5600: 00 fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
                        ^
   ffffff81beca5680: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
   ffffff81beca5700: 00 00 00 00 00 00 fc fc fc fc fc fc fc fc fc fc
  ==================================================================
  OF:  -> got it !

Signed-off-by: Stefan Wiehler <stefan.wiehler@nokia.com>
---
- Fixed of_get_address_cells() open brace found by checkpatch
---
 arch/powerpc/platforms/fsl_uli1575.c |  2 +-
 drivers/bcma/main.c                  |  2 +-
 drivers/of/irq.c                     | 65 ++++++++++++++++------------
 drivers/pci/of.c                     |  2 +-
 include/linux/of_irq.h               |  3 +-
 5 files changed, 42 insertions(+), 32 deletions(-)

diff --git a/arch/powerpc/platforms/fsl_uli1575.c b/arch/powerpc/platforms/fsl_uli1575.c
index b8d37a9932f1b..8da36f72b7b48 100644
--- a/arch/powerpc/platforms/fsl_uli1575.c
+++ b/arch/powerpc/platforms/fsl_uli1575.c
@@ -335,7 +335,7 @@ static void hpcd_final_uli5288(struct pci_dev *dev)
 	oirq.args_count = 1;
 	laddr[0] = (hose->first_busno << 16) | (PCI_DEVFN(31, 0) << 8);
 	laddr[1] = laddr[2] = 0;
-	of_irq_parse_raw(laddr, &oirq);
+	of_irq_parse_raw(laddr, ARRAY_SIZE(laddr), &oirq);
 	dev->irq = irq_create_of_mapping(&oirq);
 }
 
diff --git a/drivers/bcma/main.c b/drivers/bcma/main.c
index 6b5d34919c72b..57d946305496a 100644
--- a/drivers/bcma/main.c
+++ b/drivers/bcma/main.c
@@ -175,7 +175,7 @@ static int bcma_of_irq_parse(struct device *parent,
 	out_irq->args[0] = num;
 
 	laddr[0] = cpu_to_be32(core->addr);
-	return of_irq_parse_raw(laddr, out_irq);
+	return of_irq_parse_raw(laddr, ARRAY_SIZE(laddr), out_irq);
 }
 
 static unsigned int bcma_of_get_irq(struct device *parent,
diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index c94203ce65bb3..78c35f81cde8f 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -151,9 +151,30 @@ const __be32 *of_irq_parse_imap_parent(const __be32 *imap, int len, struct of_ph
 	return imap;
 }
 
+static u32 of_get_address_cells(struct device_node *node)
+{
+	struct device_node *tnode, *old = NULL;
+	const __be32 *tmp;
+
+	/* Look for this #address-cells. We have to implement the old linux
+	 * trick of looking for the parent here as some device-trees rely on it
+	 */
+	old = of_node_get(node);
+	do {
+		tmp = of_get_property(old, "#address-cells", NULL);
+		tnode = of_get_parent(old);
+		of_node_put(old);
+		old = tnode;
+	} while (old && tmp == NULL);
+	of_node_put(old);
+	old = NULL;
+	return (tmp == NULL) ? 2 : be32_to_cpu(*tmp);
+}
+
 /**
  * of_irq_parse_raw - Low level interrupt tree parsing
  * @addr:	address specifier (start of "reg" property of the device) in be32 format
+ * @addrsize:	address cell size ("#address-cells" property of the device (parent))
  * @out_irq:	structure of_phandle_args updated by this function
  *
  * This function is a low-level interrupt tree walking function. It
@@ -165,13 +186,13 @@ const __be32 *of_irq_parse_imap_parent(const __be32 *imap, int len, struct of_ph
  *
  * Return: 0 on success and a negative number on error
  */
-int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
+int of_irq_parse_raw(const __be32 *addr, u32 addrsize, struct of_phandle_args *out_irq)
 {
-	struct device_node *ipar, *tnode, *old = NULL;
+	struct device_node *ipar, *tnode;
 	__be32 initial_match_array[MAX_PHANDLE_ARGS];
 	const __be32 *match_array = initial_match_array;
-	const __be32 *tmp, dummy_imask[] = { [0 ... MAX_PHANDLE_ARGS] = cpu_to_be32(~0) };
-	u32 intsize = 1, addrsize;
+	const __be32 dummy_imask[] = { [0 ... MAX_PHANDLE_ARGS] = cpu_to_be32(~0) };
+	u32 intsize = 1, ipar_addrsize;
 	int i, rc = -EINVAL;
 
 #ifdef DEBUG
@@ -201,24 +222,11 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
 	if (out_irq->args_count != intsize)
 		goto fail;
 
-	/* Look for this #address-cells. We have to implement the old linux
-	 * trick of looking for the parent here as some device-trees rely on it
-	 */
-	old = of_node_get(ipar);
-	do {
-		tmp = of_get_property(old, "#address-cells", NULL);
-		tnode = of_get_parent(old);
-		of_node_put(old);
-		old = tnode;
-	} while (old && tmp == NULL);
-	of_node_put(old);
-	old = NULL;
-	addrsize = (tmp == NULL) ? 2 : be32_to_cpu(*tmp);
-
-	pr_debug(" -> addrsize=%d\n", addrsize);
+	ipar_addrsize = of_get_address_cells(ipar);
+	pr_debug(" -> addrsize=%d, ipar_addrsize=%d\n", addrsize, ipar_addrsize);
 
 	/* Range check so that the temporary buffer doesn't overflow */
-	if (WARN_ON(addrsize + intsize > MAX_PHANDLE_ARGS)) {
+	if (WARN_ON(ipar_addrsize + intsize > MAX_PHANDLE_ARGS)) {
 		rc = -EFAULT;
 		goto fail;
 	}
@@ -227,7 +235,7 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
 	for (i = 0; i < addrsize; i++)
 		initial_match_array[i] = addr ? addr[i] : 0;
 	for (i = 0; i < intsize; i++)
-		initial_match_array[addrsize + i] = cpu_to_be32(out_irq->args[i]);
+		initial_match_array[ipar_addrsize + i] = cpu_to_be32(out_irq->args[i]);
 
 	/* Now start the actual "proper" walk of the interrupt tree */
 	while (ipar != NULL) {
@@ -254,7 +262,7 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
 		 * interrupt-map parsing does not work without a reg
 		 * property when #address-cells != 0
 		 */
-		if (addrsize && !addr) {
+		if (ipar_addrsize && !addr) {
 			pr_debug(" -> no reg passed in when needed !\n");
 			goto fail;
 		}
@@ -274,10 +282,10 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
 
 		/* Parse interrupt-map */
 		match = 0;
-		while (imaplen > (addrsize + intsize + 1)) {
+		while (imaplen > (ipar_addrsize + intsize + 1)) {
 			/* Compare specifiers */
 			match = 1;
-			for (i = 0; i < (addrsize + intsize); i++, imaplen--)
+			for (i = 0; i < (ipar_addrsize + intsize); i++, imaplen--)
 				match &= !((match_array[i] ^ *imap++) & imask[i]);
 
 			pr_debug(" -> match=%d (imaplen=%d)\n", match, imaplen);
@@ -306,7 +314,7 @@ int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq)
 
 		newpar = out_irq->np;
 		intsize = out_irq->args_count;
-		addrsize = (imap - match_array) - intsize;
+		ipar_addrsize = (imap - match_array) - intsize;
 
 		if (ipar == newpar) {
 			pr_debug("%pOF interrupt-map entry to self\n", ipar);
@@ -343,7 +351,7 @@ int of_irq_parse_one(struct device_node *device, int index, struct of_phandle_ar
 {
 	struct device_node *p;
 	const __be32 *addr;
-	u32 intsize;
+	u32 addrsize, intsize;
 	int i, res;
 
 	pr_debug("of_irq_parse_one: dev=%pOF, index=%d\n", device, index);
@@ -354,12 +362,13 @@ int of_irq_parse_one(struct device_node *device, int index, struct of_phandle_ar
 
 	/* Get the reg property (if any) */
 	addr = of_get_property(device, "reg", NULL);
+	addrsize = of_get_address_cells(device);
 
 	/* Try the new-style interrupts-extended first */
 	res = of_parse_phandle_with_args(device, "interrupts-extended",
 					"#interrupt-cells", index, out_irq);
 	if (!res)
-		return of_irq_parse_raw(addr, out_irq);
+		return of_irq_parse_raw(addr, addrsize, out_irq);
 
 	/* Look for the interrupt parent. */
 	p = of_irq_find_parent(device);
@@ -389,7 +398,7 @@ int of_irq_parse_one(struct device_node *device, int index, struct of_phandle_ar
 
 
 	/* Check if there are any interrupt-map translations to process */
-	res = of_irq_parse_raw(addr, out_irq);
+	res = of_irq_parse_raw(addr, addrsize, out_irq);
  out:
 	of_node_put(p);
 	return res;
diff --git a/drivers/pci/of.c b/drivers/pci/of.c
index 51e3dd0ea5abe..5d4b2ec5a3b13 100644
--- a/drivers/pci/of.c
+++ b/drivers/pci/of.c
@@ -506,7 +506,7 @@ static int of_irq_parse_pci(const struct pci_dev *pdev, struct of_phandle_args *
 	out_irq->args[0] = pin;
 	laddr[0] = cpu_to_be32((pdev->bus->number << 16) | (pdev->devfn << 8));
 	laddr[1] = laddr[2] = cpu_to_be32(0);
-	rc = of_irq_parse_raw(laddr, out_irq);
+	rc = of_irq_parse_raw(laddr, ARRAY_SIZE(laddr), out_irq);
 	if (rc)
 		goto err;
 	return 0;
diff --git a/include/linux/of_irq.h b/include/linux/of_irq.h
index d6d3eae2f1452..74268e33d7305 100644
--- a/include/linux/of_irq.h
+++ b/include/linux/of_irq.h
@@ -32,7 +32,8 @@ static inline int of_irq_parse_oldworld(const struct device_node *device, int in
 }
 #endif /* CONFIG_PPC32 && CONFIG_PPC_PMAC */
 
-extern int of_irq_parse_raw(const __be32 *addr, struct of_phandle_args *out_irq);
+extern int of_irq_parse_raw(const __be32 *addr, u32 addrsize,
+			    struct of_phandle_args *out_irq);
 extern unsigned int irq_create_of_mapping(struct of_phandle_args *irq_data);
 extern int of_irq_to_resource(struct device_node *dev, int index,
 			      struct resource *r);
-- 
2.42.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-08-09  0:21 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-07 13:40 [PATCH v2] of/irq: Consider device address size in interrupt map walk Stefan Wiehler
2024-08-07 14:09 ` Rob Herring
2024-08-09  0:20 ` kernel test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).