Skip to content

Conversation

@kernel-patches-daemon-bpf
Copy link

Pull request for series with
subject: bpf: use preempt_disable/enable() to protect bpf_bprintf_buffers nesting
version: 1
url: https://patchwork.kernel.org/project/netdevbpf/list/?series=1021330

@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: f8c67d8
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1021330
version: 1

@kernel-patches-review-bot
Copy link

commit 0d4534a197e660c783c856339a77484b5d706b3d
Author: Sahil Chandna <chandna.sahil@gmail.com>

bpf: use preempt_disable/enable() to protect bpf_bprintf_buffers nesting

This patch adds preempt_disable()/enable() around per-CPU buffer operations
to prevent task migration between CPUs, fixing a race where nest_level counters
could become inconsistent across CPU migrations.

Closes: https://syzkaller.appspot.com/bug?extid=b0cff308140f79a9c4cb

> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 865b0dae3..99780b654 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -774,9 +774,11 @@ int bpf_try_get_buffers(struct bpf_bprintf_buffers **bufs)
>  {
>  	int nest_level;
>
> +	preempt_disable();
>  	nest_level = this_cpu_inc_return(bpf_bprintf_nest_level);
>  	if (WARN_ON_ONCE(nest_level > MAX_BPRINTF_NEST_LEVEL)) {
>  		this_cpu_dec(bpf_bprintf_nest_level);
> +		preempt_enable();
>  		return -EBUSY;
>  	}
>  	*bufs = this_cpu_ptr(&bpf_bprintf_bufs[nest_level - 1]);
> @@ -786,9 +788,12 @@ int bpf_try_get_buffers(struct bpf_bprintf_buffers **bufs)
>
>  void bpf_put_buffers(void)
>  {
> -	if (WARN_ON_ONCE(this_cpu_read(bpf_bprintf_nest_level) == 0))
> +	if (WARN_ON_ONCE(this_cpu_read(bpf_bprintf_nest_level) == 0)) {
> +		preempt_enable();
                 ^^^^^^^^^^^^^^

Can this cause a preempt count underflow? The WARN_ON_ONCE checks for
nest_level==0, which means bpf_try_get_buffers() was never successfully
called. In that case, preempt_disable() was never called either, so calling
preempt_enable() here would underflow the preempt count.

In the normal calling pattern, bpf_try_get_buffers() either succeeds
(disabling preemption) or fails before reaching bpf_put_buffers(). But this
WARN exists to catch programming bugs where the get/put contract is violated.
If such a bug occurs, adding preempt_enable() here makes it worse by
introducing a preempt count imbalance on top of the original error.

> +		return;
> +	}
>  	this_cpu_dec(bpf_bprintf_nest_level);
> +	preempt_enable();
>  }



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: use preempt_disable/enable() to protect bpf_bprintf_buffers nesting
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/19212189476

@kernel-patches-daemon-bpf
Copy link
Author

The bpf_bprintf_prepare() and related helpers (bpf_try_get_buffers() /
bpf_put_buffers()) rely on a per-CPU counter bpf_bprintf_nest_level to
manage nested buffer usage. However, when invoked from different contexts
(process, softirq, NMI), the nesting counter can become inconsistent if
task  migration occurs between CPUs during these operations. This can
result in warnings such as:

WARNING: CPU: 1 PID: 6145 at kernel/bpf/helpers.c:781 bpf_try_get_buffers kernel/bpf/helpers.c:781 [inline]
WARNING: CPU: 1 PID: 6145 at kernel/bpf/helpers.c:781 bpf_bprintf_prepare+0x12cf/0x13a0 kernel/bpf/helpers.c:834

Having only migrate_disable is insufficient here to prevent nesting,
hence add preempt_disable()/enable() around buffer acquisition and release.

Reported-by: syzbot+b0cff308140f79a9c4cb@syzkaller.appspotmail.comi
Closes: https://syzkaller.appspot.com/bug?extid=b0cff308140f79a9c4cb
Fixes: 7c33e97 ("bpf: Do not disable preemption in bpf_test_run().")
Suggested-by: Yonghong Song <yonghong.song@linux.dev>
Signed-off-by: Sahil Chandna <chandna.sahil@gmail.com>
@kernel-patches-daemon-bpf
Copy link
Author

Upstream branch: 026bcf9
series: https://patchwork.kernel.org/project/netdevbpf/list/?series=1021330
version: 1

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.

2 participants