BPF List
 help / color / mirror / Atom feed
* [PATCH bpf-next v1 0/2] Fix sleepable context checks in verifier
@ 2026-08-06 16:40 Kumar Kartikeya Dwivedi
  2026-08-06 16:40 ` [PATCH bpf-next v1 1/2] bpf: Fix sleepable context checks and remove in_sleepable helper Kumar Kartikeya Dwivedi
  2026-08-06 16:40 ` [PATCH bpf-next v1 2/2] selftests/bpf: Test sleepable context distinctions Kumar Kartikeya Dwivedi
  0 siblings, 2 replies; 10+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2026-08-06 16:40 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Eduard Zingerman, Emil Tsalapatis, kkd, kernel-team

Fix insufficient in_sleepable() checks that allowed sleepable iterators and
kfuncs in non-sleepable contexts (e.g., inside bpf_rcu_read_lock()). Remove
in_sleepable() helper and enforce in_sleepable_context() everywhere, with
direct env->cur_state->in_sleepable checks where needed. Add selftests that
fail before the fix and pass after the fix.

Kumar Kartikeya Dwivedi (2):
  bpf: Fix sleepable context checks and remove in_sleepable helper
  selftests/bpf: Test sleepable context distinctions

 kernel/bpf/verifier.c                         |  13 +-
 .../selftests/bpf/progs/iters_task_failure.c  | 153 ++++++++++++++++++
 2 files changed, 157 insertions(+), 9 deletions(-)


base-commit: 8c7f55d60aa3512fd3da7e9cf313723460e487b4
-- 
2.53.0-Meta


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH bpf-next v1 1/2] bpf: Fix sleepable context checks and remove in_sleepable helper
  2026-08-06 16:40 [PATCH bpf-next v1 0/2] Fix sleepable context checks in verifier Kumar Kartikeya Dwivedi
@ 2026-08-06 16:40 ` Kumar Kartikeya Dwivedi
  2026-08-06 17:14   ` sashiko-bot
                     ` (2 more replies)
  2026-08-06 16:40 ` [PATCH bpf-next v1 2/2] selftests/bpf: Test sleepable context distinctions Kumar Kartikeya Dwivedi
  1 sibling, 3 replies; 10+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2026-08-06 16:40 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Eduard Zingerman, Emil Tsalapatis, kkd, kernel-team

The verifier has two helpers to determine if the current program can
sleep:

  - in_sleepable() checks only env->cur_state->in_sleepable
  - in_sleepable_context() checks active RCU, preempt, lock and IRQ
    state in addition to in_sleepable

Several call sites incorrectly used in_sleepable() where
in_sleepable_context() is required. This allowed sleepable programs
in non-sleepable contexts (e.g., inside bpf_rcu_read_lock()) to
incorrectly use sleepable iterators and kfuncs:

  - check_css_task_iter_allowlist() returned in_sleepable() and
    therefore allowed css_task iterator in any sleepable program,
    even inside RCU/preempt/lock/IRQ-disabled regions. Fix it to
    use in_sleepable_context().

  - The first sleepable check in check_kfunc_call() used
    in_sleepable() and only verified the program flag, not the
    current context. Replace it with in_sleepable_context() so
    sleepable kfuncs are rejected when called from any
    non-sleepable context, consistent with the second check in the
    same function.

To prevent future misuse, remove the in_sleepable() helper
completely and replace its legitimate uses with a direct check of
env->cur_state->in_sleepable. We do not want anyone to test this
bit directly and in isolation; all sleepable-context decisions must
go through in_sleepable_context(), which accounts for locks and
preemption. The remaining direct checks are in in_rcu_cs() and
in_sleepable_context() itself, where testing the bit in isolation
is intentional.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/verifier.c | 13 ++++---------
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index e6233c0081d1..3dadf2b33a36 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -4441,11 +4441,6 @@ static int map_kptr_match_type(struct bpf_verifier_env *env,
 	return -EINVAL;
 }
 
-static bool in_sleepable(struct bpf_verifier_env *env)
-{
-	return env->cur_state->in_sleepable;
-}
-
 /* The non-sleepable programs and sleepable programs with explicit bpf_rcu_read_lock()
  * can dereference RCU protected pointers and result is PTR_TRUSTED.
  */
@@ -4455,7 +4450,7 @@ static bool in_rcu_cs(struct bpf_verifier_env *env)
 	       env->cur_state->active_preempt_locks ||
 	       env->cur_state->active_locks ||
 	       env->cur_state->active_irq_id ||
-	       !in_sleepable(env);
+	       !env->cur_state->in_sleepable;
 }
 
 /* Once GCC supports btf_type_tag the following mechanism will be replaced with tag check */
@@ -10258,7 +10253,7 @@ static inline bool in_sleepable_context(struct bpf_verifier_env *env)
 	       !env->cur_state->active_preempt_locks &&
 	       !env->cur_state->active_locks &&
 	       !env->cur_state->active_irq_id &&
-	       in_sleepable(env);
+	       env->cur_state->in_sleepable;
 }
 
 static const char *non_sleepable_context_description(struct bpf_verifier_env *env)
@@ -12083,7 +12078,7 @@ static bool check_css_task_iter_allowlist(struct bpf_verifier_env *env)
 			return true;
 		fallthrough;
 	default:
-		return in_sleepable(env);
+		return in_sleepable_context(env);
 	}
 }
 
@@ -13090,7 +13085,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 	}
 
 	sleepable = bpf_is_kfunc_sleepable(&meta);
-	if (sleepable && !in_sleepable(env)) {
+	if (sleepable && !in_sleepable_context(env)) {
 		verbose(env, "program must be sleepable to call sleepable kfunc %s\n", func_name);
 		return -EACCES;
 	}
-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH bpf-next v1 2/2] selftests/bpf: Test sleepable context distinctions
  2026-08-06 16:40 [PATCH bpf-next v1 0/2] Fix sleepable context checks in verifier Kumar Kartikeya Dwivedi
  2026-08-06 16:40 ` [PATCH bpf-next v1 1/2] bpf: Fix sleepable context checks and remove in_sleepable helper Kumar Kartikeya Dwivedi
@ 2026-08-06 16:40 ` Kumar Kartikeya Dwivedi
  1 sibling, 0 replies; 10+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2026-08-06 16:40 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Eduard Zingerman, Emil Tsalapatis, kkd, kernel-team

Exercise verifier decisions which depend on the complete sleepable context
rather than only the current state's in_sleepable bit.

Verify that preemption-disabled and IRQ-disabled regions provide implicit RCU
protection to KF_RCU_PROTECTED css iterators.  Also verify that iterator
pointers become untrusted after the final preemption enable or IRQ restore,
and that css_task iterators are rejected in explicit RCU, preemption-disabled,
and IRQ-disabled regions of sleepable programs.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 .../selftests/bpf/progs/iters_task_failure.c  | 153 ++++++++++++++++++
 1 file changed, 153 insertions(+)

diff --git a/tools/testing/selftests/bpf/progs/iters_task_failure.c b/tools/testing/selftests/bpf/progs/iters_task_failure.c
index fe3663dedbe1..0a2ea8b6adda 100644
--- a/tools/testing/selftests/bpf/progs/iters_task_failure.c
+++ b/tools/testing/selftests/bpf/progs/iters_task_failure.c
@@ -13,6 +13,8 @@ struct cgroup *bpf_cgroup_from_id(u64 cgid) __ksym;
 void bpf_cgroup_release(struct cgroup *p) __ksym;
 void bpf_rcu_read_lock(void) __ksym;
 void bpf_rcu_read_unlock(void) __ksym;
+void bpf_local_irq_save(unsigned long *flags) __weak __ksym;
+void bpf_local_irq_restore(unsigned long *flags) __weak __ksym;
 
 SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
 __failure __msg("kernel func bpf_iter_task_new requires RCU critical section protection")
@@ -84,6 +86,157 @@ int BPF_PROG(iter_css_lock_and_unlock)
 	return 0;
 }
 
+SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
+__success
+int BPF_PROG(iter_css_preempt)
+{
+	u64 cg_id = bpf_get_current_cgroup_id();
+	struct cgroup *cgrp = bpf_cgroup_from_id(cg_id);
+	struct cgroup_subsys_state *root_css, *pos;
+
+	if (!cgrp)
+		return 0;
+	root_css = &cgrp->self;
+
+	bpf_preempt_disable();
+	bpf_for_each(css, pos, root_css, BPF_CGROUP_ITER_DESCENDANTS_POST) {}
+	bpf_preempt_enable();
+	bpf_cgroup_release(cgrp);
+	return 0;
+}
+
+SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
+__success
+int BPF_PROG(iter_css_irq)
+{
+	u64 cg_id = bpf_get_current_cgroup_id();
+	struct cgroup *cgrp = bpf_cgroup_from_id(cg_id);
+	struct cgroup_subsys_state *root_css, *pos;
+	unsigned long flags;
+
+	if (!cgrp)
+		return 0;
+	root_css = &cgrp->self;
+
+	bpf_local_irq_save(&flags);
+	bpf_for_each(css, pos, root_css, BPF_CGROUP_ITER_DESCENDANTS_POST) {}
+	bpf_local_irq_restore(&flags);
+	bpf_cgroup_release(cgrp);
+	return 0;
+}
+
+SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
+__failure __msg("R2 must be referenced or trusted")
+int BPF_PROG(iter_css_preempt_escape)
+{
+	u64 cg_id = bpf_get_current_cgroup_id();
+	struct cgroup *cgrp = bpf_cgroup_from_id(cg_id);
+	struct cgroup_subsys_state *root_css, *pos;
+	struct task_struct *task;
+
+	if (!cgrp)
+		return 0;
+	root_css = &cgrp->self;
+
+	bpf_preempt_disable();
+	bpf_for_each(css, pos, root_css, BPF_CGROUP_ITER_DESCENDANTS_POST) {
+		break;
+	}
+	bpf_preempt_enable();
+	if (pos) {
+		bpf_for_each(css_task, task, pos, CSS_TASK_ITER_PROCS) {}
+	}
+	bpf_cgroup_release(cgrp);
+	return 0;
+}
+
+SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
+__failure __msg("R2 must be referenced or trusted")
+int BPF_PROG(iter_css_irq_escape)
+{
+	u64 cg_id = bpf_get_current_cgroup_id();
+	struct cgroup *cgrp = bpf_cgroup_from_id(cg_id);
+	struct cgroup_subsys_state *root_css, *pos;
+	struct task_struct *task;
+	unsigned long flags;
+
+	if (!cgrp)
+		return 0;
+	root_css = &cgrp->self;
+
+	bpf_local_irq_save(&flags);
+	bpf_for_each(css, pos, root_css, BPF_CGROUP_ITER_DESCENDANTS_POST) {
+		break;
+	}
+	bpf_local_irq_restore(&flags);
+	if (pos) {
+		bpf_for_each(css_task, task, pos, CSS_TASK_ITER_PROCS) {}
+	}
+	bpf_cgroup_release(cgrp);
+	return 0;
+}
+
+SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
+__failure __msg("css_task_iter is only allowed in bpf_lsm, bpf_iter and sleepable progs")
+int BPF_PROG(iter_css_task_rcu)
+{
+	u64 cg_id = bpf_get_current_cgroup_id();
+	struct cgroup *cgrp = bpf_cgroup_from_id(cg_id);
+	struct cgroup_subsys_state *css;
+	struct task_struct *task;
+
+	if (!cgrp)
+		return 0;
+	css = &cgrp->self;
+
+	bpf_rcu_read_lock();
+	bpf_for_each(css_task, task, css, CSS_TASK_ITER_PROCS) {}
+	bpf_rcu_read_unlock();
+	bpf_cgroup_release(cgrp);
+	return 0;
+}
+
+SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
+__failure __msg("css_task_iter is only allowed in bpf_lsm, bpf_iter and sleepable progs")
+int BPF_PROG(iter_css_task_preempt)
+{
+	u64 cg_id = bpf_get_current_cgroup_id();
+	struct cgroup *cgrp = bpf_cgroup_from_id(cg_id);
+	struct cgroup_subsys_state *css;
+	struct task_struct *task;
+
+	if (!cgrp)
+		return 0;
+	css = &cgrp->self;
+
+	bpf_preempt_disable();
+	bpf_for_each(css_task, task, css, CSS_TASK_ITER_PROCS) {}
+	bpf_preempt_enable();
+	bpf_cgroup_release(cgrp);
+	return 0;
+}
+
+SEC("?fentry.s/" SYS_PREFIX "sys_getpgid")
+__failure __msg("css_task_iter is only allowed in bpf_lsm, bpf_iter and sleepable progs")
+int BPF_PROG(iter_css_task_irq)
+{
+	u64 cg_id = bpf_get_current_cgroup_id();
+	struct cgroup *cgrp = bpf_cgroup_from_id(cg_id);
+	struct cgroup_subsys_state *css;
+	struct task_struct *task;
+	unsigned long flags;
+
+	if (!cgrp)
+		return 0;
+	css = &cgrp->self;
+
+	bpf_local_irq_save(&flags);
+	bpf_for_each(css_task, task, css, CSS_TASK_ITER_PROCS) {}
+	bpf_local_irq_restore(&flags);
+	bpf_cgroup_release(cgrp);
+	return 0;
+}
+
 SEC("?fentry/" SYS_PREFIX "sys_getpgid")
 __failure __msg("css_task_iter is only allowed in bpf_lsm, bpf_iter and sleepable progs")
 int BPF_PROG(iter_css_task_for_each)
-- 
2.53.0-Meta


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH bpf-next v1 1/2] bpf: Fix sleepable context checks and remove in_sleepable helper
  2026-08-06 16:40 ` [PATCH bpf-next v1 1/2] bpf: Fix sleepable context checks and remove in_sleepable helper Kumar Kartikeya Dwivedi
@ 2026-08-06 17:14   ` sashiko-bot
  2026-08-06 20:31   ` Eduard Zingerman
  2026-08-06 21:10   ` Eduard Zingerman
  2 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-08-06 17:14 UTC (permalink / raw)
  To: Kumar Kartikeya Dwivedi; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Replacing `in_sleepable()` with `in_sleepable_context()` in the first sleepable check renders the second check dead code and degrades the verifier error message.

Pre-existing issues:
- [High] Incomplete fix: Bypassed sleepable context restrictions on the `css_task` iterator for LSM and TRACE_ITER BPF programs allow 'sleeping in atomic context' bugs on PREEMPT_RT.
--

--- Patch [1]: [PATCH bpf-next v1 1/2] bpf: Fix sleepable context checks and remove in_sleepable helper ---
commit fe09e0b3c58d1fc2d9c5ba46b9ee32ba789423ae
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

bpf: Fix sleepable context checks and remove in_sleepable helper

This patch updates several call sites to use in_sleepable_context() instead
of in_sleepable() to ensure that sleepable kfuncs and iterators are rejected
when called from non-sleepable contexts, such as inside RCU read-side
critical sections. It also removes the in_sleepable helper entirely to
prevent future misuse.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index e6233c0081d10..3dadf2b33a367 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c

[ ... ]

> @@ -12083,7 +12078,7 @@ static bool check_css_task_iter_allowlist(struct bpf_verifier_env *env)
>  			return true;
>  		fallthrough;
>  	default:
> -		return in_sleepable(env);
> +		return in_sleepable_context(env);
>  	}
>  }

[Severity: High]
This is a pre-existing issue, but does this bypass the newly applied sleepable
context check for LSM and TRACE_ITER programs?

Looking at the full function:

static bool check_css_task_iter_allowlist(struct bpf_verifier_env *env)
{
	enum bpf_prog_type prog_type = resolve_prog_type(env->prog);

	switch (prog_type) {
	case BPF_PROG_TYPE_LSM:
		return true;
	case BPF_PROG_TYPE_TRACING:
		if (env->prog->expected_attach_type == BPF_TRACE_ITER)
			return true;
...

If an LSM or BPF_TRACE_ITER program explicitly enters a non-sleepable
context (e.g., via bpf_preempt_disable()) and then initializes a css_task
iterator via bpf_iter_css_task_new(), they return true unconditionally.

Because the css_task iterator acquires css_set_lock (which maps to a sleepable
rt_mutex on PREEMPT_RT), could this cause a fatal lockdep splat or "sleeping
in atomic context" kernel panic on PREEMPT_RT kernels?

[ ... ]

> @@ -13090,7 +13085,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>  	}
>  
>  	sleepable = bpf_is_kfunc_sleepable(&meta);
> -	if (sleepable && !in_sleepable(env)) {
> +	if (sleepable && !in_sleepable_context(env)) {
>  		verbose(env, "program must be sleepable to call sleepable kfunc %s\n", func_name);
>  		return -EACCES;
>  	}

[Severity: Medium]
Does replacing this check render the second sleepable check in this function
unreachable?

Because sleepable kfuncs are not lock-manipulation kfuncs themselves, the
context state cannot change between this first check and the second check
further down in check_kfunc_call():

	if (sleepable && !in_sleepable_context(env)) {
		verbose(env, "kernel func %s is sleepable within %s\n",
			func_name, non_sleepable_context_description(env));
		return -EACCES;
	}

If a user writes a sleepable BPF program, enters a non-sleepable context,
and then calls a sleepable kfunc, the verifier will exit at the first check
with the generic "program must be sleepable..." error.

This is factually misleading since the program is sleepable but just inside
a non-sleepable context.

Is it possible to remove the first check entirely or consolidate them so we
keep the more accurate context-aware error message?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806164049.3158887-1-memxor@gmail.com?part=1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH bpf-next v1 1/2] bpf: Fix sleepable context checks and remove in_sleepable helper
  2026-08-06 16:40 ` [PATCH bpf-next v1 1/2] bpf: Fix sleepable context checks and remove in_sleepable helper Kumar Kartikeya Dwivedi
  2026-08-06 17:14   ` sashiko-bot
@ 2026-08-06 20:31   ` Eduard Zingerman
  2026-08-06 21:22     ` Kumar Kartikeya Dwivedi
  2026-08-06 21:10   ` Eduard Zingerman
  2 siblings, 1 reply; 10+ messages in thread
From: Eduard Zingerman @ 2026-08-06 20:31 UTC (permalink / raw)
  To: Kumar Kartikeya Dwivedi, bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Emil Tsalapatis, kkd, kernel-team

On Thu, 2026-08-06 at 18:40 +0200, Kumar Kartikeya Dwivedi wrote:

...

> @@ -10258,7 +10253,7 @@ static inline bool in_sleepable_context(struct bpf_verifier_env *env)
>  	       !env->cur_state->active_preempt_locks &&
>  	       !env->cur_state->active_locks &&
>  	       !env->cur_state->active_irq_id &&
> -	       in_sleepable(env);
> +	       env->cur_state->in_sleepable;
>  }

The in_rcu_cs() and in_sleepable_context() are complementary:

  env->cur_state->active_rcu_locks ||       !env->cur_state->active_rcu_locks &&
  env->cur_state->active_preempt_locks ||   !env->cur_state->active_preempt_locks &&
  env->cur_state->active_locks ||           !env->cur_state->active_locks &&
  env->cur_state->active_irq_id ||          !env->cur_state->active_irq_id &&
  !env->cur_state->in_sleepable;            env->cur_state->in_sleepable;

Should the one be expressed through the other?

...

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH bpf-next v1 1/2] bpf: Fix sleepable context checks and remove in_sleepable helper
  2026-08-06 16:40 ` [PATCH bpf-next v1 1/2] bpf: Fix sleepable context checks and remove in_sleepable helper Kumar Kartikeya Dwivedi
  2026-08-06 17:14   ` sashiko-bot
  2026-08-06 20:31   ` Eduard Zingerman
@ 2026-08-06 21:10   ` Eduard Zingerman
  2026-08-06 21:20     ` Kumar Kartikeya Dwivedi
  2 siblings, 1 reply; 10+ messages in thread
From: Eduard Zingerman @ 2026-08-06 21:10 UTC (permalink / raw)
  To: Kumar Kartikeya Dwivedi, bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Emil Tsalapatis, kkd, kernel-team

On Thu, 2026-08-06 at 18:40 +0200, Kumar Kartikeya Dwivedi wrote:

...

> Several call sites incorrectly used in_sleepable() where
> in_sleepable_context() is required. This allowed sleepable programs
> in non-sleepable contexts (e.g., inside bpf_rcu_read_lock()) to
> incorrectly use sleepable iterators and kfuncs:
> 
>   - check_css_task_iter_allowlist() returned in_sleepable() and
>     therefore allowed css_task iterator in any sleepable program,
>     even inside RCU/preempt/lock/IRQ-disabled regions. Fix it to
>     use in_sleepable_context().

...

> @@ -12083,7 +12078,7 @@ static bool check_css_task_iter_allowlist(struct bpf_verifier_env *env)
>  			return true;
>  		fallthrough;
>  	default:
> -		return in_sleepable(env);
> +		return in_sleepable_context(env);
>  	}
>  }

I don't understand this change. The comment on top of the
check_css_task_iter_allowlist() says that it is about
cgroup.c:css_set_lock, not the current context per se.

The flags on bpf_iter_css_task_new() and bpf_iter_css_task_next()
is what should govern the decision regarding whether the function is
allowed within some kind of a critical section.

  BTF_ID_FLAGS(func, bpf_iter_css_task_new, KF_ITER_NEW)
  BTF_ID_FLAGS(func, bpf_iter_css_task_next, KF_ITER_NEXT | KF_RET_NULL)

Should these wield a KF_SLEEPABLE?

Looking at the bpf_iter_css_task_new() body, it uses bpf_mem_alloc()
which does not sleep and css_task_iter_start() which takes and
releases a spin lock.

Please elaborate.

...

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH bpf-next v1 1/2] bpf: Fix sleepable context checks and remove in_sleepable helper
  2026-08-06 21:10   ` Eduard Zingerman
@ 2026-08-06 21:20     ` Kumar Kartikeya Dwivedi
  2026-08-06 21:46       ` Kumar Kartikeya Dwivedi
  0 siblings, 1 reply; 10+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2026-08-06 21:20 UTC (permalink / raw)
  To: Eduard Zingerman, bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Emil Tsalapatis, kkd, kernel-team

On Thu Aug 6, 2026 at 11:10 PM CEST, Eduard Zingerman wrote:
> On Thu, 2026-08-06 at 18:40 +0200, Kumar Kartikeya Dwivedi wrote:
>
> ...
>
>> Several call sites incorrectly used in_sleepable() where
>> in_sleepable_context() is required. This allowed sleepable programs
>> in non-sleepable contexts (e.g., inside bpf_rcu_read_lock()) to
>> incorrectly use sleepable iterators and kfuncs:
>>
>>   - check_css_task_iter_allowlist() returned in_sleepable() and
>>     therefore allowed css_task iterator in any sleepable program,
>>     even inside RCU/preempt/lock/IRQ-disabled regions. Fix it to
>>     use in_sleepable_context().
>
> ...
>
>> @@ -12083,7 +12078,7 @@ static bool check_css_task_iter_allowlist(struct bpf_verifier_env *env)
>>  			return true;
>>  		fallthrough;
>>  	default:
>> -		return in_sleepable(env);
>> +		return in_sleepable_context(env);
>>  	}
>>  }
>
> I don't understand this change. The comment on top of the
> check_css_task_iter_allowlist() says that it is about
> cgroup.c:css_set_lock, not the current context per se.

I haven't looked deeply into this on whether this should change, that said the
usage of in_sleepable() is obviously wrong here, in that it is not enough.

>
> The flags on bpf_iter_css_task_new() and bpf_iter_css_task_next()
> is what should govern the decision regarding whether the function is
> allowed within some kind of a critical section.
>
>   BTF_ID_FLAGS(func, bpf_iter_css_task_new, KF_ITER_NEW)
>   BTF_ID_FLAGS(func, bpf_iter_css_task_next, KF_ITER_NEXT | KF_RET_NULL)
>
> Should these wield a KF_SLEEPABLE?
>
> Looking at the bpf_iter_css_task_new() body, it uses bpf_mem_alloc()
> which does not sleep and css_task_iter_start() which takes and
> releases a spin lock.
>
> Please elaborate.
>

Going back to
https://lore.kernel.org/all/272de0e9-539c-4d89-9b9c-0652b0826cdd@bytedance.com,
the reasoning certainly seems a bit dubious, esp. with changes since.  In any
case I will take a look at it tomorrow, but I think the conversion is ok. We
might also want to use a filter instead of hardcoding this into the verifier.

> ...


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH bpf-next v1 1/2] bpf: Fix sleepable context checks and remove in_sleepable helper
  2026-08-06 20:31   ` Eduard Zingerman
@ 2026-08-06 21:22     ` Kumar Kartikeya Dwivedi
  2026-08-06 22:17       ` Eduard Zingerman
  0 siblings, 1 reply; 10+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2026-08-06 21:22 UTC (permalink / raw)
  To: Eduard Zingerman, bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Emil Tsalapatis, kkd, kernel-team

On Thu Aug 6, 2026 at 10:31 PM CEST, Eduard Zingerman wrote:
> On Thu, 2026-08-06 at 18:40 +0200, Kumar Kartikeya Dwivedi wrote:
>
> ...
>
>> @@ -10258,7 +10253,7 @@ static inline bool in_sleepable_context(struct bpf_verifier_env *env)
>>  	       !env->cur_state->active_preempt_locks &&
>>  	       !env->cur_state->active_locks &&
>>  	       !env->cur_state->active_irq_id &&
>> -	       in_sleepable(env);
>> +	       env->cur_state->in_sleepable;
>>  }
>
> The in_rcu_cs() and in_sleepable_context() are complementary:
>
>   env->cur_state->active_rcu_locks ||       !env->cur_state->active_rcu_locks &&
>   env->cur_state->active_preempt_locks ||   !env->cur_state->active_preempt_locks &&
>   env->cur_state->active_locks ||           !env->cur_state->active_locks &&
>   env->cur_state->active_irq_id ||          !env->cur_state->active_irq_id &&
>   !env->cur_state->in_sleepable;            env->cur_state->in_sleepable;
>
> Should the one be expressed through the other?
>

I thought about this, and I really can't decide, so I'll let others shape the
decision. On one hand, it is true, but I am just worried we have some other way
of disabling sleepable context in the future, for which we modify
in_sleepable_context(), but that thing does not imply RCU CS. I don't know
whether we will have such a case though.

We can also add a comment to revisit in_rcu_cs() and keep it in sync everytime
in_sleepable_context() is changed though, and hopefully reviewers (human and AI)
will be reminded to not break the relationship between both.

> ...


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH bpf-next v1 1/2] bpf: Fix sleepable context checks and remove in_sleepable helper
  2026-08-06 21:20     ` Kumar Kartikeya Dwivedi
@ 2026-08-06 21:46       ` Kumar Kartikeya Dwivedi
  0 siblings, 0 replies; 10+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2026-08-06 21:46 UTC (permalink / raw)
  To: Eduard Zingerman, bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Emil Tsalapatis, kkd, kernel-team

On Thu Aug 6, 2026 at 11:20 PM CEST, Kumar Kartikeya Dwivedi wrote:
> On Thu Aug 6, 2026 at 11:10 PM CEST, Eduard Zingerman wrote:
>> On Thu, 2026-08-06 at 18:40 +0200, Kumar Kartikeya Dwivedi wrote:
>>
>> ...
>>
>>> Several call sites incorrectly used in_sleepable() where
>>> in_sleepable_context() is required. This allowed sleepable programs
>>> in non-sleepable contexts (e.g., inside bpf_rcu_read_lock()) to
>>> incorrectly use sleepable iterators and kfuncs:
>>>
>>>   - check_css_task_iter_allowlist() returned in_sleepable() and
>>>     therefore allowed css_task iterator in any sleepable program,
>>>     even inside RCU/preempt/lock/IRQ-disabled regions. Fix it to
>>>     use in_sleepable_context().
>>
>> ...
>>
>>> @@ -12083,7 +12078,7 @@ static bool check_css_task_iter_allowlist(struct bpf_verifier_env *env)
>>>  			return true;
>>>  		fallthrough;
>>>  	default:
>>> -		return in_sleepable(env);
>>> +		return in_sleepable_context(env);
>>>  	}
>>>  }
>>
>> I don't understand this change. The comment on top of the
>> check_css_task_iter_allowlist() says that it is about
>> cgroup.c:css_set_lock, not the current context per se.
>
> I haven't looked deeply into this on whether this should change, that said the
> usage of in_sleepable() is obviously wrong here, in that it is not enough.
>

Ok, I looked at it.

I guess I see what you mean, sleepable programs won't run inside css_set_lock
critical section, hence it should be ok to not check in_sleepable_context(). So
we probably only care about the program being sleepable, and not the context.

So the current thing should be ok except on PREEMPT_RT, when spin lock on
css_set_lock becomes a rt_mutex. It depends on whether we care enough to make it
context dependent on PREEMPT_RT and leave it open otherwise, or just let it be.

I can still move it to a filter outside the verifier though.

> [...]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH bpf-next v1 1/2] bpf: Fix sleepable context checks and remove in_sleepable helper
  2026-08-06 21:22     ` Kumar Kartikeya Dwivedi
@ 2026-08-06 22:17       ` Eduard Zingerman
  0 siblings, 0 replies; 10+ messages in thread
From: Eduard Zingerman @ 2026-08-06 22:17 UTC (permalink / raw)
  To: Kumar Kartikeya Dwivedi, bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Emil Tsalapatis, kkd, kernel-team

On Thu, 2026-08-06 at 23:22 +0200, Kumar Kartikeya Dwivedi wrote:
> On Thu Aug 6, 2026 at 10:31 PM CEST, Eduard Zingerman wrote:
> > On Thu, 2026-08-06 at 18:40 +0200, Kumar Kartikeya Dwivedi wrote:
> > 
> > ...
> > 
> > > @@ -10258,7 +10253,7 @@ static inline bool in_sleepable_context(struct bpf_verifier_env *env)
> > >  	       !env->cur_state->active_preempt_locks &&
> > >  	       !env->cur_state->active_locks &&
> > >  	       !env->cur_state->active_irq_id &&
> > > -	       in_sleepable(env);
> > > +	       env->cur_state->in_sleepable;
> > >  }
> > 
> > The in_rcu_cs() and in_sleepable_context() are complementary:
> > 
> >   env->cur_state->active_rcu_locks ||       !env->cur_state->active_rcu_locks &&
> >   env->cur_state->active_preempt_locks ||   !env->cur_state->active_preempt_locks &&
> >   env->cur_state->active_locks ||           !env->cur_state->active_locks &&
> >   env->cur_state->active_irq_id ||          !env->cur_state->active_irq_id &&
> >   !env->cur_state->in_sleepable;            env->cur_state->in_sleepable;
> > 
> > Should the one be expressed through the other?
> > 
> 
> I thought about this, and I really can't decide, so I'll let others shape the
> decision. On one hand, it is true, but I am just worried we have some other way
> of disabling sleepable context in the future, for which we modify
> in_sleepable_context(), but that thing does not imply RCU CS. I don't know
> whether we will have such a case though.
> 
> We can also add a comment to revisit in_rcu_cs() and keep it in sync everytime
> in_sleepable_context() is changed though, and hopefully reviewers (human and AI)
> will be reminded to not break the relationship between both.

I'd declare in_rcu_cs() as !in_sleepable_context() and put both
functions side-by-side in the verifier.c.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2026-08-06 22:17 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-06 16:40 [PATCH bpf-next v1 0/2] Fix sleepable context checks in verifier Kumar Kartikeya Dwivedi
2026-08-06 16:40 ` [PATCH bpf-next v1 1/2] bpf: Fix sleepable context checks and remove in_sleepable helper Kumar Kartikeya Dwivedi
2026-08-06 17:14   ` sashiko-bot
2026-08-06 20:31   ` Eduard Zingerman
2026-08-06 21:22     ` Kumar Kartikeya Dwivedi
2026-08-06 22:17       ` Eduard Zingerman
2026-08-06 21:10   ` Eduard Zingerman
2026-08-06 21:20     ` Kumar Kartikeya Dwivedi
2026-08-06 21:46       ` Kumar Kartikeya Dwivedi
2026-08-06 16:40 ` [PATCH bpf-next v1 2/2] selftests/bpf: Test sleepable context distinctions Kumar Kartikeya Dwivedi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox