| Message ID | 20220530082552.46113-1-sander@svanheule.net |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v1] gpio: realtek-otto: always honor empty CPU masks | expand |
On Mon, May 30, 2022 at 3:57 PM Sander Vanheule <sander@svanheule.net> wrote: > > On uniprocessor builds, for_each_cpu(cpu, mask) will assume 'mask' > always contains exactly one CPU, and ignore the actual mask contents. > This causes the loop to run, even when it shouldn't on an empty mask, > and tries to access an uninitialised pointer. It's too noisy traceback, I believe you may squeeze it out and leave something like ~5-6 lines only. ... > This patch is more of a work-around than a real fix, and ensures the > driver runs properly on uniprocessor builds. My tests were done using an > SMP-enabled build on a single-core system, which is why is missed this > erroneous behaviour. > > The real fix would be a modification of include/linux/cpumask.h, which > may take longer to finalise, but I would rather have the issue in this > driver fixed in the 5.19 release. Hmm... I dunno that cpumask fix should be hard or easy, but I think you may try it simultaneously, so we will win one way or the other.
diff --git a/drivers/gpio/gpio-realtek-otto.c b/drivers/gpio/gpio-realtek-otto.c index c52b2cb1acae..5391ebcd5bce 100644 --- a/drivers/gpio/gpio-realtek-otto.c +++ b/drivers/gpio/gpio-realtek-otto.c @@ -301,6 +301,7 @@ static int realtek_gpio_irq_set_affinity(struct irq_data *data, static int realtek_gpio_irq_init(struct gpio_chip *gc) { struct realtek_gpio_ctrl *ctrl = gpiochip_get_data(gc); + void __iomem *irq_cpu_mask; unsigned int port; int cpu; @@ -308,8 +309,16 @@ static int realtek_gpio_irq_init(struct gpio_chip *gc) realtek_gpio_write_imr(ctrl, port, 0, 0); realtek_gpio_clear_isr(ctrl, port, GENMASK(7, 0)); - for_each_cpu(cpu, &ctrl->cpu_irq_maskable) - iowrite8(GENMASK(7, 0), realtek_gpio_irq_cpu_mask(ctrl, port, cpu)); + /* + * Uniprocessor builds assume a mask always contains one CPU, + * so only start the loop if we have at least one maskable CPU. + */ + if (!cpumask_empty(&ctrl->cpu_irq_maskable)) { + for_each_cpu(cpu, &ctrl->cpu_irq_maskable) { + irq_cpu_mask = realtek_gpio_irq_cpu_mask(ctrl, port, cpu); + iowrite8(GENMASK(7, 0), irq_cpu_mask); + } + } } return 0;
On uniprocessor builds, for_each_cpu(cpu, mask) will assume 'mask' always contains exactly one CPU, and ignore the actual mask contents. This causes the loop to run, even when it shouldn't on an empty mask, and tries to access an uninitialised pointer. CPU 0 Unable to handle kernel paging request at virtual address 00000000, epc == 802967bc, ra == 802e2f4c Oops[#1]: CPU: 0 PID: 1 Comm: swapper Not tainted 5.10.115 #0 $ 0 : 00000000 00000001 00000000 00000018 $ 4 : 000000ff 00000000 00000000 00000000 $ 8 : 00000024 802cbb58 00000018 6f2d636f $12 : 00000000 00006382 00000000 fffffffc $16 : 82272c80 00000000 82272c80 00000000 $20 : 82272e08 00000000 80750000 00000000 $24 : 00000008 00000020 $28 : 8201a000 8201bc28 80750000 802e2f4c Hi : 000044fc Lo : 0000227e epc : 802967bc iowrite8+0x4/0x10 ra : 802e2f4c realtek_gpio_irq_init+0xac/0xe0 Status: 1010fc03 KERNEL EXL IE Cause : 1080000c (ExcCode 03) BadVA : 00000000 PrId : 00019070 (MIPS 4KEc) Modules linked in: Process swapper (pid: 1, threadinfo=(ptrval), task=(ptrval), tls=00000000) Stack : 82272c80 82272e00 80fc6e7c 802dee90 82272c80 82272c80 82272e00 8074d7d8 8074d7d8 802dce50 8201bc5c 806d0000 00000000 00000cc0 80f90000 82272c80 802ddb74 80650000 82084410 82272e08 82275b80 82272c80 82272c80 82084410 00000000 82272d78 00000000 8075d094 80f90000 802ddb00 80fc0000 80320364 00000000 803ac928 82272c80 00000017 00000000 82084410 82084400 802e2d88 ... Call Trace: [<802967bc>] iowrite8+0x4/0x10 [<802dce50>] gpiochip_add_data_with_key+0x508/0xb14 [<802ddb00>] devm_gpiochip_add_data_with_key+0x60/0xd4 [<802e2d88>] realtek_gpio_probe+0x1e0/0x2f8 [<8031e214>] platform_drv_probe+0x40/0x94 [<8031bf1c>] really_probe+0x108/0x4d8 [<8031c99c>] device_driver_attach+0x120/0x130 [<8031ca28>] __driver_attach+0x7c/0x13c [<80319a70>] bus_for_each_dev+0x68/0xa4 [<8031b16c>] bus_add_driver+0x1c8/0x210 [<8031d1f0>] driver_register+0x98/0x154 [<80000638>] do_one_initcall+0x50/0x1ac [<8075de8c>] do_initcalls+0x100/0x14c [<8075e044>] kernel_init_freeable+0xfc/0x138 [<80586ed0>] kernel_init+0x10/0xf8 [<80001a18>] ret_from_kernel_thread+0x14/0x1c Code: 03e00008 00000000 0000000f <a0a40000> 03e00008 00000000 0000000f a4a40000 03e00008 ---[ end trace 438028f3ae26080b ]--- Kernel panic - not syncing: Fatal exception Fixes: 95fa6dbe58f2 ("gpio: realtek-otto: Support per-cpu interrupts") Reported-by: INAGAKI Hiroshi <musashino.open@gmail.com> Reported-by: Robert Marko <robimarko@gmail.com> Signed-off-by: Sander Vanheule <sander@svanheule.net> --- This patch is more of a work-around than a real fix, and ensures the driver runs properly on uniprocessor builds. My tests were done using an SMP-enabled build on a single-core system, which is why is missed this erroneous behaviour. The real fix would be a modification of include/linux/cpumask.h, which may take longer to finalise, but I would rather have the issue in this driver fixed in the 5.19 release. Best, Sander drivers/gpio/gpio-realtek-otto.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)