Re: [PATCH bpf-next v1 1/2] bpf: Fix sleepable context checks and remove in_sleepable helper
"Kumar Kartikeya Dwivedi" <[email protected]>
| Newsgroups | org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
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. > [...]