Skip to content

Commit 271683b

Browse files
Dong Chenchenkuba-moo
authored andcommitted
page_pool: Fix use-after-free in page_pool_recycle_in_ring
syzbot reported a uaf in page_pool_recycle_in_ring: BUG: KASAN: slab-use-after-free in lock_release+0x151/0xa30 kernel/locking/lockdep.c:5862 Read of size 8 at addr ffff8880286045a0 by task syz.0.284/6943 CPU: 0 UID: 0 PID: 6943 Comm: syz.0.284 Not tainted 6.13.0-rc3-syzkaller-gdfa94ce54f41 #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024 Call Trace: <TASK> __dump_stack lib/dump_stack.c:94 [inline] dump_stack_lvl+0x241/0x360 lib/dump_stack.c:120 print_address_description mm/kasan/report.c:378 [inline] print_report+0x169/0x550 mm/kasan/report.c:489 kasan_report+0x143/0x180 mm/kasan/report.c:602 lock_release+0x151/0xa30 kernel/locking/lockdep.c:5862 __raw_spin_unlock_bh include/linux/spinlock_api_smp.h:165 [inline] _raw_spin_unlock_bh+0x1b/0x40 kernel/locking/spinlock.c:210 spin_unlock_bh include/linux/spinlock.h:396 [inline] ptr_ring_produce_bh include/linux/ptr_ring.h:164 [inline] page_pool_recycle_in_ring net/core/page_pool.c:707 [inline] page_pool_put_unrefed_netmem+0x748/0xb00 net/core/page_pool.c:826 page_pool_put_netmem include/net/page_pool/helpers.h:323 [inline] page_pool_put_full_netmem include/net/page_pool/helpers.h:353 [inline] napi_pp_put_page+0x149/0x2b0 net/core/skbuff.c:1036 skb_pp_recycle net/core/skbuff.c:1047 [inline] skb_free_head net/core/skbuff.c:1094 [inline] skb_release_data+0x6c4/0x8a0 net/core/skbuff.c:1125 skb_release_all net/core/skbuff.c:1190 [inline] __kfree_skb net/core/skbuff.c:1204 [inline] sk_skb_reason_drop+0x1c9/0x380 net/core/skbuff.c:1242 kfree_skb_reason include/linux/skbuff.h:1263 [inline] __skb_queue_purge_reason include/linux/skbuff.h:3343 [inline] root cause is: page_pool_recycle_in_ring ptr_ring_produce spin_lock(&r->producer_lock); WRITE_ONCE(r->queue[r->producer++], ptr) //recycle last page to pool page_pool_release page_pool_scrub page_pool_empty_ring ptr_ring_consume page_pool_return_page //release all page __page_pool_destroy free_percpu(pool->recycle_stats); free(pool) //free spin_unlock(&r->producer_lock); //pool->ring uaf read recycle_stat_inc(pool, ring); page_pool can be free while page pool recycle the last page in ring. Add producer-lock barrier to page_pool_release to prevent the page pool from being free before all pages have been recycled. recycle_stat_inc() is empty when CONFIG_PAGE_POOL_STATS is not enabled, which will trigger Wempty-body build warning. Add definition for pool stat macro to fix warning. Suggested-by: Jakub Kicinski <[email protected]> Link: https://lore.kernel.org/netdev/[email protected] Fixes: ff7d6b2 ("page_pool: refurbish version of page_pool code") Reported-by: [email protected] Closes: https://syzkaller.appspot.com/bug?extid=204a4382fcb3311f3858 Signed-off-by: Dong Chenchen <[email protected]> Reviewed-by: Toke Høiland-Jørgensen <[email protected]> Reviewed-by: Mina Almasry <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]>
1 parent 9ad0452 commit 271683b

File tree

1 file changed

+14
-13
lines changed

1 file changed

+14
-13
lines changed

net/core/page_pool.c

Lines changed: 14 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -153,9 +153,9 @@ u64 *page_pool_ethtool_stats_get(u64 *data, const void *stats)
153153
EXPORT_SYMBOL(page_pool_ethtool_stats_get);
154154

155155
#else
156-
#define alloc_stat_inc(pool, __stat)
157-
#define recycle_stat_inc(pool, __stat)
158-
#define recycle_stat_add(pool, __stat, val)
156+
#define alloc_stat_inc(...) do { } while (0)
157+
#define recycle_stat_inc(...) do { } while (0)
158+
#define recycle_stat_add(...) do { } while (0)
159159
#endif
160160

161161
static bool page_pool_producer_lock(struct page_pool *pool)
@@ -741,19 +741,16 @@ void page_pool_return_page(struct page_pool *pool, netmem_ref netmem)
741741

742742
static bool page_pool_recycle_in_ring(struct page_pool *pool, netmem_ref netmem)
743743
{
744-
int ret;
745-
/* BH protection not needed if current is softirq */
746-
if (in_softirq())
747-
ret = ptr_ring_produce(&pool->ring, (__force void *)netmem);
748-
else
749-
ret = ptr_ring_produce_bh(&pool->ring, (__force void *)netmem);
744+
bool in_softirq, ret;
750745

751-
if (!ret) {
746+
/* BH protection not needed if current is softirq */
747+
in_softirq = page_pool_producer_lock(pool);
748+
ret = !__ptr_ring_produce(&pool->ring, (__force void *)netmem);
749+
if (ret)
752750
recycle_stat_inc(pool, ring);
753-
return true;
754-
}
751+
page_pool_producer_unlock(pool, in_softirq);
755752

756-
return false;
753+
return ret;
757754
}
758755

759756
/* Only allow direct recycling in special circumstances, into the
@@ -1150,10 +1147,14 @@ static void page_pool_scrub(struct page_pool *pool)
11501147

11511148
static int page_pool_release(struct page_pool *pool)
11521149
{
1150+
bool in_softirq;
11531151
int inflight;
11541152

11551153
page_pool_scrub(pool);
11561154
inflight = page_pool_inflight(pool, true);
1155+
/* Acquire producer lock to make sure producers have exited. */
1156+
in_softirq = page_pool_producer_lock(pool);
1157+
page_pool_producer_unlock(pool, in_softirq);
11571158
if (!inflight)
11581159
__page_pool_destroy(pool);
11591160

0 commit comments

Comments
 (0)