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 <[email protected]>
Reported-by: Robert Marko <[email protected]>
Signed-off-by: Sander Vanheule <[email protected]>
---
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(-)
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;
--
2.36.1
On Mon, May 30, 2022 at 3:57 PM Sander Vanheule <[email protected]> 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.
--
With Best Regards,
Andy Shevchenko