| Message ID | 20220914051842.69776-1-dzm91@hust.edu.cn |
|---|---|
| State | New |
| Headers | show |
| Series | gpio: tpmx86: fix uninitialized variable girq | expand |
On Wed, 14 Sep 2022 06:18:42 +0100, Dongliang Mu <dzm91@hust.edu.cn> wrote: > > From: Dongliang Mu <mudongliangabcd@gmail.com> > > The commit 924610607f19 ("gpio: tpmx86: Move PM device over to > irq domain") adds a dereference of girq that may be uninitialized. > > Fix this by initializing girq and checking irq before invoking > irq_domain_set_pm_device. > > Fixes: 924610607f19 ("gpio: tpmx86: Move PM device over to irq domain") > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> > --- > drivers/gpio/gpio-tqmx86.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpio/gpio-tqmx86.c b/drivers/gpio/gpio-tqmx86.c > index fa4bc7481f9a..bdef182c11c2 100644 > --- a/drivers/gpio/gpio-tqmx86.c > +++ b/drivers/gpio/gpio-tqmx86.c > @@ -231,7 +231,7 @@ static int tqmx86_gpio_probe(struct platform_device *pdev) > struct device *dev = &pdev->dev; > struct tqmx86_gpio_data *gpio; > struct gpio_chip *chip; > - struct gpio_irq_chip *girq; > + struct gpio_irq_chip *girq = NULL; > void __iomem *io_base; > struct resource *res; > int ret, irq; > @@ -315,7 +315,9 @@ static int tqmx86_gpio_probe(struct platform_device *pdev) > goto out_pm_dis; > } > > - irq_domain_set_pm_device(girq->domain, dev); > + if (girq) { > + irq_domain_set_pm_device(girq->domain, dev); > + } > > dev_info(dev, "GPIO functionality initialized with %d pins\n", > chip->ngpio); I wonder if a better fix wouldn't be to directly hoist the irq_domain_set_pm_device() call into the 'if (irq > 0)' block. Thanks, M.
On Wed, Sep 14, 2022 at 3:36 PM Marc Zyngier <maz@kernel.org> wrote: > > On Wed, 14 Sep 2022 06:18:42 +0100, > Dongliang Mu <dzm91@hust.edu.cn> wrote: > > > > From: Dongliang Mu <mudongliangabcd@gmail.com> > > > > The commit 924610607f19 ("gpio: tpmx86: Move PM device over to > > irq domain") adds a dereference of girq that may be uninitialized. > > > > Fix this by initializing girq and checking irq before invoking > > irq_domain_set_pm_device. > > > > Fixes: 924610607f19 ("gpio: tpmx86: Move PM device over to irq domain") > > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> > > --- > > drivers/gpio/gpio-tqmx86.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpio/gpio-tqmx86.c b/drivers/gpio/gpio-tqmx86.c > > index fa4bc7481f9a..bdef182c11c2 100644 > > --- a/drivers/gpio/gpio-tqmx86.c > > +++ b/drivers/gpio/gpio-tqmx86.c > > @@ -231,7 +231,7 @@ static int tqmx86_gpio_probe(struct platform_device *pdev) > > struct device *dev = &pdev->dev; > > struct tqmx86_gpio_data *gpio; > > struct gpio_chip *chip; > > - struct gpio_irq_chip *girq; > > + struct gpio_irq_chip *girq = NULL; > > void __iomem *io_base; > > struct resource *res; > > int ret, irq; > > @@ -315,7 +315,9 @@ static int tqmx86_gpio_probe(struct platform_device *pdev) > > goto out_pm_dis; > > } > > > > - irq_domain_set_pm_device(girq->domain, dev); > > + if (girq) { > > + irq_domain_set_pm_device(girq->domain, dev); > > + } > > > > dev_info(dev, "GPIO functionality initialized with %d pins\n", > > chip->ngpio); > > I wonder if a better fix wouldn't be to directly hoist the > irq_domain_set_pm_device() call into the 'if (irq > 0)' block. If irq_domain_set_pm_device has no dependency on devm_gpiochip_add_data, we can directly move irq_domain_set_pm_device to the if block. > > Thanks, > > M. > > -- > Without deviation from the norm, progress is not possible.
On Thu, 15 Sep 2022 07:08:10 +0100, Dongliang Mu <mudongliangabcd@gmail.com> wrote: > > On Wed, Sep 14, 2022 at 3:36 PM Marc Zyngier <maz@kernel.org> wrote: > > > > On Wed, 14 Sep 2022 06:18:42 +0100, > > Dongliang Mu <dzm91@hust.edu.cn> wrote: > > > > > > From: Dongliang Mu <mudongliangabcd@gmail.com> > > > > > > The commit 924610607f19 ("gpio: tpmx86: Move PM device over to > > > irq domain") adds a dereference of girq that may be uninitialized. > > > > > > Fix this by initializing girq and checking irq before invoking > > > irq_domain_set_pm_device. > > > > > > Fixes: 924610607f19 ("gpio: tpmx86: Move PM device over to irq domain") > > > Signed-off-by: Dongliang Mu <mudongliangabcd@gmail.com> > > > --- > > > drivers/gpio/gpio-tqmx86.c | 6 ++++-- > > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/gpio/gpio-tqmx86.c b/drivers/gpio/gpio-tqmx86.c > > > index fa4bc7481f9a..bdef182c11c2 100644 > > > --- a/drivers/gpio/gpio-tqmx86.c > > > +++ b/drivers/gpio/gpio-tqmx86.c > > > @@ -231,7 +231,7 @@ static int tqmx86_gpio_probe(struct platform_device *pdev) > > > struct device *dev = &pdev->dev; > > > struct tqmx86_gpio_data *gpio; > > > struct gpio_chip *chip; > > > - struct gpio_irq_chip *girq; > > > + struct gpio_irq_chip *girq = NULL; > > > void __iomem *io_base; > > > struct resource *res; > > > int ret, irq; > > > @@ -315,7 +315,9 @@ static int tqmx86_gpio_probe(struct platform_device *pdev) > > > goto out_pm_dis; > > > } > > > > > > - irq_domain_set_pm_device(girq->domain, dev); > > > + if (girq) { > > > + irq_domain_set_pm_device(girq->domain, dev); > > > + } > > > > > > dev_info(dev, "GPIO functionality initialized with %d pins\n", > > > chip->ngpio); > > > > I wonder if a better fix wouldn't be to directly hoist the > > irq_domain_set_pm_device() call into the 'if (irq > 0)' block. > > If irq_domain_set_pm_device has no dependency on > devm_gpiochip_add_data, we can directly move irq_domain_set_pm_device > to the if block. There is no dependency there, and you can convince yourself by looking at the (trivial) code. M.
diff --git a/drivers/gpio/gpio-tqmx86.c b/drivers/gpio/gpio-tqmx86.c index fa4bc7481f9a..bdef182c11c2 100644 --- a/drivers/gpio/gpio-tqmx86.c +++ b/drivers/gpio/gpio-tqmx86.c @@ -231,7 +231,7 @@ static int tqmx86_gpio_probe(struct platform_device *pdev) struct device *dev = &pdev->dev; struct tqmx86_gpio_data *gpio; struct gpio_chip *chip; - struct gpio_irq_chip *girq; + struct gpio_irq_chip *girq = NULL; void __iomem *io_base; struct resource *res; int ret, irq; @@ -315,7 +315,9 @@ static int tqmx86_gpio_probe(struct platform_device *pdev) goto out_pm_dis; } - irq_domain_set_pm_device(girq->domain, dev); + if (girq) { + irq_domain_set_pm_device(girq->domain, dev); + } dev_info(dev, "GPIO functionality initialized with %d pins\n", chip->ngpio);