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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] bpf: cpumap: UAF of netdev due to unlinking dying entry before kthread stop
- [High] bpf: cpumap: TOCTOU race in dev unregistering check allows UAF via chained redirects
--

commit 357385e1a9c647836291f6c8f562ac50211ab22f
Author: Jiayuan Chen <jiayuan.chen@linux.dev>

bpf: cpumap: fix use-after-free of dev_rx on netdev unregister

This commit adds a netdev unregister notifier to drain cpumap kthreads to
prevent use-after-free of stale netdev pointers in queued frames. It forces
kthreads to consume existing frames and flush GRO caches before unregistration
completes. It also checks for unregistered devices during cpumap redirects to
drop stale packets.

> diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
> index 5e59ab896f058..bc2186adcd9a4 100644
> --- a/kernel/bpf/cpumap.c
> +++ b/kernel/bpf/cpumap.c
[ ... ]
> @@ -153,6 +171,11 @@ static u32 cpu_map_bpf_prog_run_skb(struct bpf_cpu_map_entry *rcpu,
>  			skbs[pass++] = skb;
>  			break;
>  		case XDP_REDIRECT:
> +			if (cpu_map_dev_unregistering(skb->dev)) {
> +				kfree_skb(skb);
> +				stats->drop++;
> +				break;
> +			}

[Severity: High]
Is there a time-of-check to time-of-use (TOCTOU) race here when chaining
cpumap-to-cpumap redirects?

During a redirect from cpumap A to cpumap B, kthread A might check
cpu_map_dev_unregistering() and see the device is still registered, then get
preempted.

A netdev unregister could then trigger the notifier, which sets drain_left
for all kthreads and starts waiting. Kthread B might wake up, find its queue
empty, report drain_left = 0, and wake the notifier.

Then kthread A resumes, enqueues its packet to cpumap B, finishes its batch,
reports drain_left = 0, and wakes the notifier.

Since the notifier checks the wait queues one-by-one independently in
cpu_map_netdev_event():

	list_for_each_entry(rcpu, &cpu_map_list, list)
		wait_event(rcpu->drain_wq, !READ_ONCE(rcpu->drain_left));

It could unblock because both queues eventually reported 0, allowing the
netdev to be freed. Kthread B could then wake up to process the newly
enqueued packet, leading to a use-after-free on the netdevice.

>  			err = xdp_do_generic_redirect(skb->dev, skb, &xdp,
>  						      rcpu->prog);
[ ... ]
> @@ -528,6 +576,11 @@ static void __cpu_map_entry_free(struct work_struct *work)
>  	 */
>  	rcpu = container_of(to_rcu_work(work), struct bpf_cpu_map_entry, free_work);
>  
> +	/* Unlink first, so the notifier can't wait on a kthread we stop */
> +	mutex_lock(&cpu_map_mutex);
> +	list_del(&rcpu->list);
> +	mutex_unlock(&cpu_map_mutex);
> +

[Severity: High]
Does unlinking this entry before stopping the kthread open a window for a
use-after-free?

If a netdev unregister happens concurrently with a map update freeing this
entry, cpu_map_netdev_event() will iterate over cpu_map_list.

Since the entry is already removed from the list via list_del(), the notifier
won't wait for this dying kthread to drain.

The netdevice can then be freed, but the kthread could still be processing
its queue while shutting down via kthread_stop(), potentially accessing the
freed netdevice through xdpf->dev_rx or skb->dev.

>  	/* kthread_stop will wake_up_process and wait for it to complete.
>  	 * cpu_map_kthread_run() makes sure the pointer ring is empty
>  	 * before exiting.
>  	 */
>  	kthread_stop(rcpu->kthread);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260920133017.248620-1-jiayuan.chen@linux.dev?part=1