[PATCH] net: dsa: vitesse-vsc73xx: Fix SPI device reference leak in vsc73xx_spi_probe()

[PATCH] net: dsa: vitesse-vsc73xx: Fix SPI device reference leak in vsc73xx_spi_probe()

[Wentao Liang]

spi_dev_get() takes a reference on the SPI device, but neither the
probe error paths nor vsc73xx_spi_remove() ever release it, so the
spi_device can never be freed after the driver has been unbound.

Release the reference on both probe error paths and in remove().

Fixes: 95711cd5f0b4 ("net: dsa: vsc73xx: Split vsc73xx driver")
Cc: stable@vger.kernel.org
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 drivers/net/dsa/vitesse-vsc73xx-spi.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/dsa/vitesse-vsc73xx-spi.c b/drivers/net/dsa/vitesse-vsc73xx-spi.c
index 85b9a0f51dd8..7b784320c41e 100644
--- a/drivers/net/dsa/vitesse-vsc73xx-spi.c
+++ b/drivers/net/dsa/vitesse-vsc73xx-spi.c
@@ -153,10 +153,15 @@ static int vsc73xx_spi_probe(struct spi_device *spi)
 	ret = spi_setup(spi);
 	if (ret < 0) {
 		dev_err(dev, "spi setup failed.\n");
+		spi_dev_put(vsc_spi->spi);
 		return ret;
 	}
 
-	return vsc73xx_probe(&vsc_spi->vsc);
+	ret = vsc73xx_probe(&vsc_spi->vsc);
+	if (ret)
+		spi_dev_put(vsc_spi->spi);
+
+	return ret;
 }
 
 static void vsc73xx_spi_remove(struct spi_device *spi)
@@ -167,6 +172,8 @@ static void vsc73xx_spi_remove(struct spi_device *spi)
 		return;
 
 	vsc73xx_remove(&vsc_spi->vsc);
+
+	spi_dev_put(vsc_spi->spi);
 }
 
 static void vsc73xx_spi_shutdown(struct spi_device *spi)
-- 
2.34.1

Re: [PATCH] net: dsa: vitesse-vsc73xx: Fix SPI device reference leak in vsc73xx_spi_probe()

[Andrew Lunn]

On Thu, Sep 17, 2026 at 10:49:20AM +0000, Wentao Liang wrote:
> spi_dev_get() takes a reference on the SPI device, but neither the
> probe error paths nor vsc73xx_spi_remove() ever release it, so the
> spi_device can never be freed after the driver has been unbound.
> 
> Release the reference on both probe error paths and in remove().
> 
> Fixes: 95711cd5f0b4 ("net: dsa: vsc73xx: Split vsc73xx driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>

Please set the Subject line to indicate net:

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

Re: [PATCH] net: dsa: vitesse-vsc73xx: Fix SPI device reference leak in vsc73xx_spi_probe()

[Linus Walleij]

On Thu, Sep 17, 2026 at 12:49 PM Wentao Liang <vulab@iscas.ac.cn> wrote:

> spi_dev_get() takes a reference on the SPI device, but neither the
> probe error paths nor vsc73xx_spi_remove() ever release it, so the
> spi_device can never be freed after the driver has been unbound.
>
> Release the reference on both probe error paths and in remove().
>
> Fixes: 95711cd5f0b4 ("net: dsa: vsc73xx: Split vsc73xx driver")
> Cc: stable@vger.kernel.org

I think we don't set this on networking patches, the network maintainer
decides himself what is stable material.

> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>

Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

Re: [PATCH] net: dsa: vitesse-vsc73xx: Fix SPI device reference leak in vsc73xx_spi_probe()

[Andrew Lunn]

On Fri, Sep 18, 2026 at 12:22:12AM +0200, Linus Walleij wrote:
> On Thu, Sep 17, 2026 at 12:49 PM Wentao Liang <vulab@iscas.ac.cn> wrote:
> 
> > spi_dev_get() takes a reference on the SPI device, but neither the
> > probe error paths nor vsc73xx_spi_remove() ever release it, so the
> > spi_device can never be freed after the driver has been unbound.
> >
> > Release the reference on both probe error paths and in remove().
> >
> > Fixes: 95711cd5f0b4 ("net: dsa: vsc73xx: Split vsc73xx driver")
> > Cc: stable@vger.kernel.org
> 
> I think we don't set this on networking patches, the network maintainer
> decides himself what is stable material.

That was true when DaveM did all the Maintainer work. But it is no
longer true. Please follow the normal stable rules.

	Andrew

Re: [PATCH] net: dsa: vitesse-vsc73xx: Fix SPI device reference leak in vsc73xx_spi_probe()

[Linus Walleij]

On Fri, Sep 18, 2026 at 3:53 AM Andrew Lunn <andrew@lunn.ch> wrote:
> On Fri, Sep 18, 2026 at 12:22:12AM +0200, Linus Walleij wrote:
> > On Thu, Sep 17, 2026 at 12:49 PM Wentao Liang <vulab@iscas.ac.cn> wrote:
> >
> > > spi_dev_get() takes a reference on the SPI device, but neither the
> > > probe error paths nor vsc73xx_spi_remove() ever release it, so the
> > > spi_device can never be freed after the driver has been unbound.
> > >
> > > Release the reference on both probe error paths and in remove().
> > >
> > > Fixes: 95711cd5f0b4 ("net: dsa: vsc73xx: Split vsc73xx driver")
> > > Cc: stable@vger.kernel.org
> >
> > I think we don't set this on networking patches, the network maintainer
> > decides himself what is stable material.
>
> That was true when DaveM did all the Maintainer work. But it is no
> longer true. Please follow the normal stable rules.

Aha! I didn't know that.

Thanks Andrew.

Yours,
Linus Walleij

Re: [PATCH] net: dsa: vitesse-vsc73xx: Fix SPI device reference leak in vsc73xx_spi_probe()

[Simon Horman]

On Fri, Sep 18, 2026 at 12:22:12AM +0200, Linus Walleij wrote:
> On Thu, Sep 17, 2026 at 12:49 PM Wentao Liang <vulab@iscas.ac.cn> wrote:
> 
> > spi_dev_get() takes a reference on the SPI device, but neither the
> > probe error paths nor vsc73xx_spi_remove() ever release it, so the
> > spi_device can never be freed after the driver has been unbound.
> >
> > Release the reference on both probe error paths and in remove().
> >
> > Fixes: 95711cd5f0b4 ("net: dsa: vsc73xx: Split vsc73xx driver")
> > Cc: stable@vger.kernel.org
> 
> I think we don't set this on networking patches, the network maintainer
> decides himself what is stable material.

Hi Linus,

That was the case in the past.
But these days following the standard stable procedure is encouraged.

Link: https://docs.kernel.org/process/maintainer-netdev.html#stable-tree

> 
> > Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> 
> Reviewed-by: Linus Walleij <linusw@kernel.org>
> 
> Yours,
> Linus Walleij

Re: [PATCH] net: dsa: vitesse-vsc73xx: Fix SPI device reference leak in vsc73xx_spi_probe()

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] Incomplete fix: the newly added spi_dev_put() calls do not cover
  the shutdown()->remove() ordering, so the reference leak the commit…

--

net: dsa: vitesse-vsc73xx: Fix SPI device reference leak in vsc73xx_spi_probe()

This adds spi_dev_put() calls to balance the spi_dev_get() done in
vsc73xx_spi_probe(), covering the two probe error paths and
vsc73xx_spi_remove().

> diff --git a/drivers/net/dsa/vitesse-vsc73xx-spi.c b/drivers/net/dsa/vitesse-vsc73xx-spi.c
> index 85b9a0f51dd89..7b784320c41e7 100644
> --- a/drivers/net/dsa/vitesse-vsc73xx-spi.c
> +++ b/drivers/net/dsa/vitesse-vsc73xx-spi.c
> @@ -153,10 +153,15 @@ static int vsc73xx_spi_probe(struct spi_device *spi)
>  	ret = spi_setup(spi);
>  	if (ret < 0) {
>  		dev_err(dev, "spi setup failed.\n");
> +		spi_dev_put(vsc_spi->spi);
>  		return ret;
>  	}
>  
> -	return vsc73xx_probe(&vsc_spi->vsc);
> +	ret = vsc73xx_probe(&vsc_spi->vsc);
> +	if (ret)
> +		spi_dev_put(vsc_spi->spi);
> +
> +	return ret;
>  }
>  
>  static void vsc73xx_spi_remove(struct spi_device *spi)
> @@ -167,6 +172,8 @@ static void vsc73xx_spi_remove(struct spi_device *spi)
>  		return;
>  
>  	vsc73xx_remove(&vsc_spi->vsc);
> +
> +	spi_dev_put(vsc_spi->spi);
>  }
>  
>  static void vsc73xx_spi_shutdown(struct spi_device *spi)

[Severity: High]
Does this still leak the spi_device reference when shutdown() runs before
remove()?

The new spi_dev_put() sits after the NULL drvdata guard in
vsc73xx_spi_remove():

	struct vsc73xx_spi *vsc_spi = spi_get_drvdata(spi);

	if (!vsc_spi)
		return;

	vsc73xx_remove(&vsc_spi->vsc);

	spi_dev_put(vsc_spi->spi);

and vsc73xx_spi_shutdown() deliberately clears drvdata without dropping
the reference:

	vsc73xx_shutdown(&vsc_spi->vsc);

	spi_set_drvdata(spi, NULL);

Documentation/networking/dsa/dsa.rst describes this ordering as permitted:

  to be called even if ``shutdown()`` was already called. Therefore, drivers are
  expected to implement a mutual exclusion method between ``remove()`` and
  ``shutdown()`` by setting their drvdata to NULL after any of these has run

So on reboot followed by SPI controller teardown, for example
dspi_shutdown() -> dspi_remove() -> spi_unregister_controller() ->
spi_unregister_device() -> device_del() -> vsc73xx_spi_remove(), the
remove callback returns early at the !vsc_spi check and the extra
reference on the spi_device (and its per-CPU spi_statistics) is never
dropped. The commit message does not mention the shutdown path.

Would it be simpler to drop the reference entirely instead of adding
matching puts, i.e. in vsc73xx_spi_probe():

	vsc_spi->spi = spi;

vsc_spi is devm_kzalloc()'d on &spi->dev, and vsc_spi->spi is only
dereferenced from vsc73xx_spi_read()/vsc73xx_spi_write() while the driver
is bound, a window in which the driver core already keeps the spi_device
alive. This file is also the only user of spi_dev_get()/spi_dev_put()
outside drivers/spi and drivers/platform/x86; peer DSA SPI drivers just
store the pointer, see b53_spi_probe() in drivers/net/dsa/b53/b53_spi.c
and ksz_spi_probe() in drivers/net/dsa/microchip/ksz_spi.c. That would
make every path, including shutdown, symmetric without adding three call
sites to keep in sync.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260917104920.2147568-1-vulab%40iscas.ac.cn