Re: [PATCH v2 12/13] PCI/P2PDMA: Add KUnit coverage for the ACS P2P routing walk

[Leon Romanovsky <leon@kernel.org>]

On Thu, Aug 06, 2026 at 11:44:32AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] pci_p2pdma_release() skips synchronize_rcu() when p2pdma->pool is NULL, leading to a Use-After-Free of the pci_p2pdma struct and its XArray inside RCU read-side critical sections.
> --
> 
> --- Patch [12]: [PATCH v2 12/13] PCI/P2PDMA: Add KUnit coverage for the ACS P2P routing walk ---
> commit c4a68b62f4ec894cc35240eda46bf42dc5f562d9
> Author: Leon Romanovsky <leon@kernel.org>
> 
> PCI/P2PDMA: Add KUnit coverage for the ACS P2P routing walk
> 
> Extend the ACS KUnit suite with end-to-end coverage of
> calc_map_type_and_dist(), the provider-to-client hierarchy walk.
> 
> > diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> > index c7680964d8a64..18e5339bc49e9 100644
> > --- a/drivers/pci/p2pdma.c
> > +++ b/drivers/pci/p2pdma.c
> [ ... ]
> > @@ -859,6 +859,7 @@ calc_map_type_and_dist(struct pci_dev *provider, struct pci_dev *client,
> >  	rcu_read_unlock();
> >  	return map_type;
> >  }
> > +EXPORT_SYMBOL_IF_KUNIT(calc_map_type_and_dist);
> 
> [Severity: High]
> This is a pre-existing issue, but does this code have a race condition
> leading to a use-after-free when interacting with pci_p2pdma_release()?
> 
> If a device acts as a P2PDMA provider without setting up an allocator pool
> (for example, via vfio-pci or mlx5), pci_p2pdma_release() skips
> synchronize_rcu() because p2pdma->pool is NULL:
> 
> drivers/pci/p2pdma.c:pci_p2pdma_release() {
>         ...
>         if (p2pdma->pool)
>                 synchronize_rcu();
>         ...
> }
> 
> When the provider driver is unbound, devres frees the pci_p2pdma structure
> and destroys the XArray. Concurrently, calc_map_type_and_dist() executing
> here inside its RCU read-side critical section can dereference the freed
> p2pdma pointer and call xa_store() on the destroyed XArray.
> 
> Should synchronize_rcu() be called during release regardless of whether
> the allocator pool is set up?

No, we already discussed that here.
https://lore.kernel.org/kvm/20260729100840.GM12003@unreal/
https://lore.kernel.org/kvm/20260805003127.GJ27883@nvidia.com/

But let's see if I can write a documentation patch that helps AI
tools stop complaining.

Thanks

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260806-fix-p2p-acs-v2-0-0cec14812965@nvidia.com?part=12
>