Re: [PATCH v5 02/11] PCI: liveupdate: Track outgoing preserved PCI devices

[David Matlack <dmatlack@google.com>]

On 2026-05-14 12:31 AM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Critical] Dangling pointer in dev->liveupdate.outgoing causes Use-After-Free memory corruption and state leaks if a device is not properly unpreserved.
> - [High] Missing `kho_is_enabled()` check in `pci_flb_preserve()` violates KHO subsystem guidelines.
> --
> 
> commit f946f5d8b0614325899345e29b5315ff6d5139e9
> Author: David Matlack <dmatlack@google.com>
> 
> PCI: liveupdate: Track outgoing preserved PCI devices
> 
> This patch adds APIs to allow drivers to notify the PCI core of which devices
> are being preserved across a Live Update. It allocates a struct pci_ser via
> the KHO subsystem to track the outgoing preserved devices.
> 
> > diff --git a/drivers/pci/liveupdate.c b/drivers/pci/liveupdate.c
> > --- a/drivers/pci/liveupdate.c
> > +++ b/drivers/pci/liveupdate.c
> 
> [ ... ]
> 
> > @@ -74,27 +110,30 @@ static int pci_flb_preserve(struct liveupdate_flb_op_args *args)
> >  
> >  	size = struct_size_t(struct pci_ser, devices, max_nr_devices);
> >  
> > -	ser = kho_alloc_preserve(size);
> > -	if (IS_ERR(ser))
> > -		return PTR_ERR(ser);
> > +	outgoing->ser = kho_alloc_preserve(size);
> 
> Does this allocation need to be gated by kho_is_enabled()?

No, if kho_is_enabled() is false then liveupdate_register_flb() would
have returned -EOPNOTSUPP and thus this callback would never be run.

> 
> If KHO is disabled, kho_alloc_preserve() will silently add tracking state
> that will never be used.
> 
> > +	if (IS_ERR(outgoing->ser)) {
> > +		kfree(outgoing);
> > +		return PTR_ERR(outgoing->ser);
> > +	}
> >  
> >  	pr_debug("Preserved struct pci_ser with room for %u devices\n",
> >  		 max_nr_devices);
> 
> [ ... ]
> 
> >  static void pci_flb_unpreserve(struct liveupdate_flb_op_args *args)
> >  {
> > -	struct pci_ser *ser = args->obj;
> > +	struct pci_flb_outgoing *outgoing = args->obj;
> >  
> > -	WARN_ON_ONCE(ser->nr_devices);
> > -	kho_unpreserve_free(ser);
> > +	WARN_ON_ONCE(outgoing->ser->nr_devices);
> > +	kho_unpreserve_free(outgoing->ser);
> > +	kfree(outgoing);
> 
> If a driver fails to unpreserve a device before the FLB is torn down, this
> WARN_ON_ONCE triggers, but the memory is still freed.
> 
> Does this leave dev->liveupdate.outgoing pointing to freed memory?

Yes, if a driver unpreserves a device without notifying the PCI core
then that would lead to the PCI core having a pointer to freed memory.

I can make this code more robust against buggy drivers in v6.

> 
> >  
> >  	pr_debug("Unpreserved struct pci_ser\n");
> >  }
> 
> [ ... ]
> 
> > +void pci_liveupdate_unpreserve(struct pci_dev *dev)
> > +{
> > +	struct pci_flb_outgoing *outgoing = NULL;
> > +	struct pci_dev_ser *dev_ser;
> > +	struct pci_ser *ser;
> > +	int ret;
> > +
> > +	ret = liveupdate_flb_get_outgoing(&pci_liveupdate_flb, (void **)&outgoing);
> > +
> > +	if (ret || !outgoing) {
> > +		pci_warn(dev, "Cannot unpreserve device without outgoing Live Update state\n");
> > +		return;
> > +	}
> > +
> > +	guard(mutex)(&outgoing->lock);
> > +	ser = outgoing->ser;
> > +
> > +	guard(write_lock)(&dev->liveupdate.lock);
> > +
> > +	dev_ser = dev->liveupdate.outgoing;
> > +	if (!dev_ser) {
> > +		pci_warn(dev, "Cannot unpreserve device that is not preserved\n");
> > +		return;
> > +	}
> > +
> > +	pci_info(dev, "Device will no longer be preserved across next Live Update\n");
> > +	ser->nr_devices--;
> > +	memset(dev_ser, 0, sizeof(*dev_ser));
> 
> If dev->liveupdate.outgoing was left dangling from a previous torn-down
> session, can this corrupt memory?
> 
> If a new Live Update session starts, pci_liveupdate_unpreserve() would
> fetch the new outgoing FLB but operate on the old dangling
> dev->liveupdate.outgoing pointer.
> 
> Does this code cause a use-after-free in the memset() here, as well as
> corrupt the new session's counter when decrementing ser->nr_devices?

I will clean up any dangling pointers during pci_flb_unpreserve() in v6
to protect the PCI core against buggy drivers.

> 
> > +	dev->liveupdate.outgoing = NULL;
> > +}
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260512184846.119396-1-dmatlack@google.com?part=2