Re: [PATCH 2/2] PCI: setup-res: Guard against bus->self == NULL in _pci_assign_resource()

[hugo lee]

On Thu, Jun 11, 2026 at 5:57 PM Ilpo Järvinen
<ilpo.jarvinen@xxxxxxxxxxxxxxx> wrote:
>
> On Wed, 10 Jun 2026, Yuguo Li wrote:
>
> > _pci_assign_resource() walks up the parent buses looking for a
> > transparent bridge to retry resource allocation against.  The
> > termination check dereferences bus->self->transparent without first
> > testing bus->self.
> >
> > For SR-IOV virtual buses created by virtfn_add_bus() via
> > pci_add_new_bus(parent, NULL, busnr) -- which happens when a VF lands
> > on a bus number different from its PF -- bus->self is NULL.  When
> > __pci_assign_resource() is invoked on such a VF (e.g. via
> > pci_assign_resource() from userspace-triggered LTP coverage) and the
> > allocation fails on the first iteration, the !bus->parent test passes
> > because the virtual bus does have a parent, and the next term then
> > NULL-derefs bus->self.
> >
> > Add an explicit !bus->self check, mirroring the established pattern
> > elsewhere in drivers/pci/ (e.g. pci.c, probe.c, pciehp_hpc.c).
> >
> > Reproduced on mainline 7.1.0-rc7+ on x86_64 with an SR-IOV PF whose
> > VFs span multiple bus numbers, by triggering pci_assign_resource() on
> > a VF that lives on a virtual bus:
> >
> >     BUG: kernel NULL pointer dereference, address: 0000000000000860
> >     RIP: 0010:_pci_assign_resource+0x63/0x130
> >     Call Trace:
> >      pci_assign_resource+0xe9/0x370
> >      ... (LTP tpci test-case 12 driving pci_assign_resource via sysfs)
>
> Hi,
>
> And who called _*pci_assign_resource(), etc.? I actually wanted to check
> the code... :-( Please don't strip PCI core related parts of the
> callchain.
>

Hi Ilpo,
Thanks for the review.  A clarification on the call chain, results
of running your suggested form, and a question on the right shape.

1. Call chain
The "..." in last mail was LTP module frames and the syscall layer,
not PCI core frames -- the chain hits _pci_assign_resource() in one
core hop :
    pci_assign_resource+0xe9/0x370
    test_case+0x202/0x6e0       [ltp_tpci]
    sys_tcase+0x51/0x80         [ltp_tpci]
    kernfs_fop_write_iter+0x117/0x1f0
    vfs_write+0x28d/0x450
    ksys_write+0x69/0xe0
    do_syscall_64+0xab/0x500

The trigger is upstream LTP test_pci. Patch 1 is reached via
sub-case BUS_SCAN (case 6, pci_rescan_bus()). Patch 2
is reached via sub-case PCI_RESOURCES(case 12), which on
every prefetchable MMIO BAR runs:

    pci_release_resource(dev, i);
    ret = pci_assign_resource(dev, i);

https://github.com/linux-test-project/ltp/blob/d631e9c/testcases/kernel/device-drivers/pci/tpci_kernel/ltp_tpci.c#L428

> >      do_syscall_64+0xab/0x500
> >
> > This is the same SR-IOV-virtual-bus / self == NULL pattern fixed for
> > pci_read_bridge_bases() in commit ("PCI: Bail out of
> > pci_read_bridge_bases() for SR-IOV virtual buses").
> >
> > Fixes: d09ee9687e02 ("PCI: improve resource allocation under transparent bridges")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Yuguo Li <hugoolli@xxxxxxxxxxx>
> > ---
> >  drivers/pci/setup-res.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
> > index 991d3ed543f5..e8bd3d4ff923 100644
> > --- a/drivers/pci/setup-res.c
> > +++ b/drivers/pci/setup-res.c
> > @@ -353,7 +353,7 @@ static int _pci_assign_resource(struct pci_dev *dev, int resno,
> >
> >       bus = dev->bus;
> >       while ((ret = __pci_assign_resource(bus, dev, resno, size, min_align))) {
> > -             if (!bus->parent || !bus->self->transparent)
> > +             if (!bus->parent || !bus->self || !bus->self->transparent)
>
> I'd like to know if this should not break in case of !bus->self because
> I couldn't immediately find the code which would add window resources to
> the VF virtual bus so I'm not convinced this check is right.
>
> __pci_assign_resource() obviously must have failed to assign the resource
> if you can trigger NULL deref here so that hints VF bus does not have the
> resource that could parent dev's resource present (another explanation
> could be that it just doesn't fit there but I cannot say it based on the
> limited information).
>
> Did the resource fail to assign? What is the resulting resource tree with
> this change?
>
> The more approriate check might be this one:
>
>                 if (!bus->parent || (bus->self && !bus->self->transparent))
>
> --
>  i.
>

2.SRIOV VF and pci resource management

Yes, resource failed to assign. Confirmed on a system carrying
these two patches: cross-bus VFs end up with resource[0] either
0/0/0 or "start=0, end=size-1, flags preserved" (the residue
pci_release_resource() leaves), while same-bus VFs on the same
PF remain assigned, but the result comes from the generic allocator
on the PF bus, not from the SR-IOV slice that virtfn_add_bus()
set up. Either way, dev->resource[] no longer reflects what the
hardware decodes.

VF resources don't normally come from _pci_assign_resource(). At
probe time, pci_iov_add_virtfn() programs them as a deterministic
slice of the PF VF BAR window. So it is inappropriate to call
pci_release_resource()/pci_assign_resource() for a VF.

Maybe we should add an early bail-out on dev->is_virtfn at
the top of both -- with -EBUSY from _pci_assign_resource(), which
ltp_tpci already treats as TPASS, and keep dev->resource[] consistent
with the probe-time slice.