Re: [PATCH v5 06/11] PCI: liveupdate: Auto-preserve upstream bridges across Live Update

[David Matlack <dmatlack@google.com>]

On 2026-05-14 02:05 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Kernel stack overflow via deep recursion in `pci_liveupdate_preserve_path()`.
> --
> 
> commit 9dcae64241d06ee18edeb26c25b9a7b263aa05a4
> Author: David Matlack <dmatlack@google.com>
> 
> PCI: liveupdate: Auto-preserve upstream bridges across Live Update
> 
> This commit introduces tracking to auto-preserve upstream PCI bridges when a
> device is preserved across a Live Update. It adds functions to preserve,
> unpreserve, and finish the preservation of devices and recursively applies
> these to upstream bridges up to the root port.
> 
> > diff --git a/drivers/pci/liveupdate.c b/drivers/pci/liveupdate.c
> > index 558fbaec8dddd..d8e06afde2c76 100644
> > --- a/drivers/pci/liveupdate.c
> > +++ b/drivers/pci/liveupdate.c
> 
> [ ... ]
> 
> > @@ -363,8 +389,82 @@ int pci_liveupdate_preserve(struct pci_dev *dev)
> 
> [ ... ]
> 
> > +static int pci_liveupdate_preserve_path(struct pci_ser *ser, struct pci_dev *dev)
> > +{
> > +	int ret;
> > +
> > +	if (!dev)
> > +		return 0;
> > +
> > +	ret = pci_liveupdate_preserve_device(ser, dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ret = pci_liveupdate_preserve_path(ser, dev->bus->self);
> 
> Can this recursion cause a kernel stack overflow?
> 
> The PCI specification allows up to 256 nested buses in a single domain, which
> can occur with deep Thunderbolt or USB4 topologies.
> 
> Because the recursive call to pci_liveupdate_preserve_path() is followed by a
> potential call to pci_liveupdate_unpreserve_device(), it cannot be optimized
> as a tail call. This forces a new stack frame for every level of the PCI
> hierarchy, which could overflow the standard kernel stack, especially with
> KASAN enabled, and cause a crash.
> 
> Could this be rewritten to use an iterative approach instead?

I used recursion to simplify the code and based on the pre-existing
examples of using recursion in the PCI core, e.g.
pci_scan_bridge_extend(). But I will take a look at an iterative
approach for v6.

> 
> > +	if (ret) {
> > +		pci_liveupdate_unpreserve_device(ser, dev);
> > +		return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> 
> [ ... ]
> 
> > +static void pci_liveupdate_unpreserve_path(struct pci_ser *ser, struct pci_dev *dev)
> > +{
> > +	if (!dev)
> > +		return;
> > +
> > +	if (pci_liveupdate_unpreserve_device(ser, dev))
> > +		return;
> > +
> > +	pci_liveupdate_unpreserve_path(ser, dev->bus->self);
> 
> Does this code have the same recursion issue as pci_liveupdate_preserve_path()
> and require an iterative approach?

See above.

> 
> > +}
> 
> [ ... ]
> 
> > @@ -519,28 +656,18 @@ void pci_liveupdate_cleanup_device(struct pci_dev *dev)
> 
> [ ... ]
> 
> >  void pci_liveupdate_finish(struct pci_dev *dev)
> >  {
> > -	guard(write_lock)(&dev->liveupdate.lock);
> > -
> > -	if (!dev->liveupdate.incoming) {
> > -		pci_warn(dev, "Cannot finish preserving an unpreserved device\n");
> > +	if (!dev)
> >  		return;
> > -	}
> > -
> > -	pci_info(dev, "Device is finished participating in Live Update\n");
> >  
> > -	/*
> > -	 * Drop the refcount so this device does not get treated as an incoming
> > -	 * device again, e.g. in case pci_liveupdate_setup_device() gets called
> > -	 * again because the device is hot-plugged.
> > -	 */
> > -	dev->liveupdate.incoming->refcount = 0;
> > -	dev->liveupdate.incoming = NULL;
> > +	if (pci_liveupdate_finish_device(dev))
> > +		return;
> >  
> > -	/* Drop this device's reference on the incoming FLB. */
> > -	pci_liveupdate_flb_put_incoming();
> > +	pci_liveupdate_finish(dev->bus->self);
> 
> Does this code also have the same recursion issue?

See above.

> 
> >  }
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260512184846.119396-1-dmatlack@google.com?part=6