[PATCH] PCI/P2PDMA: Use RCU_INIT_POINTER() to clear pdev->p2pdma

[PATCH] PCI/P2PDMA: Use RCU_INIT_POINTER() to clear pdev->p2pdma

[lirongqing]

From: Li RongQing <lirongqing@baidu.com>

Since pdev->p2pdma is an RCU-protected pointer, directly assigning
NULL to it violates RCU semantics and triggers sparse static analysis
warnings.

Fix this by using RCU_INIT_POINTER() instead of a raw assignment,
ensuring proper RCU annotations and consistency with the subsequent
synchronize_rcu() call.

Signed-off-by: Li RongQing <lirongqing@baidu.com>
---
 drivers/pci/p2pdma.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
index 7c89854..fd986242 100644
--- a/drivers/pci/p2pdma.c
+++ b/drivers/pci/p2pdma.c
@@ -236,7 +236,7 @@ static void pci_p2pdma_release(void *data)
 		return;
 
 	/* Flush and disable pci_alloc_p2p_mem() */
-	pdev->p2pdma = NULL;
+	RCU_INIT_POINTER(pdev->p2pdma, NULL);
 	if (p2pdma->pool)
 		synchronize_rcu();
 	xa_destroy(&p2pdma->map_types);
-- 
2.9.4

Re: [PATCH] PCI/P2PDMA: Use RCU_INIT_POINTER() to clear pdev->p2pdma

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] pci_p2pdma_release() skips synchronize_rcu() when p2pdma->pool is NULL, causing a use-after-free of the devm-allocated p2pdma structure and corruption of its map_types xarray for concurrent RCU readers.
- [High] Missing devres action removal in pci_p2pdma_add_resource() error paths causes Use-After-Free of p2p_pgmap during devres teardown.
--

commit eb4ec2c32d602512abbeccbe720637cf83f85cb6
Author: Li RongQing <lirongqing@baidu.com>

PCI/P2PDMA: Use RCU_INIT_POINTER() to clear pdev->p2pdma

Since pdev->p2pdma is an RCU-protected pointer, directly assigning
NULL to it violates RCU semantics and triggers sparse static analysis
warnings.

Fix this by using RCU_INIT_POINTER() instead of a raw assignment,
ensuring proper RCU annotations and consistency with the subsequent
synchronize_rcu() call.

> diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> index 7c898542af8d5..fd98624208c50 100644
> --- a/drivers/pci/p2pdma.c
> +++ b/drivers/pci/p2pdma.c
> @@ -236,7 +236,7 @@ static void pci_p2pdma_release(void *data)
>  		return;
>  
>  	/* Flush and disable pci_alloc_p2p_mem() */
> -	pdev->p2pdma = NULL;
> +	RCU_INIT_POINTER(pdev->p2pdma, NULL);
>  	if (p2pdma->pool)
>  		synchronize_rcu();

[Severity: High]
This isn't a bug introduced by this patch, but could skipping
synchronize_rcu() here result in a use-after-free for concurrent RCU readers?

If p2pdma->pool is NULL, synchronize_rcu() is bypassed before destroying the
map_types xarray and returning. Since the p2pdma structure is managed by
devres, it will be freed shortly after this callback returns.

If concurrent RCU readers (like those executing pci_p2pdma_map_type()) obtain
the pointer just before it is cleared, will they continue to access the freed
map_types xarray and p2pdma structure?

>  	xa_destroy(&p2pdma->map_types);


While reviewing this file, I noticed another pre-existing issue in
pci_p2pdma_add_resource():

drivers/pci/p2pdma.c:pci_p2pdma_add_resource()
{
[ ... ]
	error = devm_add_action_or_reset(&pdev->dev, pci_p2pdma_unmap_mappings,
					 p2p_pgmap);
	if (error)
		goto pages_free;

	p2pdma = rcu_dereference_protected(pdev->p2pdma, 1);
	error = gen_pool_add_owner(p2pdma->pool, (unsigned long)addr,
			pci_bus_address(pdev, bar) + offset,
			range_len(&pgmap->range), dev_to_node(&pdev->dev),
			&pgmap->ref);
	if (error)
		goto pages_free;
[ ... ]
pages_free:
	devm_memunmap_pages(&pdev->dev, pgmap);
pgmap_free:
	devm_kfree(&pdev->dev, p2p_pgmap);

[Severity: High]
This is also a pre-existing issue, but are we leaving a dangling pointer in
the devres list here?

If gen_pool_add_owner() fails, the code jumps to the pages_free label and
explicitly frees p2p_pgmap via devm_kfree(). However, the devres action
pci_p2pdma_unmap_mappings is not removed.

During devres teardown, the registered action will execute and call
pci_p2pdma_unmap_mappings() with the freed p2p_pgmap pointer. This
dereferences p2p_pgmap->mem->owner->kobj, resulting in a use-after-free.

Would it be safer to use devm_remove_action() here to prevent this?

	return error;
}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612083452.1639-1-lirongqing@baidu.com?part=1

Re: [PATCH] PCI/P2PDMA: Use RCU_INIT_POINTER() to clear pdev->p2pdma

[Leon Romanovsky]

On Fri, Jun 12, 2026 at 08:48:24AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> 
> Pre-existing issues:
> - [High] pci_p2pdma_release() skips synchronize_rcu() when p2pdma->pool is NULL, causing a use-after-free of the devm-allocated p2pdma structure and corruption of its map_types xarray for concurrent RCU readers.
> - [High] Missing devres action removal in pci_p2pdma_add_resource() error paths causes Use-After-Free of p2p_pgmap during devres teardown.
> --
> 
> commit eb4ec2c32d602512abbeccbe720637cf83f85cb6
> Author: Li RongQing <lirongqing@baidu.com>
> 
> PCI/P2PDMA: Use RCU_INIT_POINTER() to clear pdev->p2pdma
> 
> Since pdev->p2pdma is an RCU-protected pointer, directly assigning
> NULL to it violates RCU semantics and triggers sparse static analysis
> warnings.
> 
> Fix this by using RCU_INIT_POINTER() instead of a raw assignment,
> ensuring proper RCU annotations and consistency with the subsequent
> synchronize_rcu() call.
> 
> > diff --git a/drivers/pci/p2pdma.c b/drivers/pci/p2pdma.c
> > index 7c898542af8d5..fd98624208c50 100644
> > --- a/drivers/pci/p2pdma.c
> > +++ b/drivers/pci/p2pdma.c
> > @@ -236,7 +236,7 @@ static void pci_p2pdma_release(void *data)
> >  		return;
> >  
> >  	/* Flush and disable pci_alloc_p2p_mem() */
> > -	pdev->p2pdma = NULL;
> > +	RCU_INIT_POINTER(pdev->p2pdma, NULL);
> >  	if (p2pdma->pool)
> >  		synchronize_rcu();
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but could skipping
> synchronize_rcu() here result in a use-after-free for concurrent RCU readers?
> 
> If p2pdma->pool is NULL, synchronize_rcu() is bypassed before destroying the
> map_types xarray and returning. Since the p2pdma structure is managed by
> devres, it will be freed shortly after this callback returns.
> 
> If concurrent RCU readers (like those executing pci_p2pdma_map_type()) obtain
> the pointer just before it is cleared, will they continue to access the freed
> map_types xarray and p2pdma structure?
> 
> >  	xa_destroy(&p2pdma->map_types);

I do not believe this is a bug. The p2pdma->pool field is NULL only for
in-kernel users that fully control the lifecycle of their drivers.
In that mode, there should not be any concurrent RCU readers when the
device is released.

Thanks