Re: [PATCH bpf v2] bpf: cpumap: fix use-after-free of dev_rx on netdev unregister

[Jiayuan Chen <jiayuan.chen@linux.dev>]

On 9/22/26 9:36 PM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
>> index 5e59ab896f058..0c9647f28034d 100644
>> --- a/kernel/bpf/cpumap.c
>> +++ b/kernel/bpf/cpumap.c
> [ ... ]
>
>> @@ -832,3 +879,39 @@ void __cpu_map_flush(struct list_head *flush_list)
>>   		wake_up_process(bq->obj->kthread);
>>   	}
>>   }
>> +
>> +/* Frames in the ring and skbs in GRO hold a raw pointer to the ingress
>> + * device, make every kthread consume them before the device is freed.
>> + */
>                                     ^^^
>
> The BPF subsystem requires multi-line comments to have the opening /*
> on its own line, with text starting on the next line. This is a newly
> added comment. Should it be reformatted like this?
>
> /*
>   * Frames in the ring and skbs in GRO hold a raw pointer to the ingress
>   * device, make every kthread consume them before the device is freed.
>   */
>
>> +static int cpu_map_netdev_event(struct notifier_block *nb,
>> +				unsigned long event, void *ptr)
>> +{
>> +	struct bpf_cpu_map_entry *rcpu;
>> +
>> +	if (event != NETDEV_UNREGISTER)
>> +		return NOTIFY_OK;
>> +
>> +	mutex_lock(&cpu_map_mutex);
>> +	list_for_each_entry(rcpu, &cpu_map_list, list) {
>> +		/* the whole ring, plus a batch already pulled out */
>> +		WRITE_ONCE(rcpu->drain_left, rcpu->queue->size + CPUMAP_BATCH);
>> +		wake_up_process(rcpu->kthread);
>> +	}
>> +	list_for_each_entry(rcpu, &cpu_map_list, list)
>> +		wait_event(rcpu->drain_wq, !READ_ONCE(rcpu->drain_left));
>> +	mutex_unlock(&cpu_map_mutex);
>> +
>> +	return NOTIFY_OK;
>> +}
> NETDEV_UNREGISTER notifiers run with RTNL held:
> call_netdevice_notifiers_info() contains ASSERT_RTNL(), and the call
> site in unregister_netdevice_many_notify() also does ASSERT_RTNL() at
> entry.
>
> The wait_event() holds RTNL until every cpumap kthread in the system
> has consumed rcpu->queue->size + CPUMAP_BATCH frames. The ring size is
> bounded only by the 16384 sanity limit in cpu_map_update_elem(), and
> the countdown is decremented by frames actually consumed, so a ring
> that is continuously fed by a different, still-registered device forces
> the full 16392-frame count to be pushed through before returning.
>
> unregister_netdevice_many_notify() calls the notifier once per device
> in the batch, so a batched teardown such as netns exit with many veths
> multiplies this: N devices x 16392 frames of RTNL-held work,
> serialized.
>
> The commit message addresses the bound ("The work is bounded by the
> ring size, and the frames had to be consumed anyway") but not the fact
> that RTNL is held for the whole drain. Also, the "frames had to be
> consumed anyway" statement is only true for frames already queued - the
> size+BATCH countdown can force consumption of many newly produced
> frames belonging to unrelated devices.
>
> This is not a deadlock (the drain is bounded and the kthread cannot
> block indefinitely since its work runs under local_bh_disable()), but
> could this cause latency or stall issues on systems with many devices
> or large ring sizes?
>
> [ ... ]


The concern is an unbounded wait under RTNL. It is bounded: qsize is
capped at 16384 in cpu_map_update_elem(), so each entry consumes at
most one ring plus a GRO flush, and the kthreads do that in parallel.

Those frames had to be consumed anyway, the drain only moves that
work before free_netdev(). We can't tell which ring holds frames of
the device without tracking it per frame, and that would touch the
fast path.


>> -    Fixes: 9c270af37bb6 ("bpf: XDP_REDIRECT enable use of cpumap")
>> +    Fixes: 70280ed91cb8 ("bpf: cpumap convert to use generic xdp_frame")
>>       Signed-off-by: Jiayuan Chen <jiayuan.chen@linux.dev>
> The Fixes tag points to commit 9c270af37bb6 ("bpf: XDP_REDIRECT enable
> use of cpumap"), but is this the right commit?
>
> That commit (October 2017) introduced the cpu_map_enqueue() function
> signature but did not store dev_rx anywhere - it used data_hard_start
> as a void pointer with a comment saying "Followup patch will generalize
> this".
>
> The actual storage of the device pointer without a reference
> (xdpf->dev_rx = dev_rx;) was added 6 months later in commit
> 70280ed91cb8 ("bpf: cpumap convert to use generic xdp_frame", April
> 2018) when the code was converted to use the generic xdp_frame
> structure. This is where the use-after-free bug was introduced.
>
> Should the Fixes tag be:
>
>    Fixes: 70280ed91cb8 ("bpf: cpumap convert to use generic xdp_frame")


Right, 9c270af37bb6 only added the dev_rx argument. The pointer is

stored and used from 1c601d829ab0 ("bpf: cpumap xdp_buff to skb

conversion and allocation").

pw-bot: cr


Sorry for the noise

>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/35727192473