Re: [PATCH v7 02/12] PCI: liveupdate: Track outgoing preserved PCI devices

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing lock acquisition in FLB destruction callbacks allows concurrent operations to trigger a Use-After-Free.
- [High] A dangling dev->liveupdate.outgoing pointer after an aborted Live Update leads to a Use-After-Free if a subsequent Live Update is attempted.
--

--- Patch [2]: [PATCH v7 02/12] PCI: liveupdate: Track outgoing preserved PCI devices ---
commit 9a74c06de23d913aa84d2cbcc2aa3409bb1d2eb3
Author: David Matlack <dmatlack@google.com>

PCI: liveupdate: Track outgoing preserved PCI devices

This patch adds tracking for outgoing preserved PCI devices across Kexec
Handover (Live Update). It introduces APIs for drivers to notify the core
when devices are preserved or unpreserved, updating the PCI core's FLB data.

> diff --git a/drivers/pci/liveupdate.c b/drivers/pci/liveupdate.c
> index 899758883dd5d..03075ce06ac98 100644
> --- a/drivers/pci/liveupdate.c
> +++ b/drivers/pci/liveupdate.c

[ ... ]

> +static void pci_liveupdate_unpreserve_device(struct pci_flb_outgoing *outgoing, struct pci_dev *dev)
> +{
> +	struct pci_dev_ser *dev_ser = dev->liveupdate.outgoing;

[Severity: High]
If a previous Live Update was aborted without explicitly unpreserving all
devices, will dev->liveupdate.outgoing point to freed memory here?

During an abort, pci_flb_unpreserve() executes:

    WARN_ON(outgoing->ser->nr_devices);
    kho_block_set_destroy(&outgoing->block_set);

This destroys the block set backing the dev_ser allocations, but the
dev->liveupdate.outgoing pointers on the actual pci_dev structs are never
cleared.

If a subsequent Live Update is started and pci_liveupdate_unpreserve() is
called on one of those devices, could we dereference that dangling dev_ser
pointer and corrupt memory when clearing it below?

> +
> +	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");
> +	outgoing->ser->nr_devices--;
> +	memset(dev_ser, 0, sizeof(*dev_ser));
> +	dev->liveupdate.outgoing = NULL;
> +}

[ ... ]

> +void pci_liveupdate_unpreserve(struct pci_dev *dev)
> +{
> +	struct pci_flb_outgoing *outgoing = NULL;
> +
> +	guard(rwsem_write)(&pci_liveupdate.rwsem);
> +
> +	outgoing = pci_liveupdate_flb_get_outgoing();

[Severity: High]
Does the destruction path in pci_flb_unpreserve() also need to acquire
pci_liveupdate.rwsem?

If a PCI device is hot-removed concurrently with a Live Update abort:

Thread 1 (driver remove):
pci_liveupdate_unpreserve()
    guard(rwsem_write)(&pci_liveupdate.rwsem);
    outgoing = pci_liveupdate_flb_get_outgoing();
    ... begins modifying outgoing and dev_ser

Thread 2 (Live Update core abort):
pci_flb_unpreserve()
    kho_block_set_destroy(&outgoing->block_set);
    kho_unpreserve_free(outgoing->ser);
    kfree(outgoing);

If pci_flb_unpreserve() destroys the outgoing FLB without acquiring the
semaphore, could Thread 1 end up writing to freed memory?

> +	if (IS_ERR(outgoing)) {
> +		pci_warn(dev, "Cannot unpreserve device without outgoing Live Update state\n");
> +		return;
> +	}
> +
> +	pci_liveupdate_unpreserve_device(outgoing, dev);
> +}
> +EXPORT_SYMBOL_GPL(pci_liveupdate_unpreserve);

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