Re: [PATCH v8 03/12] PCI: liveupdate: Track incoming preserved PCI devices

On Tue, Jul 28, 2026 at 3:22 PM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
> - [High] Unconditional kernel panic in the incoming FLB finish path if a preserved PCI device is missing or fails setup.
> - [High] `pci_flb_unpreserve()` destroys outgoing FLB memory without clearing device pointers, leading to a Use-After-Free.
> - [High] Transient errors or version mismatches during FLB retrieval cause an immediate kernel panic on device setup.
> - [Medium] Leaking preserved KHO memory for `ser->devices` on early error paths in `pci_flb_retrieve()`.
> --
>
> --- Patch [3]: [PATCH v8 03/12] PCI: liveupdate: Track incoming preserved PCI devices ---
> commit 8fdc703d6dd44076cd03589af0bcffa13c79e7f4
> Author: David Matlack <[email protected]>
>
> PCI: liveupdate: Track incoming preserved PCI devices
>
> This patch allows the PCI core to track incoming preserved PCI devices
> during enumeration. It fetches the serialized state from the Live Update
> Orchestrator and exposes APIs for drivers to handle incoming devices.
>
> > diff --git a/drivers/pci/liveupdate.c b/drivers/pci/liveupdate.c
> > index b003b7069cdb0..5ce5f8b369029 100644
> > --- a/drivers/pci/liveupdate.c
> > +++ b/drivers/pci/liveupdate.c
> [ ... ]
> > @@ -140,15 +174,91 @@ static int pci_flb_retrieve(struct liveupdate_flb_op_args *args)
> >  {
> > +     struct pci_ser *ser = phys_to_virt(args->data);
> > +     struct pci_flb_incoming *incoming;
> > +     struct pci_dev_ser *dev_ser;
> > +     struct kho_block_set_it it;
> > +     int ret;
> >
> >       pr_debug("Retrieving struct pci_ser (0x%llx)\n", args->data);
> > -     args->obj = phys_to_virt(args->data);
> > +
> > +     if (ser->version != PCI_LUO_FLB_VERSION) {
> > +             pr_err("Incoming PCI FLB version (v%d) is incompatible with this kernel (v%d)\n",
> > +                    ser->version, PCI_LUO_FLB_VERSION);
> > +             ret = -EINVAL;
> > +             goto err_restore_free;
> > +     }
> > +
> > +     incoming = kzalloc_obj(*incoming);
> > +     if (!incoming) {
> > +             ret = -ENOMEM;
> > +             goto err_restore_free;
> > +     }
> [ ... ]
> > +err_restore_free:
> > +     kho_restore_free(ser);
> > +     return ret;
> > +}
>
> [Severity: Medium]
> Does this error path leak the KHO memory for ser->devices?
>
> When jumping to err_restore_free (e.g. due to a version mismatch or kzalloc
> failure), kho_restore_free(ser) is called, but it doesn't appear to restore
> or free the memory pointed to by ser->devices.
>
> Since KHO pages are reserved across kexec, will this permanently leak the
> memory for the uptime of the successor kernel?

Yes, but PCI FLB retrieve errors are fatal (panic).

>
> [ ... ]
> > +static void pci_check_all_devices_finished(struct pci_flb_incoming *incoming)
> > +{
> > +     struct pci_dev *dev = NULL;
> > +
> > +     if (READ_ONCE(incoming->ser->nr_devices) == 0)
> > +             return;
> > +
> > +     for_each_pci_dev(dev) {
> > +             if (READ_ONCE(dev->liveupdate.incoming))
> > +                     pci_emerg(dev, "Preserved device was never finished!\n");
> > +     }
> > +
> > +     /*
> > +      * This should only happen if a driver violated the contract to call
> > +      * pci_liveupdate_finish() (something is extremely broken).
> > +      */
> > +     panic("Some preserved devices were never finished!\n");
> > +}
>
> [Severity: High]
> Is an unconditional panic here safe if a preserved device is missing or
> fails to probe?
>
> If a PCI device that was preserved by the previous kernel fails to probe in
> the new kernel (e.g. the driver module is missing, or it encounters a
> transient error), pci_liveupdate_finish() will never be called and
> incoming->ser->nr_devices will not be decremented.
>
> When the Live Update orchestrator concludes the update and calls
> pci_flb_finish(), pci_check_all_devices_finished() is invoked and sees
> that nr_devices is still > 0. Does this mean normal system conditions like
> a missing driver will trigger a kernel panic?

The file associated with a device that fails to be enumerated or
probed cannever be retrieved by userspace, and therefore never go
through its finish() callback. So LUO will never call
pci_flb_finish(). This panic will only happen if there is a serious
bug in the driver or LUO.

>
> [ ... ]
> > @@ -325,6 +435,75 @@ void pci_liveupdate_unpreserve(struct pci_dev *dev)
> >  }
> >  EXPORT_SYMBOL_GPL(pci_liveupdate_unpreserve);
> >
> > +static struct pci_flb_incoming *pci_liveupdate_flb_get_incoming(void)
> > +{
> > +     struct pci_flb_incoming *incoming = NULL;
> > +     int ret;
> [ ... ]
> > +     /*
> > +      * There is incoming FLB data that matches pci_liveupdate_flb.compatible
> > +      * but retrieve failed (pci_flb_retrieve() returned an error or LUO
> > +      * failed to acquire a reference to pci_liveupdate_flb_ops.owner).
> > +      */
> > +     if (ret)
> > +             panic("Failed to retrieve incoming FLB data (%d)\n", ret);
> > +
> > +     return incoming;
> > +}
>
> [Severity: High]
> Does this panic prevent the new kernel from booting if there's a simple
> FLB version mismatch?
>
> During PCI enumeration in the new kernel, pci_setup_device() calls
> pci_liveupdate_setup_device(), which invokes
> pci_liveupdate_flb_get_incoming().
>
> If pci_flb_retrieve() encounters an error like a version mismatch (-EINVAL)
> or an allocation failure (-ENOMEM), it returns that error code. This branch
> then catches the error and unconditionally panics. Will this reliably crash
> the new kernel during early boot and break the kexec handover fallback
> mechanism?

Yes all FLB retrieve errors are considered fatal. A version mismatch
is considered a retrieve error because it means the current kernel
can't understand the data passed by the previous kernel.

We do want more graceful versioning support to handle
upgrade/downgrade scenarios, but that requires deeper design work in
LUO before the PCI core can use it. We aren't going to build a
PCI-specific versioning solution.

>
> [ ... ]
>
> I also have a question about the unpreserve path in pci_flb_unpreserve()
> which might lead to a regression:
>
> 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);
> }
>
> [Severity: High]
> Does pci_flb_unpreserve() destroy outgoing FLB memory without clearing the
> device pointers?
>
> If a driver fails to unpreserve its devices before the orchestrator aborts a
> Live Update, pci_flb_unpreserve() will issue a WARN_ON but proceed to
> destroy the block set and free the outgoing FLB object.
>
> The dev->liveupdate.outgoing pointers on the pci_dev structs are left
> dangling. If a new Live Update is subsequently initiated and the driver
> attempts to preserve the device again, pci_liveupdate_preserve_device()
> will access dev->liveupdate.outgoing->refcount via the dangling pointer:
>
> drivers/pci/liveupdate.c:pci_liveupdate_preserve_device() {
>     ...
>     if (dev->liveupdate.outgoing) {
>         if (!dev->liveupdate.outgoing->refcount) {
>     ...
> }
>
> Can this lead to a use-after-free of the KHO block set memory?

Yes. We can handle that the same way as dangling incoming pointers and
panic. This will only happen if the driver or LUO are very broken.

>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3