netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Yonghong Song <yonghong.song@linux.dev>
To: Sahil Chandna <chandna.linuxkernel@gmail.com>,
	ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
	martin.lau@linux.dev, song@kernel.org, john.fastabend@gmail.com,
	haoluo@google.com, jolsa@kernel.org, bpf@vger.kernel.org,
	netdev@vger.kernel.org
Cc: david.hunter.linux@gmail.com, skhan@linuxfoundation.org,
	khalid@kernel.org,
	syzbot+1f1fbecb9413cdbfbef8@syzkaller.appspotmail.com
Subject: Re: [PATCH v2] bpf: test_run: Use migrate_enable()/disable() universally
Date: Fri, 10 Oct 2025 08:28:04 -0700	[thread overview]
Message-ID: <cb3b5d30-d232-4eb8-af31-2a1518ed8966@linux.dev> (raw)
In-Reply-To: <20251010075923.408195-1-chandna.linuxkernel@gmail.com>



On 10/10/25 12:59 AM, Sahil Chandna wrote:
> The timer context can safely use migrate_disable()/migrate_enable()
> universally instead of conditional preemption or migration disabling.
> Previously, the timer was initialized in NO_PREEMPT mode by default,
> which disabled preemption and forced execution in atomic context.
> This caused issues on PREEMPT_RT configurations when invoking
> spin_lock_bh() — a sleeping lock — leading to the following warning:
>
> BUG: sleeping function called from invalid context at kernel/locking/spinlock_rt.c:48
> in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 6107, name: syz.0.17
> preempt_count: 1, expected: 0
> RCU nest depth: 1, expected: 1
> Preemption disabled at:
> [<ffffffff891fce58>] bpf_test_timer_enter+0xf8/0x140 net/bpf/test_run.c:42
>
> Reported-by: syzbot+1f1fbecb9413cdbfbef8@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=1f1fbecb9413cdbfbef8
> Tested-by: syzbot+1f1fbecb9413cdbfbef8@syzkaller.appspotmail.com
> Signed-off-by: Sahil Chandna <chandna.linuxkernel@gmail.com>
>
> ---
> Link to v1: https://lore.kernel.org/all/20251006054320.159321-1-chandna.linuxkernel@gmail.com/
>
> Changes since v1:
> - Dropped `enum { NO_PREEMPT, NO_MIGRATE } mode` from `struct bpf_test_timer`.
> - Removed all conditional preempt/migrate disable logic.
> - Unified timer handling to use `migrate_disable()` / `migrate_enable()` universally.
>
> Testing:
> - Reproduced syzbot bug locally using the provided reproducer.
> - Observed `BUG: sleeping function called from invalid context` on v1.
> - Confirmed bug disappears after applying this patch.
> - Validated normal functionality of `bpf_prog_test_run_*` helpers with C
>    reproducer.
> ---
>   net/bpf/test_run.c | 20 ++++++--------------
>   1 file changed, 6 insertions(+), 14 deletions(-)
>
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index dfb03ee0bb62..b23bc93e738e 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -29,7 +29,6 @@
>   #include <trace/events/bpf_test_run.h>
>   
>   struct bpf_test_timer {
> -	enum { NO_PREEMPT, NO_MIGRATE } mode;
>   	u32 i;
>   	u64 time_start, time_spent;
>   };
> @@ -38,10 +37,7 @@ static void bpf_test_timer_enter(struct bpf_test_timer *t)
>   	__acquires(rcu)
>   {
>   	rcu_read_lock();
> -	if (t->mode == NO_PREEMPT)
> -		preempt_disable();
> -	else
> -		migrate_disable();
> +	migrate_disable();
>   
>   	t->time_start = ktime_get_ns();
>   }
> @@ -50,11 +46,7 @@ static void bpf_test_timer_leave(struct bpf_test_timer *t)
>   	__releases(rcu)
>   {
>   	t->time_start = 0;
> -
> -	if (t->mode == NO_PREEMPT)
> -		preempt_enable();
> -	else
> -		migrate_enable();
> +	migrate_enable();
>   	rcu_read_unlock();
>   }
>   
> @@ -374,7 +366,7 @@ static int bpf_test_run_xdp_live(struct bpf_prog *prog, struct xdp_buff *ctx,
>   
>   {
>   	struct xdp_test_data xdp = { .batch_size = batch_size };
> -	struct bpf_test_timer t = { .mode = NO_MIGRATE };
> +	struct bpf_test_timer t;

We still need to initialize 'struct bpf_test_timer t' with t.time_spent = 0 like
	struct bpf_test_timer t = {};
since time_spent is used like
         t->time_spent += ktime_get_ns() - t->time_start;


>   	int ret;
>   
>   	if (!repeat)
> @@ -404,7 +396,7 @@ static int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat,
>   	struct bpf_prog_array_item item = {.prog = prog};
>   	struct bpf_run_ctx *old_ctx;
>   	struct bpf_cg_run_ctx run_ctx;
> -	struct bpf_test_timer t = { NO_MIGRATE };
> +	struct bpf_test_timer t;
>   	enum bpf_cgroup_storage_type stype;
>   	int ret;
>   
> @@ -1377,7 +1369,7 @@ int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
>   				     const union bpf_attr *kattr,
>   				     union bpf_attr __user *uattr)
>   {
> -	struct bpf_test_timer t = { NO_PREEMPT };
> +	struct bpf_test_timer t;
>   	u32 size = kattr->test.data_size_in;
>   	struct bpf_flow_dissector ctx = {};
>   	u32 repeat = kattr->test.repeat;
> @@ -1445,7 +1437,7 @@ int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
>   int bpf_prog_test_run_sk_lookup(struct bpf_prog *prog, const union bpf_attr *kattr,
>   				union bpf_attr __user *uattr)
>   {
> -	struct bpf_test_timer t = { NO_PREEMPT };
> +	struct bpf_test_timer t;
>   	struct bpf_prog_array *progs = NULL;
>   	struct bpf_sk_lookup_kern ctx = {};
>   	u32 repeat = kattr->test.repeat;


  reply	other threads:[~2025-10-10 15:28 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-10  7:59 [PATCH v2] bpf: test_run: Use migrate_enable()/disable() universally Sahil Chandna
2025-10-10 15:28 ` Yonghong Song [this message]
2025-10-11 13:17 ` Menglong Dong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cb3b5d30-d232-4eb8-af31-2a1518ed8966@linux.dev \
    --to=yonghong.song@linux.dev \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=chandna.linuxkernel@gmail.com \
    --cc=daniel@iogearbox.net \
    --cc=david.hunter.linux@gmail.com \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=khalid@kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=netdev@vger.kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=song@kernel.org \
    --cc=syzbot+1f1fbecb9413cdbfbef8@syzkaller.appspotmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).