On Wed, Aug 7, 2024 at 6:04 AM Stefan Wiehler <stefan.wiehler@xxxxxxxxx> 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"): You've missed a bunch of people/lists. Use get_maintainers.pl. > 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 Can you provide some details on what these nodes look like. The interrupt provider to an SoC device is a PCI device? That's weird... > ================================================================== > 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 Note that of_irq_parse_raw() was refactored in 6.10, so your patch is not going to apply. > 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 I wonder if it is possible for KASAN to detect this if it is a base DT given the allocation is done by memblock. > ... > > 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@xxxxxxxxx> > --- > arch/powerpc/platforms/fsl_uli1575.c | 2 +- > drivers/bcma/main.c | 2 +- > drivers/of/irq.c | 64 ++++++++++++++++------------ > drivers/pci/of.c | 2 +- > include/linux/of_irq.h | 3 +- > 5 files changed, 41 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); That's not the right information to parse the address correctly. You would need the device's #address-cells. However, in most cases we don't really need to parse the address. The address is not really used except in cases where interrupt routing matches the bus and so there is only one size. That's effectively only PCI devices today. In that case, the address size would always be equal, and the code implicitly assumes that. It would be better if we could just detect when to use the address or not. I think we'd have to look at 'interrupt-map-mask' first to see whether or not to read the address cells. Or maybe we could detect when the interrupt parent is the device's parent node. Either way, this code is tricky and hard to change without breaking something. A simpler way to fix this is just always pass in an address buffer of 3 cells to of_irq_parse_raw. You would just need to copy the cells in of_irq_parse_one() to a temporary buffer. Rob
