linux-kernel - Re: [PATCH v2] drivers: spi: Insert the missing pci_dev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMuHMdWNjo69_W6f+R9QJJOf8uF0htg2XazeS-yjugJv3UM+kg@mail.gmail.com>
Date: Fri, 30 Aug 2024 10:55:06 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Yang Ruibin <11162571@...o.com>, Mark Brown <broonie@...nel.org>
Cc: Daniel Mack <daniel@...que.org>, Haojian Zhuang <haojian.zhuang@...il.com>, 
	Robert Jarzmik <robert.jarzmik@...e.fr>, linux-arm-kernel@...ts.infradead.org, 
	linux-spi@...r.kernel.org, linux-kernel@...r.kernel.org, 
	opensource.kernel@...o.com, 
	Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Subject: Re: [PATCH v2] drivers: spi: Insert the missing pci_dev_put()before return

Hi Yang,

On Thu, Aug 29, 2024 at 5:35 AM Yang Ruibin <11162571@...o.com> wrote:
> Increase the reference count by calling pci_get_slot(), and remember to
> decrement the reference count by calling pci_dev_put().
>
> Signed-off-by: Yang Ruibin <11162571@...o.com>

Thanks for your patch, which is now commit 8a0ec8c2d736961f ("spi:
Insert the missing pci_dev_put()before return") in spi/for-next.

> --- a/drivers/spi/spi-pxa2xx-pci.c
> +++ b/drivers/spi/spi-pxa2xx-pci.c
> @@ -146,8 +146,10 @@ static int lpss_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c)
>         c->num_chipselect = 1;
>
>         ret = pxa2xx_spi_pci_clk_register(dev, ssp, 50000000);
> -       if (ret)
> +       if (ret) {
> +               pci_dev_put(dma_dev);

dma_dev is still uninitialized at this point.

>                 return ret;
> +       }
>
>         dma_dev = pci_get_slot(dev->bus, PCI_DEVFN(PCI_SLOT(dev->devfn), 0));

dma_dev is initialized only here...

>         ret = devm_add_action_or_reset(&dev->dev, lpss_dma_put_device, dma_dev);

... and freed automatically by lpss_dma_put_device() in case of
any later failures since commit 609d7ffdc42199a0 ("spi: pxa2xx-pci:
Balance reference count for PCI DMA device") in v5.18.

> @@ -222,8 +224,10 @@ static int mrfld_spi_setup(struct pci_dev *dev, struct pxa2xx_spi_controller *c)
>         }
>
>         ret = pxa2xx_spi_pci_clk_register(dev, ssp, 25000000);
> -       if (ret)
> +       if (ret) {
> +               pci_dev_put(dma_dev);
>                 return ret;
> +       }
>
>         dma_dev = pci_get_slot(dev->bus, PCI_DEVFN(21, 0));
>         ret = devm_add_action_or_reset(&dev->dev, lpss_dma_put_device, dma_dev);

Likewise.

Hence this patch is not needed, and introduced two bugs.

Gr{oetje,eeting}s,

                        Geert


--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds