Re: [PATCH v10 11/13] PCI: liveupdate: Freeze preservation status during shutdown

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] pci: dangling outgoing pointer on concurrent unpreserve during shutdown
--

commit 87d44fc8f0a5a6587310a2bfb5c312e9de1fe1c8
Author: David Matlack <dmatlack@google.com>

PCI: liveupdate: Freeze preservation status during shutdown

Freeze a device's outgoing preservation status (preserved or not
preserved) during shutdown. This enables the PCI core and drivers to
safely make decisions based on the device's preservation status during
shutdown.

> diff --git a/drivers/pci/liveupdate.c b/drivers/pci/liveupdate.c
> index e640794cc8e6b..38b7815fe7d4a 100644
> --- a/drivers/pci/liveupdate.c
> +++ b/drivers/pci/liveupdate.c
[ ... ]
> @@ -499,6 +499,11 @@ static int pci_liveupdate_unpreserve_device(struct pci_flb_outgoing *outgoing,
>  {
>  	struct pci_dev_ser *dev_ser = dev->liveupdate.outgoing;
>  
> +	if (dev->liveupdate.frozen) {
> +		pci_warn(dev, "Cannot unpreserve device after it is frozen!\n");
> +		return -EINVAL;
> +	}
> +

[Severity: High]
If a user task exits or closes the Live Update session fd during system shutdown,
the LU core tears down the preservation session.

When dev->liveupdate.frozen is true, pci_liveupdate_unpreserve_device()
returns -EINVAL and leaves dev->liveupdate.outgoing pointing to the FLB memory.

However, the LU core ignores this failure and unconditionally drops the FLB
reference:

kernel/liveupdate/luo_file.c:luo_file_unpreserve_files() {
	...
	luo_file->fh->ops->unpreserve(&args);
	luo_flb_file_unpreserve(luo_file->fh);
	...
}

This invokes pci_flb_unpreserve() which destroys the KHO state and frees the
memory:

drivers/pci/liveupdate.c:pci_flb_unpreserve() {
	...
	WARN_ON(outgoing->ser->nr_devices);
	kho_block_set_destroy(&outgoing->block_set);
	kho_unpreserve_free(outgoing->ser);
	kfree(outgoing);
}

Will this leave a dangling pointer in dev->liveupdate.outgoing?

Since pci_device_shutdown() relies on this pointer to skip disabling Bus Master,
could this result in the old kernel leaving DMA enabled while the next kernel
treats the device as unpreserved, creating a risk of memory corruption?

>  	if (!dev_ser) {
>  		pci_warn(dev, "Cannot unpreserve device that is not preserved\n");
>  		return -EINVAL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261013162529.1928093-1-dmatlack@google.com?part=11