Re: [PATCH bpf-next] bpf: support nested rcu critical sections

[Puranjay Mohan <puranjay@kernel.org>]

Leon Hwang <hffilwlqm@gmail.com> writes:

> On 16/9/25 19:36, Puranjay Mohan wrote:
>> Currently, nested rcu critical sections are rejected by the verifier and
>> rcu_lock state is managed by a boolean variable. Add support for nested
>> rcu critical sections by make active_rcu_locks a counter similar to
>> active_preempt_locks. bpf_rcu_read_lock() increments this counter and
>> bpf_rcu_read_unlock() decrements it, MEM_RCU -> PTR_UNTRUSTED transition
>> happens when active_rcu_locks drops to 0.
>>
>> Signed-off-by: Puranjay Mohan <puranjay@kernel.org>
>> ---
>
> [...]
>
>> @@ -13863,7 +13863,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>>  	preempt_disable = is_kfunc_bpf_preempt_disable(&meta);
>>  	preempt_enable = is_kfunc_bpf_preempt_enable(&meta);
>>
>> -	if (env->cur_state->active_rcu_lock) {
>> +	if (env->cur_state->active_rcu_locks) {
>>  		struct bpf_func_state *state;
>>  		struct bpf_reg_state *reg;
>>  		u32 clear_mask = (1 << STACK_SPILL) | (1 << STACK_ITER);
>> @@ -13874,22 +13874,22 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>>  		}
>>
>>  		if (rcu_lock) {
>> -			verbose(env, "nested rcu read lock (kernel function %s)\n", func_name);
>> -			return -EINVAL;
>> +			env->cur_state->active_rcu_locks++;
>
> Could we add a check for the maximum of 'active_rcu_locks'?
>
> From a cracker's perspective, this could potentially be abused to
> stall the kernel or trigger a deadlock. Underneath 'rcu_read_lock()',
> there are several RCU functions that tracing programs are able to
> attach to. If those functions are traced, a deadlock can be triggered.
>

IIUC what you are saying is that if I attach a BPF tracing program to
something under rcu_read_lock() and then call bpf_rcu_read_lock() in the
BPF program then there could recursion? Wouldn't that be triggered even
with a single call to bpf_rcu_read_lock() ?

Thanks,
Puranjay