Skip to content

Conversation

@kernel-patches-daemon-bpf
Copy link

Pull request for series with
subject: bpf: Fix NULL deref in __list_del_clearprev for flush_node
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1033576

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 6f0b824
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1033576
version: 1

@kernel-patches-review-bot
Copy link

commit 120974cc28b023d86af8b1f0e0cf8b8c0ed7e575
Author: DLpang <[email protected]>

bpf: Fix NULL deref in __list_del_clearprev for flush_node

This patch fixes a NULL pointer dereference in the BPF subsystem by
adding initialization and defensive checks for the flush_node list_head
in per-CPU XDP bulk queues.

Link: https://syzkaller.appspot.com/bug?extid=2b3391f44313b3983e91

> diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
> index 04171fbc39cb..6ab6eae31989 100644
> --- a/kernel/bpf/cpumap.c
> +++ b/kernel/bpf/cpumap.c
> @@ -450,6 +450,7 @@ __cpu_map_entry_alloc(struct bpf_map *map, struct bpf_cpumap_val *value,
>
>  	for_each_possible_cpu(i) {
>  		bq = per_cpu_ptr(rcpu->bulkq, i);
> +		INIT_LIST_HEAD(&bq->flush_node);
>  		bq->obj = rcpu;
>  	}
>

[ ... ]

> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> index 2625601de76e..7a7347e709cc 100644
> --- a/kernel/bpf/devmap.c
> +++ b/kernel/bpf/devmap.c
> @@ -428,7 +428,8 @@ void __dev_flush(struct list_head *flush_list)
>  		bq_xmit_all(bq, XDP_XMIT_FLUSH);
>  		bq->dev_rx = NULL;
>  		bq->xdp_prog = NULL;
> -		__list_del_clearprev(&bq->flush_node);
> +		if (bq->flush_node.prev)
> +			__list_del_clearprev(&bq->flush_node);
                ^^^^

The devmap code adds the same defensive check but doesn't initialize
flush_node the way cpumap does. Looking at dev_map_notification(), when
NETDEV_REGISTER occurs, it calls:

    alloc_percpu(struct xdp_dev_bulk_queue)

And then only initializes the dev field:

    per_cpu_ptr(netdev->xdp_bulkq, cpu)->dev = netdev;

Without INIT_LIST_HEAD(&bq->flush_node), if alloc_percpu() returns
memory with non-zero bytes, bq->flush_node.prev could contain garbage.

The enqueue path in bq_enqueue() uses:

    if (!bq->dev_rx)
        list_add(&bq->flush_node, flush_list);

So if dev_rx contains non-zero garbage, the node never gets added to the
list. But this flush code checks bq->flush_node.prev which could also be
garbage, causing __list_del_clearprev() to be called with invalid
pointers.

Does devmap need INIT_LIST_HEAD(&bq->flush_node) similar to cpumap? The
commit message states that initialization is part of the fix, but only
cpumap gets it.

[ ... ]



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

In-Reply-To-Subject: bpf: Fix NULL deref in __list_del_clearprev for flush_node
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20255377282

@kernel-patches-daemon-bpf
Copy link
Author

Forwarding comment 3658573525 via email
In-Reply-To: [email protected]
Patch: https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 6f0b824
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1033576
version: 1

#syz test

Hi,

This patch fixes a NULL pointer dereference in the BPF subsystem that occurs
when __list_del_clearprev() is called on an already-cleared flush_node list_head.

The fix includes two parts:
1. Properly initialize the flush_node list_head during per-CPU bulk queue allocation
   using INIT_LIST_HEAD(&bq->flush_node)
2. Add defensive checks before calling __list_del_clearprev() to ensure the node
   is actually in the list by checking if (bq->flush_node.prev)

According to the __list_del_clearprev documentation in include/linux/list.h,
'The code that uses this needs to check the node 'prev' pointer instead of calling list_empty()'.

This patch fixes the following syzbot-reported issue:
https://syzkaller.appspot.com/bug?extid=2b3391f44313b3983e91

Reported-by: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=2b3391f44313b3983e91
Signed-off-by: DLpang <[email protected]>
Reported-by: [email protected]
Tested-by: [email protected]
@kernel-patches-daemon-bpf
Copy link
Author

At least one diff in series https://patchwork.kernel.org/project/netdevbpf/list/?series=1033576 expired. Closing PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant