All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.