mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-08-30 13:23:02 -04:00
net: page_pool: fix UAF in __page_pool_release_netmem_dma on xa_cmpxchg race
This bug was discovered while testing the hns3 driver under channel
reconfiguration (`ethtool -L` / `ethtool -G`) with iperf3 traffic on
arm64. The race is intermittently triggered when page_pool_destroy()
runs page_pool_scrub() concurrently with page return via
page_pool_put_netmem() on a different CPU. A WARN in
page_pool_clear_pp_info() surfaced the dangling DMA index bits left
by the cmpxchg loser, which led to the investigation.
page_pool_scrub() iterates pool->dma_mapped via xa_for_each() with no
page ref held. __page_pool_release_netmem_dma() currently reads and
writes netmem fields (dma_addr, DMA index bits in pp_magic) after
xa_cmpxchg() returns. The unref path calls put_page() unconditionally
regardless of the cmpxchg outcome; when it loses the cmpxchg, it still
frees the page before the scrub winner finishes these netmem accesses,
so scrub touches a freed page -- a Use-After-Free.
Fix this by splitting the DMA release into two functions:
1. __page_pool_unmap_netmem_dma() caches dma_addr before xa_cmpxchg(),
does the cmpxchg to remove the DMA mapping, and calls dma_unmap on
the cached address. It never touches netmem fields after the cmpxchg,
making it safe for the scrub path which holds no page ref.
2. __page_pool_release_netmem_dma() wraps the above and additionally
clears dma_addr and DMA index bits in netmem fields. This is safe
only when the caller holds a page ref, so it is used by the return
path (page_pool_return_netmem).
The scrub path calls __page_pool_unmap_netmem_dma() directly; the return
path calls __page_pool_release_netmem_dma().
Fixes: ee62ce7a1d ("page_pool: Track DMA-mapped pages and unmap them when destroying the pool")
Suggested-by: Mina Almasry <almasrymina@google.com>
Reviewed-by: Mina Almasry <almasrymina@google.com>
Signed-off-by: Jijie Shao <shaojijie@huawei.com>
Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
Link: https://patch.msgid.link/20260807114830.344336-1-shaojijie@huawei.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
committed by
Jakub Kicinski
parent
447c930394
commit
24ef02f934
@@ -500,29 +500,40 @@ static int page_pool_register_dma_index(struct page_pool *pool,
|
||||
return err;
|
||||
}
|
||||
|
||||
static int page_pool_release_dma_index(struct page_pool *pool,
|
||||
netmem_ref netmem)
|
||||
static void __page_pool_unmap_netmem_dma(struct page_pool *pool,
|
||||
netmem_ref netmem)
|
||||
{
|
||||
struct page *old, *page = netmem_to_page(netmem);
|
||||
unsigned long id;
|
||||
dma_addr_t dma;
|
||||
|
||||
if (unlikely(!PP_DMA_INDEX_BITS))
|
||||
return 0;
|
||||
if (!pool->dma_map)
|
||||
return;
|
||||
|
||||
id = netmem_get_dma_index(netmem);
|
||||
if (!id)
|
||||
return -1;
|
||||
/* Cache dma_addr before xa_cmpxchg. The scrub path holds no page ref;
|
||||
* the unref path calls put_page() regardless of cmpxchg outcome, so
|
||||
* after the cmpxchg we cannot safely touch netmem fields.
|
||||
*/
|
||||
dma = page_pool_get_dma_addr_netmem(netmem);
|
||||
|
||||
if (in_softirq())
|
||||
old = xa_cmpxchg(&pool->dma_mapped, id, page, NULL, 0);
|
||||
else
|
||||
old = xa_cmpxchg_bh(&pool->dma_mapped, id, page, NULL, 0);
|
||||
if (old != page)
|
||||
return -1;
|
||||
if (likely(PP_DMA_INDEX_BITS)) {
|
||||
id = netmem_get_dma_index(netmem);
|
||||
if (!id)
|
||||
return;
|
||||
|
||||
netmem_set_dma_index(netmem, 0);
|
||||
if (in_softirq())
|
||||
old = xa_cmpxchg(&pool->dma_mapped,
|
||||
id, page, NULL, 0);
|
||||
else
|
||||
old = xa_cmpxchg_bh(&pool->dma_mapped,
|
||||
id, page, NULL, 0);
|
||||
if (old != page)
|
||||
return;
|
||||
}
|
||||
|
||||
return 0;
|
||||
dma_unmap_page_attrs(pool->p.dev, dma,
|
||||
PAGE_SIZE << pool->p.order, pool->p.dma_dir,
|
||||
DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING);
|
||||
}
|
||||
|
||||
static bool page_pool_dma_map(struct page_pool *pool, netmem_ref netmem, gfp_t gfp)
|
||||
@@ -728,24 +739,16 @@ void page_pool_clear_pp_info(netmem_ref netmem)
|
||||
static __always_inline void __page_pool_release_netmem_dma(struct page_pool *pool,
|
||||
netmem_ref netmem)
|
||||
{
|
||||
dma_addr_t dma;
|
||||
|
||||
/* Caller must hold a page ref: __page_pool_unmap_netmem_dma() is
|
||||
* safe without a ref, but the field clears below require it.
|
||||
*/
|
||||
if (!pool->dma_map)
|
||||
/* Always account for inflight pages, even if we didn't
|
||||
* map them
|
||||
*/
|
||||
return;
|
||||
|
||||
if (page_pool_release_dma_index(pool, netmem))
|
||||
return;
|
||||
|
||||
dma = page_pool_get_dma_addr_netmem(netmem);
|
||||
|
||||
/* When page is unmapped, it cannot be returned to our pool */
|
||||
dma_unmap_page_attrs(pool->p.dev, dma,
|
||||
PAGE_SIZE << pool->p.order, pool->p.dma_dir,
|
||||
DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_WEAK_ORDERING);
|
||||
__page_pool_unmap_netmem_dma(pool, netmem);
|
||||
page_pool_set_dma_addr_netmem(netmem, 0);
|
||||
if (likely(PP_DMA_INDEX_BITS))
|
||||
netmem_set_dma_index(netmem, 0);
|
||||
}
|
||||
|
||||
/* Disconnects a page (from a page_pool). API users can have a need
|
||||
@@ -1171,8 +1174,9 @@ static void page_pool_scrub(struct page_pool *pool)
|
||||
synchronize_net();
|
||||
}
|
||||
|
||||
/* No page ref, dma-unmap only. */
|
||||
xa_for_each(&pool->dma_mapped, id, ptr)
|
||||
__page_pool_release_netmem_dma(pool, page_to_netmem((struct page *)ptr));
|
||||
__page_pool_unmap_netmem_dma(pool, page_to_netmem((struct page *)ptr));
|
||||
}
|
||||
|
||||
/* No more consumers should exist, but producers could still
|
||||
|
||||
Reference in New Issue
Block a user