| Message ID | 20220529174933.293405-1-sander@svanheule.net |
|---|---|
| State | Accepted |
| Delegated to: | Sander Vanheule |
| Headers | show |
| Series | realtek: don't unmask non-maskable GPIO IRQs | expand |
On Sun, 29 May 2022 at 19:49, 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 unintialised pointer. > > Fix this by wrapping the loop in a cpumask_empty() check, to ensure it > will not run on uniprocessor builds if the CPU mask is empty. > > Fixes: af6cd37f42f3 ("realtek: replace RTL93xx GPIO patches") > Reported-by: Robert Marko <robimarko@gmail.com> > Reported-by: INAGAKI Hiroshi <musashino.open@gmail.com> > Signed-off-by: Sander Vanheule <sander@svanheule.net> Works on DGS-1210-28P, so: Tested-by: Robert Marko <robimarko@gmail.com> Regards, Robert > --- > .../316-otto-gpio-uniprocessor-irq-mask.patch | 29 +++++++++++++++++++ > 1 file changed, 29 insertions(+) > create mode 100644 target/linux/realtek/patches-5.10/316-otto-gpio-uniprocessor-irq-mask.patch > > diff --git a/target/linux/realtek/patches-5.10/316-otto-gpio-uniprocessor-irq-mask.patch b/target/linux/realtek/patches-5.10/316-otto-gpio-uniprocessor-irq-mask.patch > new file mode 100644 > index 000000000000..f83f5a781f74 > --- /dev/null > +++ b/target/linux/realtek/patches-5.10/316-otto-gpio-uniprocessor-irq-mask.patch > @@ -0,0 +1,29 @@ > +--- a/drivers/gpio/gpio-realtek-otto.c > ++++ b/drivers/gpio/gpio-realtek-otto.c > +@@ -304,6 +304,7 @@ static int realtek_gpio_irq_set_affinity > + 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; > + > +@@ -311,8 +312,16 @@ static int realtek_gpio_irq_init(struct > + 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 Sun, 2022-05-29 at 23:22 +0200, Robert Marko wrote: > On Sun, 29 May 2022 at 19:49, 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 unintialised pointer. > > > > Fix this by wrapping the loop in a cpumask_empty() check, to ensure it > > will not run on uniprocessor builds if the CPU mask is empty. > > > > Fixes: af6cd37f42f3 ("realtek: replace RTL93xx GPIO patches") > > Reported-by: Robert Marko <robimarko@gmail.com> > > Reported-by: INAGAKI Hiroshi <musashino.open@gmail.com> > > Signed-off-by: Sander Vanheule <sander@svanheule.net> > > Works on DGS-1210-28P, so: > Tested-by: Robert Marko <robimarko@gmail.com> Thanks for giving it a spin as well! I've merged the patch into master, after fixing a whitespace issue and typo in the commit message. Best, Sander
diff --git a/target/linux/realtek/patches-5.10/316-otto-gpio-uniprocessor-irq-mask.patch b/target/linux/realtek/patches-5.10/316-otto-gpio-uniprocessor-irq-mask.patch new file mode 100644 index 000000000000..f83f5a781f74 --- /dev/null +++ b/target/linux/realtek/patches-5.10/316-otto-gpio-uniprocessor-irq-mask.patch @@ -0,0 +1,29 @@ +--- a/drivers/gpio/gpio-realtek-otto.c ++++ b/drivers/gpio/gpio-realtek-otto.c +@@ -304,6 +304,7 @@ static int realtek_gpio_irq_set_affinity + 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; + +@@ -311,8 +312,16 @@ static int realtek_gpio_irq_init(struct + 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 unintialised pointer. Fix this by wrapping the loop in a cpumask_empty() check, to ensure it will not run on uniprocessor builds if the CPU mask is empty. Fixes: af6cd37f42f3 ("realtek: replace RTL93xx GPIO patches") Reported-by: Robert Marko <robimarko@gmail.com> Reported-by: INAGAKI Hiroshi <musashino.open@gmail.com> Signed-off-by: Sander Vanheule <sander@svanheule.net> --- .../316-otto-gpio-uniprocessor-irq-mask.patch | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 target/linux/realtek/patches-5.10/316-otto-gpio-uniprocessor-irq-mask.patch