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.

> [...]
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.