drivers/of/irq.c | 51 ++++++++++++++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 19 deletions(-)
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 !
Prevent the out-of-bounds read by copying the device address into a
buffer of sufficient size.
Signed-off-by: Stefan Wiehler <stefan.wiehler@nokia.com>
---
drivers/of/irq.c | 51 ++++++++++++++++++++++++++++++------------------
1 file changed, 32 insertions(+), 19 deletions(-)
diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index c94203ce65bb3..34fe0593036d8 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -151,6 +151,26 @@ 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
@@ -167,10 +187,10 @@ const __be32 *of_irq_parse_imap_parent(const __be32 *imap, int len, struct of_ph
*/
int of_irq_parse_raw(const __be32 *addr, 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) };
+ const __be32 dummy_imask[] = { [0 ... MAX_PHANDLE_ARGS] = cpu_to_be32(~0) };
u32 intsize = 1, addrsize;
int i, rc = -EINVAL;
@@ -201,20 +221,7 @@ 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);
-
+ addrsize = of_get_address_cells(ipar);
pr_debug(" -> addrsize=%d\n", addrsize);
/* Range check so that the temporary buffer doesn't overflow */
@@ -343,8 +350,9 @@ 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;
+ __be32 addr_buf[3] = { 0 };
pr_debug("of_irq_parse_one: dev=%pOF, index=%d\n", device, index);
@@ -354,12 +362,17 @@ 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);
+
+ /* Prevent out-of-bounds read in case of longer interrupt parent address size */
+ if (addr)
+ memcpy(addr_buf, addr, addrsize * sizeof(addrsize));
/* 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_buf, out_irq);
/* Look for the interrupt parent. */
p = of_irq_find_parent(device);
@@ -389,7 +402,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_buf, out_irq);
out:
of_node_put(p);
return res;
--
2.42.0On Fri, Aug 9, 2024 at 3:03 AM Stefan Wiehler <stefan.wiehler@nokia.com> wrote:
>
> 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 !
>
> Prevent the out-of-bounds read by copying the device address into a
> buffer of sufficient size.
>
> Signed-off-by: Stefan Wiehler <stefan.wiehler@nokia.com>
> ---
> drivers/of/irq.c | 51 ++++++++++++++++++++++++++++++------------------
> 1 file changed, 32 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index c94203ce65bb3..34fe0593036d8 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -151,6 +151,26 @@ 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)
> +{
See of_bus_n_addr_cells(). It does the same thing. However, I don't
think you need this. See below.
> + 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
> @@ -167,10 +187,10 @@ const __be32 *of_irq_parse_imap_parent(const __be32 *imap, int len, struct of_ph
> */
> int of_irq_parse_raw(const __be32 *addr, 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) };
> + const __be32 dummy_imask[] = { [0 ... MAX_PHANDLE_ARGS] = cpu_to_be32(~0) };
> u32 intsize = 1, addrsize;
> int i, rc = -EINVAL;
>
> @@ -201,20 +221,7 @@ 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);
> -
> + addrsize = of_get_address_cells(ipar);
> pr_debug(" -> addrsize=%d\n", addrsize);
>
> /* Range check so that the temporary buffer doesn't overflow */
> @@ -343,8 +350,9 @@ 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;
> + __be32 addr_buf[3] = { 0 };
>
> pr_debug("of_irq_parse_one: dev=%pOF, index=%d\n", device, index);
>
> @@ -354,12 +362,17 @@ 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);
NULL here is the length returned. Use that and just copy length/4 or 3
cells, which ever is less.
> + addrsize = of_get_address_cells(device);
> +
> + /* Prevent out-of-bounds read in case of longer interrupt parent address size */
> + if (addr)
> + memcpy(addr_buf, addr, addrsize * sizeof(addrsize));
I find sizeof(addrsize) a bit odd here as it just happens to match,
but it's storage size isn't really related. I would do sizeof(__be32)
instead. Though with the above changes, you'll probably have something
like this:
if (len > (3 * sizeof(__be32))
len = 3 * sizeof(__be32);
Rob
© 2016 - 2026 Red Hat, Inc.