diff mbox series

[bpf-next,v3,5/7] bpf: teach the verifier to enforce css_iter and task_iter in RCU CS

Message ID 20230925105552.817513-6-zhouchuyi@bytedance.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series Add Open-coded task, css_task and css iters | expand

Checks

Context Check Description
netdev/tree_selection success Clearly marked for bpf-next, async
netdev/apply fail Patch does not apply to bpf-next
bpf/vmtest-bpf-next-VM_Test-2 success Logs for build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-3 success Logs for build for x86_64 with gcc
bpf/vmtest-bpf-next-PR success PR summary
bpf/vmtest-bpf-next-VM_Test-0 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-4 success Logs for build for x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-1 success Logs for build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-5 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-6 success Logs for test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for test_maps on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-8 success Logs for test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for test_maps on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-10 success Logs for test_progs on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-11 success Logs for test_progs on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-12 success Logs for test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for test_progs on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-14 success Logs for test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-15 success Logs for test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-16 success Logs for test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-17 success Logs for test_progs_no_alu32 on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-18 success Logs for test_progs_no_alu32_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-19 success Logs for test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-21 success Logs for test_progs_parallel on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-22 success Logs for test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for test_progs_parallel on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-24 success Logs for test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for test_verifier on s390x with gcc
bpf/vmtest-bpf-next-VM_Test-26 success Logs for test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-27 success Logs for test_verifier on x86_64 with llvm-16
bpf/vmtest-bpf-next-VM_Test-28 success Logs for veristat

Commit Message

Chuyi Zhou Sept. 25, 2023, 10:55 a.m. UTC 
css_iter and task_iter should be used in rcu section. Specifically, in
sleepable progs explicit bpf_rcu_read_lock() is needed before use these
iters. In normal bpf progs that have implicit rcu_read_lock(), it's OK to
use them directly.

This patch adds a new a KF flag KF_RCU_PROTECTED for bpf_iter_task_new and
bpf_iter_css_new. It means the kfunc should be used in RCU CS. We check
whether we are in rcu cs before we want to invoke this kfunc. If the rcu
protection is guaranteed, we would let st->type = PTR_TO_STACK | MEM_RCU.
Once user do rcu_unlock during the iteration, state MEM_RCU of regs would
be cleared. is_iter_reg_valid_init() will reject if reg->type is UNTRUSTED.

It is worth noting that currently, bpf_rcu_read_unlock does not
clear the state of the STACK_ITER reg, since bpf_for_each_spilled_reg
only considers STACK_SPILL. This patch also let bpf_for_each_spilled_reg
search STACK_ITER.

Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
---
 include/linux/bpf_verifier.h | 19 ++++++++------
 include/linux/btf.h          |  1 +
 kernel/bpf/helpers.c         |  4 +--
 kernel/bpf/verifier.c        | 48 +++++++++++++++++++++++++++---------
 4 files changed, 50 insertions(+), 22 deletions(-)

Comments

Yafang Shao Sept. 27, 2023, 10 a.m. UTC | #1 
On Mon, Sep 25, 2023 at 6:56 PM Chuyi Zhou <zhouchuyi@bytedance.com> wrote:
>
> css_iter and task_iter should be used in rcu section. Specifically, in
> sleepable progs explicit bpf_rcu_read_lock() is needed before use these
> iters. In normal bpf progs that have implicit rcu_read_lock(), it's OK to
> use them directly.
>
> This patch adds a new a KF flag KF_RCU_PROTECTED for bpf_iter_task_new and
> bpf_iter_css_new. It means the kfunc should be used in RCU CS. We check
> whether we are in rcu cs before we want to invoke this kfunc. If the rcu
> protection is guaranteed, we would let st->type = PTR_TO_STACK | MEM_RCU.
> Once user do rcu_unlock during the iteration, state MEM_RCU of regs would
> be cleared. is_iter_reg_valid_init() will reject if reg->type is UNTRUSTED.
>
> It is worth noting that currently, bpf_rcu_read_unlock does not
> clear the state of the STACK_ITER reg, since bpf_for_each_spilled_reg
> only considers STACK_SPILL. This patch also let bpf_for_each_spilled_reg
> search STACK_ITER.
>
> Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>

This patch should be ahead of patch #2 and you introduce
KF_RCU_PROTECTED in it then use this flag in later patches.
BTW, I can't apply your series on bpf-next. I think you should rebase
it on the latest bpf-next, otherwise the BPF CI can't be triggered.

> ---
>  include/linux/bpf_verifier.h | 19 ++++++++------
>  include/linux/btf.h          |  1 +
>  kernel/bpf/helpers.c         |  4 +--
>  kernel/bpf/verifier.c        | 48 +++++++++++++++++++++++++++---------
>  4 files changed, 50 insertions(+), 22 deletions(-)
>
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index a3236651ec64..b5cdcc332b0a 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -385,19 +385,18 @@ struct bpf_verifier_state {
>         u32 jmp_history_cnt;
>  };
>
> -#define bpf_get_spilled_reg(slot, frame)                               \
> +#define bpf_get_spilled_reg(slot, frame, mask)                         \
>         (((slot < frame->allocated_stack / BPF_REG_SIZE) &&             \
> -         (frame->stack[slot].slot_type[0] == STACK_SPILL))             \
> +         ((1 << frame->stack[slot].slot_type[0]) & (mask))) \
>          ? &frame->stack[slot].spilled_ptr : NULL)
>
>  /* Iterate over 'frame', setting 'reg' to either NULL or a spilled register. */
> -#define bpf_for_each_spilled_reg(iter, frame, reg)                     \
> -       for (iter = 0, reg = bpf_get_spilled_reg(iter, frame);          \
> +#define bpf_for_each_spilled_reg(iter, frame, reg, mask)                       \
> +       for (iter = 0, reg = bpf_get_spilled_reg(iter, frame, mask);            \
>              iter < frame->allocated_stack / BPF_REG_SIZE;              \
> -            iter++, reg = bpf_get_spilled_reg(iter, frame))
> +            iter++, reg = bpf_get_spilled_reg(iter, frame, mask))
>
> -/* Invoke __expr over regsiters in __vst, setting __state and __reg */
> -#define bpf_for_each_reg_in_vstate(__vst, __state, __reg, __expr)   \
> +#define bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, __mask, __expr)   \
>         ({                                                               \
>                 struct bpf_verifier_state *___vstate = __vst;            \
>                 int ___i, ___j;                                          \
> @@ -409,7 +408,7 @@ struct bpf_verifier_state {
>                                 __reg = &___regs[___j];                  \
>                                 (void)(__expr);                          \
>                         }                                                \
> -                       bpf_for_each_spilled_reg(___j, __state, __reg) { \
> +                       bpf_for_each_spilled_reg(___j, __state, __reg, __mask) { \
>                                 if (!__reg)                              \
>                                         continue;                        \
>                                 (void)(__expr);                          \
> @@ -417,6 +416,10 @@ struct bpf_verifier_state {
>                 }                                                        \
>         })
>
> +/* Invoke __expr over regsiters in __vst, setting __state and __reg */
> +#define bpf_for_each_reg_in_vstate(__vst, __state, __reg, __expr) \
> +       bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, 1 << STACK_SPILL, __expr)
> +
>  /* linked list of verifier states used to prune search */
>  struct bpf_verifier_state_list {
>         struct bpf_verifier_state state;
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index 928113a80a95..c2231c64d60b 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -74,6 +74,7 @@
>  #define KF_ITER_NEW     (1 << 8) /* kfunc implements BPF iter constructor */
>  #define KF_ITER_NEXT    (1 << 9) /* kfunc implements BPF iter next method */
>  #define KF_ITER_DESTROY (1 << 10) /* kfunc implements BPF iter destructor */
> +#define KF_RCU_PROTECTED (1 << 11) /* kfunc should be protected by rcu cs when they are invoked */
>
>  /*
>   * Tag marking a kernel function as a kfunc. This is meant to minimize the
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 9c3af36249a2..aa9e03fbfe1a 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2507,10 +2507,10 @@ BTF_ID_FLAGS(func, bpf_iter_num_destroy, KF_ITER_DESTROY)
>  BTF_ID_FLAGS(func, bpf_iter_css_task_new, KF_ITER_NEW | KF_TRUSTED_ARGS)
>  BTF_ID_FLAGS(func, bpf_iter_css_task_next, KF_ITER_NEXT | KF_RET_NULL)
>  BTF_ID_FLAGS(func, bpf_iter_css_task_destroy, KF_ITER_DESTROY)
> -BTF_ID_FLAGS(func, bpf_iter_task_new, KF_ITER_NEW | KF_TRUSTED_ARGS)
> +BTF_ID_FLAGS(func, bpf_iter_task_new, KF_ITER_NEW | KF_TRUSTED_ARGS | KF_RCU_PROTECTED)
>  BTF_ID_FLAGS(func, bpf_iter_task_next, KF_ITER_NEXT | KF_RET_NULL)
>  BTF_ID_FLAGS(func, bpf_iter_task_destroy, KF_ITER_DESTROY)
> -BTF_ID_FLAGS(func, bpf_iter_css_new, KF_ITER_NEW | KF_TRUSTED_ARGS)
> +BTF_ID_FLAGS(func, bpf_iter_css_new, KF_ITER_NEW | KF_TRUSTED_ARGS | KF_RCU_PROTECTED)
>  BTF_ID_FLAGS(func, bpf_iter_css_next, KF_ITER_NEXT | KF_RET_NULL)
>  BTF_ID_FLAGS(func, bpf_iter_css_destroy, KF_ITER_DESTROY)
>  BTF_ID_FLAGS(func, bpf_dynptr_adjust)
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 2367483bf4c2..a065e18a0b3a 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -1172,7 +1172,12 @@ static bool is_dynptr_type_expected(struct bpf_verifier_env *env, struct bpf_reg
>
>  static void __mark_reg_known_zero(struct bpf_reg_state *reg);
>
> +static bool in_rcu_cs(struct bpf_verifier_env *env);
> +
> +static bool is_kfunc_rcu_protected(struct bpf_kfunc_call_arg_meta *meta);
> +
>  static int mark_stack_slots_iter(struct bpf_verifier_env *env,
> +                                struct bpf_kfunc_call_arg_meta *meta,
>                                  struct bpf_reg_state *reg, int insn_idx,
>                                  struct btf *btf, u32 btf_id, int nr_slots)
>  {
> @@ -1193,6 +1198,12 @@ static int mark_stack_slots_iter(struct bpf_verifier_env *env,
>
>                 __mark_reg_known_zero(st);
>                 st->type = PTR_TO_STACK; /* we don't have dedicated reg type */
> +               if (is_kfunc_rcu_protected(meta)) {
> +                       if (in_rcu_cs(env))
> +                               st->type |= MEM_RCU;

I think this change is incorrect.  The type of st->type is enum
bpf_reg_type, but MEM_RCU is enum bpf_type_flag.
Or am I missing something?

> +                       else
> +                               st->type |= PTR_UNTRUSTED;
> +               }
>                 st->live |= REG_LIVE_WRITTEN;
>                 st->ref_obj_id = i == 0 ? id : 0;
>                 st->iter.btf = btf;
> @@ -1267,7 +1278,7 @@ static bool is_iter_reg_valid_uninit(struct bpf_verifier_env *env,
>         return true;
>  }
>
> -static bool is_iter_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
> +static int is_iter_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
>                                    struct btf *btf, u32 btf_id, int nr_slots)
>  {
>         struct bpf_func_state *state = func(env, reg);
> @@ -1275,26 +1286,28 @@ static bool is_iter_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_
>
>         spi = iter_get_spi(env, reg, nr_slots);
>         if (spi < 0)
> -               return false;
> +               return -EINVAL;
>
>         for (i = 0; i < nr_slots; i++) {
>                 struct bpf_stack_state *slot = &state->stack[spi - i];
>                 struct bpf_reg_state *st = &slot->spilled_ptr;
>
> +               if (st->type & PTR_UNTRUSTED)
> +                       return -EPERM;
>                 /* only main (first) slot has ref_obj_id set */
>                 if (i == 0 && !st->ref_obj_id)
> -                       return false;
> +                       return -EINVAL;
>                 if (i != 0 && st->ref_obj_id)
> -                       return false;
> +                       return -EINVAL;
>                 if (st->iter.btf != btf || st->iter.btf_id != btf_id)
> -                       return false;
> +                       return -EINVAL;
>
>                 for (j = 0; j < BPF_REG_SIZE; j++)
>                         if (slot->slot_type[j] != STACK_ITER)
> -                               return false;
> +                               return -EINVAL;
>         }
>
> -       return true;
> +       return 0;
>  }
>
>  /* Check if given stack slot is "special":
> @@ -7503,15 +7516,20 @@ static int process_iter_arg(struct bpf_verifier_env *env, int regno, int insn_id
>                                 return err;
>                 }
>
> -               err = mark_stack_slots_iter(env, reg, insn_idx, meta->btf, btf_id, nr_slots);
> +               err = mark_stack_slots_iter(env, meta, reg, insn_idx, meta->btf, btf_id, nr_slots);
>                 if (err)
>                         return err;
>         } else {
>                 /* iter_next() or iter_destroy() expect initialized iter state*/
> -               if (!is_iter_reg_valid_init(env, reg, meta->btf, btf_id, nr_slots)) {
> -                       verbose(env, "expected an initialized iter_%s as arg #%d\n",
> +               err = is_iter_reg_valid_init(env, reg, meta->btf, btf_id, nr_slots);
> +               switch (err) {
> +               case -EINVAL:
> +                       verbose(env, "expected an initialized iter_%s as arg #%d or without bpf_rcu_read_lock()\n",
>                                 iter_type_str(meta->btf, btf_id), regno);
> -                       return -EINVAL;
> +                       return err;
> +               case -EPERM:
> +                       verbose(env, "expected an RCU CS when using %s\n", meta->func_name);
> +                       return err;
>                 }
>
>                 spi = iter_get_spi(env, reg, nr_slots);
> @@ -10092,6 +10110,11 @@ static bool is_kfunc_rcu(struct bpf_kfunc_call_arg_meta *meta)
>         return meta->kfunc_flags & KF_RCU;
>  }
>
> +static bool is_kfunc_rcu_protected(struct bpf_kfunc_call_arg_meta *meta)
> +{
> +       return meta->kfunc_flags & KF_RCU_PROTECTED;
> +}
> +
>  static bool __kfunc_param_match_suffix(const struct btf *btf,
>                                        const struct btf_param *arg,
>                                        const char *suffix)
> @@ -11428,6 +11451,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>         if (env->cur_state->active_rcu_lock) {
>                 struct bpf_func_state *state;
>                 struct bpf_reg_state *reg;
> +               u32 clear_mask = (1 << STACK_SPILL) | (1 << STACK_ITER);
>
>                 if (in_rbtree_lock_required_cb(env) && (rcu_lock || rcu_unlock)) {
>                         verbose(env, "Calling bpf_rcu_read_{lock,unlock} in unnecessary rbtree callback\n");
> @@ -11438,7 +11462,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>                         verbose(env, "nested rcu read lock (kernel function %s)\n", func_name);
>                         return -EINVAL;
>                 } else if (rcu_unlock) {
> -                       bpf_for_each_reg_in_vstate(env->cur_state, state, reg, ({
> +                       bpf_for_each_reg_in_vstate_mask(env->cur_state, state, reg, clear_mask, ({
>                                 if (reg->type & MEM_RCU) {
>                                         reg->type &= ~(MEM_RCU | PTR_MAYBE_NULL);
>                                         reg->type |= PTR_UNTRUSTED;
> --
> 2.20.1
>
>
Chuyi Zhou Sept. 27, 2023, 10:16 a.m. UTC | #2 
在 2023/9/27 18:00, Yafang Shao 写道:
> On Mon, Sep 25, 2023 at 6:56 PM Chuyi Zhou <zhouchuyi@bytedance.com> wrote:
>>
>> css_iter and task_iter should be used in rcu section. Specifically, in
>> sleepable progs explicit bpf_rcu_read_lock() is needed before use these
>> iters. In normal bpf progs that have implicit rcu_read_lock(), it's OK to
>> use them directly.
>>
>> This patch adds a new a KF flag KF_RCU_PROTECTED for bpf_iter_task_new and
>> bpf_iter_css_new. It means the kfunc should be used in RCU CS. We check
>> whether we are in rcu cs before we want to invoke this kfunc. If the rcu
>> protection is guaranteed, we would let st->type = PTR_TO_STACK | MEM_RCU.
>> Once user do rcu_unlock during the iteration, state MEM_RCU of regs would
>> be cleared. is_iter_reg_valid_init() will reject if reg->type is UNTRUSTED.
>>
>> It is worth noting that currently, bpf_rcu_read_unlock does not
>> clear the state of the STACK_ITER reg, since bpf_for_each_spilled_reg
>> only considers STACK_SPILL. This patch also let bpf_for_each_spilled_reg
>> search STACK_ITER.
>>
>> Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
> 
> This patch should be ahead of patch #2 and you introduce
> KF_RCU_PROTECTED in it then use this flag in later patches.
> BTW, I can't apply your series on bpf-next. I think you should rebase
> it on the latest bpf-next, otherwise the BPF CI can't be triggered.
> 

Sorry for the mistake, will rebase in v4.

>> ---
>>   include/linux/bpf_verifier.h | 19 ++++++++------
>>   include/linux/btf.h          |  1 +
>>   kernel/bpf/helpers.c         |  4 +--
>>   kernel/bpf/verifier.c        | 48 +++++++++++++++++++++++++++---------
>>   4 files changed, 50 insertions(+), 22 deletions(-)
>>
>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
>> index a3236651ec64..b5cdcc332b0a 100644
>> --- a/include/linux/bpf_verifier.h
>> +++ b/include/linux/bpf_verifier.h
>> @@ -385,19 +385,18 @@ struct bpf_verifier_state {
>>          u32 jmp_history_cnt;
>>   };
>>
>> -#define bpf_get_spilled_reg(slot, frame)                               \
>> +#define bpf_get_spilled_reg(slot, frame, mask)                         \
>>          (((slot < frame->allocated_stack / BPF_REG_SIZE) &&             \
>> -         (frame->stack[slot].slot_type[0] == STACK_SPILL))             \
>> +         ((1 << frame->stack[slot].slot_type[0]) & (mask))) \
>>           ? &frame->stack[slot].spilled_ptr : NULL)
>>
>>   /* Iterate over 'frame', setting 'reg' to either NULL or a spilled register. */
>> -#define bpf_for_each_spilled_reg(iter, frame, reg)                     \
>> -       for (iter = 0, reg = bpf_get_spilled_reg(iter, frame);          \
>> +#define bpf_for_each_spilled_reg(iter, frame, reg, mask)                       \
>> +       for (iter = 0, reg = bpf_get_spilled_reg(iter, frame, mask);            \
>>               iter < frame->allocated_stack / BPF_REG_SIZE;              \
>> -            iter++, reg = bpf_get_spilled_reg(iter, frame))
>> +            iter++, reg = bpf_get_spilled_reg(iter, frame, mask))
>>
>> -/* Invoke __expr over regsiters in __vst, setting __state and __reg */
>> -#define bpf_for_each_reg_in_vstate(__vst, __state, __reg, __expr)   \
>> +#define bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, __mask, __expr)   \
>>          ({                                                               \
>>                  struct bpf_verifier_state *___vstate = __vst;            \
>>                  int ___i, ___j;                                          \
>> @@ -409,7 +408,7 @@ struct bpf_verifier_state {
>>                                  __reg = &___regs[___j];                  \
>>                                  (void)(__expr);                          \
>>                          }                                                \
>> -                       bpf_for_each_spilled_reg(___j, __state, __reg) { \
>> +                       bpf_for_each_spilled_reg(___j, __state, __reg, __mask) { \
>>                                  if (!__reg)                              \
>>                                          continue;                        \
>>                                  (void)(__expr);                          \
>> @@ -417,6 +416,10 @@ struct bpf_verifier_state {
>>                  }                                                        \
>>          })
>>
>> +/* Invoke __expr over regsiters in __vst, setting __state and __reg */
>> +#define bpf_for_each_reg_in_vstate(__vst, __state, __reg, __expr) \
>> +       bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, 1 << STACK_SPILL, __expr)
>> +
>>   /* linked list of verifier states used to prune search */
>>   struct bpf_verifier_state_list {
>>          struct bpf_verifier_state state;
>> diff --git a/include/linux/btf.h b/include/linux/btf.h
>> index 928113a80a95..c2231c64d60b 100644
>> --- a/include/linux/btf.h
>> +++ b/include/linux/btf.h
>> @@ -74,6 +74,7 @@
>>   #define KF_ITER_NEW     (1 << 8) /* kfunc implements BPF iter constructor */
>>   #define KF_ITER_NEXT    (1 << 9) /* kfunc implements BPF iter next method */
>>   #define KF_ITER_DESTROY (1 << 10) /* kfunc implements BPF iter destructor */
>> +#define KF_RCU_PROTECTED (1 << 11) /* kfunc should be protected by rcu cs when they are invoked */
>>
>>   /*
>>    * Tag marking a kernel function as a kfunc. This is meant to minimize the
>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
>> index 9c3af36249a2..aa9e03fbfe1a 100644
>> --- a/kernel/bpf/helpers.c
>> +++ b/kernel/bpf/helpers.c
>> @@ -2507,10 +2507,10 @@ BTF_ID_FLAGS(func, bpf_iter_num_destroy, KF_ITER_DESTROY)
>>   BTF_ID_FLAGS(func, bpf_iter_css_task_new, KF_ITER_NEW | KF_TRUSTED_ARGS)
>>   BTF_ID_FLAGS(func, bpf_iter_css_task_next, KF_ITER_NEXT | KF_RET_NULL)
>>   BTF_ID_FLAGS(func, bpf_iter_css_task_destroy, KF_ITER_DESTROY)
>> -BTF_ID_FLAGS(func, bpf_iter_task_new, KF_ITER_NEW | KF_TRUSTED_ARGS)
>> +BTF_ID_FLAGS(func, bpf_iter_task_new, KF_ITER_NEW | KF_TRUSTED_ARGS | KF_RCU_PROTECTED)
>>   BTF_ID_FLAGS(func, bpf_iter_task_next, KF_ITER_NEXT | KF_RET_NULL)
>>   BTF_ID_FLAGS(func, bpf_iter_task_destroy, KF_ITER_DESTROY)
>> -BTF_ID_FLAGS(func, bpf_iter_css_new, KF_ITER_NEW | KF_TRUSTED_ARGS)
>> +BTF_ID_FLAGS(func, bpf_iter_css_new, KF_ITER_NEW | KF_TRUSTED_ARGS | KF_RCU_PROTECTED)
>>   BTF_ID_FLAGS(func, bpf_iter_css_next, KF_ITER_NEXT | KF_RET_NULL)
>>   BTF_ID_FLAGS(func, bpf_iter_css_destroy, KF_ITER_DESTROY)
>>   BTF_ID_FLAGS(func, bpf_dynptr_adjust)
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 2367483bf4c2..a065e18a0b3a 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -1172,7 +1172,12 @@ static bool is_dynptr_type_expected(struct bpf_verifier_env *env, struct bpf_reg
>>
>>   static void __mark_reg_known_zero(struct bpf_reg_state *reg);
>>
>> +static bool in_rcu_cs(struct bpf_verifier_env *env);
>> +
>> +static bool is_kfunc_rcu_protected(struct bpf_kfunc_call_arg_meta *meta);
>> +
>>   static int mark_stack_slots_iter(struct bpf_verifier_env *env,
>> +                                struct bpf_kfunc_call_arg_meta *meta,
>>                                   struct bpf_reg_state *reg, int insn_idx,
>>                                   struct btf *btf, u32 btf_id, int nr_slots)
>>   {
>> @@ -1193,6 +1198,12 @@ static int mark_stack_slots_iter(struct bpf_verifier_env *env,
>>
>>                  __mark_reg_known_zero(st);
>>                  st->type = PTR_TO_STACK; /* we don't have dedicated reg type */
>> +               if (is_kfunc_rcu_protected(meta)) {
>> +                       if (in_rcu_cs(env))
>> +                               st->type |= MEM_RCU;
> 
> I think this change is incorrect.  The type of st->type is enum
> bpf_reg_type, but MEM_RCU is enum bpf_type_flag.
> Or am I missing something?
Looking at is_rcu_reg(), It seems OK to add MEM_RCU flag to st->type.

static bool is_rcu_reg(const struct bpf_reg_state *reg)
{
	return reg->type & MEM_RCU;
}

Here is the previous discussion link:
https://lore.kernel.org/lkml/CAADnVQKu+a6MKKfJy8NVmwtpEw1ae-_8opsGjdvvfoUjwE1sog@mail.gmail.com/

Thanks.

> 
>> +                       else
>> +                               st->type |= PTR_UNTRUSTED;
>> +               }
>>                  st->live |= REG_LIVE_WRITTEN;
>>                  st->ref_obj_id = i == 0 ? id : 0;
>>                  st->iter.btf = btf;
>> @@ -1267,7 +1278,7 @@ static bool is_iter_reg_valid_uninit(struct bpf_verifier_env *env,
>>          return true;
>>   }
>>
>> -static bool is_iter_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
>> +static int is_iter_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
>>                                     struct btf *btf, u32 btf_id, int nr_slots)
>>   {
>>          struct bpf_func_state *state = func(env, reg);
>> @@ -1275,26 +1286,28 @@ static bool is_iter_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_
>>
>>          spi = iter_get_spi(env, reg, nr_slots);
>>          if (spi < 0)
>> -               return false;
>> +               return -EINVAL;
>>
>>          for (i = 0; i < nr_slots; i++) {
>>                  struct bpf_stack_state *slot = &state->stack[spi - i];
>>                  struct bpf_reg_state *st = &slot->spilled_ptr;
>>
>> +               if (st->type & PTR_UNTRUSTED)
>> +                       return -EPERM;
>>                  /* only main (first) slot has ref_obj_id set */
>>                  if (i == 0 && !st->ref_obj_id)
>> -                       return false;
>> +                       return -EINVAL;
>>                  if (i != 0 && st->ref_obj_id)
>> -                       return false;
>> +                       return -EINVAL;
>>                  if (st->iter.btf != btf || st->iter.btf_id != btf_id)
>> -                       return false;
>> +                       return -EINVAL;
>>
>>                  for (j = 0; j < BPF_REG_SIZE; j++)
>>                          if (slot->slot_type[j] != STACK_ITER)
>> -                               return false;
>> +                               return -EINVAL;
>>          }
>>
>> -       return true;
>> +       return 0;
>>   }
>>
>>   /* Check if given stack slot is "special":
>> @@ -7503,15 +7516,20 @@ static int process_iter_arg(struct bpf_verifier_env *env, int regno, int insn_id
>>                                  return err;
>>                  }
>>
>> -               err = mark_stack_slots_iter(env, reg, insn_idx, meta->btf, btf_id, nr_slots);
>> +               err = mark_stack_slots_iter(env, meta, reg, insn_idx, meta->btf, btf_id, nr_slots);
>>                  if (err)
>>                          return err;
>>          } else {
>>                  /* iter_next() or iter_destroy() expect initialized iter state*/
>> -               if (!is_iter_reg_valid_init(env, reg, meta->btf, btf_id, nr_slots)) {
>> -                       verbose(env, "expected an initialized iter_%s as arg #%d\n",
>> +               err = is_iter_reg_valid_init(env, reg, meta->btf, btf_id, nr_slots);
>> +               switch (err) {
>> +               case -EINVAL:
>> +                       verbose(env, "expected an initialized iter_%s as arg #%d or without bpf_rcu_read_lock()\n",
>>                                  iter_type_str(meta->btf, btf_id), regno);
>> -                       return -EINVAL;
>> +                       return err;
>> +               case -EPERM:
>> +                       verbose(env, "expected an RCU CS when using %s\n", meta->func_name);
>> +                       return err;
>>                  }
>>
>>                  spi = iter_get_spi(env, reg, nr_slots);
>> @@ -10092,6 +10110,11 @@ static bool is_kfunc_rcu(struct bpf_kfunc_call_arg_meta *meta)
>>          return meta->kfunc_flags & KF_RCU;
>>   }
>>
>> +static bool is_kfunc_rcu_protected(struct bpf_kfunc_call_arg_meta *meta)
>> +{
>> +       return meta->kfunc_flags & KF_RCU_PROTECTED;
>> +}
>> +
>>   static bool __kfunc_param_match_suffix(const struct btf *btf,
>>                                         const struct btf_param *arg,
>>                                         const char *suffix)
>> @@ -11428,6 +11451,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>>          if (env->cur_state->active_rcu_lock) {
>>                  struct bpf_func_state *state;
>>                  struct bpf_reg_state *reg;
>> +               u32 clear_mask = (1 << STACK_SPILL) | (1 << STACK_ITER);
>>
>>                  if (in_rbtree_lock_required_cb(env) && (rcu_lock || rcu_unlock)) {
>>                          verbose(env, "Calling bpf_rcu_read_{lock,unlock} in unnecessary rbtree callback\n");
>> @@ -11438,7 +11462,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>>                          verbose(env, "nested rcu read lock (kernel function %s)\n", func_name);
>>                          return -EINVAL;
>>                  } else if (rcu_unlock) {
>> -                       bpf_for_each_reg_in_vstate(env->cur_state, state, reg, ({
>> +                       bpf_for_each_reg_in_vstate_mask(env->cur_state, state, reg, clear_mask, ({
>>                                  if (reg->type & MEM_RCU) {
>>                                          reg->type &= ~(MEM_RCU | PTR_MAYBE_NULL);
>>                                          reg->type |= PTR_UNTRUSTED;
>> --
>> 2.20.1
>>
>>
> 
>
Andrii Nakryiko Sept. 27, 2023, 11:29 p.m. UTC | #3 
On Mon, Sep 25, 2023 at 3:56 AM Chuyi Zhou <zhouchuyi@bytedance.com> wrote:
>
> css_iter and task_iter should be used in rcu section. Specifically, in
> sleepable progs explicit bpf_rcu_read_lock() is needed before use these
> iters. In normal bpf progs that have implicit rcu_read_lock(), it's OK to
> use them directly.
>
> This patch adds a new a KF flag KF_RCU_PROTECTED for bpf_iter_task_new and
> bpf_iter_css_new. It means the kfunc should be used in RCU CS. We check
> whether we are in rcu cs before we want to invoke this kfunc. If the rcu
> protection is guaranteed, we would let st->type = PTR_TO_STACK | MEM_RCU.
> Once user do rcu_unlock during the iteration, state MEM_RCU of regs would
> be cleared. is_iter_reg_valid_init() will reject if reg->type is UNTRUSTED.
>
> It is worth noting that currently, bpf_rcu_read_unlock does not
> clear the state of the STACK_ITER reg, since bpf_for_each_spilled_reg
> only considers STACK_SPILL. This patch also let bpf_for_each_spilled_reg
> search STACK_ITER.
>
> Signed-off-by: Chuyi Zhou <zhouchuyi@bytedance.com>
> ---
>  include/linux/bpf_verifier.h | 19 ++++++++------
>  include/linux/btf.h          |  1 +
>  kernel/bpf/helpers.c         |  4 +--
>  kernel/bpf/verifier.c        | 48 +++++++++++++++++++++++++++---------
>  4 files changed, 50 insertions(+), 22 deletions(-)
>

[...]

>
> -static bool is_iter_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
> +static int is_iter_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
>                                    struct btf *btf, u32 btf_id, int nr_slots)
>  {
>         struct bpf_func_state *state = func(env, reg);
> @@ -1275,26 +1286,28 @@ static bool is_iter_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_
>
>         spi = iter_get_spi(env, reg, nr_slots);
>         if (spi < 0)
> -               return false;
> +               return -EINVAL;
>
>         for (i = 0; i < nr_slots; i++) {
>                 struct bpf_stack_state *slot = &state->stack[spi - i];
>                 struct bpf_reg_state *st = &slot->spilled_ptr;
>
> +               if (st->type & PTR_UNTRUSTED)
> +                       return -EPERM;
>                 /* only main (first) slot has ref_obj_id set */
>                 if (i == 0 && !st->ref_obj_id)
> -                       return false;
> +                       return -EINVAL;
>                 if (i != 0 && st->ref_obj_id)
> -                       return false;
> +                       return -EINVAL;
>                 if (st->iter.btf != btf || st->iter.btf_id != btf_id)
> -                       return false;
> +                       return -EINVAL;
>
>                 for (j = 0; j < BPF_REG_SIZE; j++)
>                         if (slot->slot_type[j] != STACK_ITER)
> -                               return false;
> +                               return -EINVAL;
>         }
>
> -       return true;
> +       return 0;
>  }
>
>  /* Check if given stack slot is "special":
> @@ -7503,15 +7516,20 @@ static int process_iter_arg(struct bpf_verifier_env *env, int regno, int insn_id
>                                 return err;
>                 }
>
> -               err = mark_stack_slots_iter(env, reg, insn_idx, meta->btf, btf_id, nr_slots);
> +               err = mark_stack_slots_iter(env, meta, reg, insn_idx, meta->btf, btf_id, nr_slots);
>                 if (err)
>                         return err;
>         } else {
>                 /* iter_next() or iter_destroy() expect initialized iter state*/
> -               if (!is_iter_reg_valid_init(env, reg, meta->btf, btf_id, nr_slots)) {
> -                       verbose(env, "expected an initialized iter_%s as arg #%d\n",
> +               err = is_iter_reg_valid_init(env, reg, meta->btf, btf_id, nr_slots);
> +               switch (err) {
> +               case -EINVAL:

I'd also add default: here, in case we ever emit some other error from
is_iter_reg_valid_init()

> +                       verbose(env, "expected an initialized iter_%s as arg #%d or without bpf_rcu_read_lock()\n",
>                                 iter_type_str(meta->btf, btf_id), regno);
> -                       return -EINVAL;
> +                       return err;
> +               case -EPERM:

I find -EPERM a bit confusing. Let's use something a bit more
specific, e.g., -EPROTO? We are basically not following a protocol if
we don't keep RCU-protected iterators within a single RCU region,
right?

> +                       verbose(env, "expected an RCU CS when using %s\n", meta->func_name);
> +                       return err;
>                 }
>
>                 spi = iter_get_spi(env, reg, nr_slots);
> @@ -10092,6 +10110,11 @@ static bool is_kfunc_rcu(struct bpf_kfunc_call_arg_meta *meta)
>         return meta->kfunc_flags & KF_RCU;
>  }
>
> +static bool is_kfunc_rcu_protected(struct bpf_kfunc_call_arg_meta *meta)
> +{
> +       return meta->kfunc_flags & KF_RCU_PROTECTED;
> +}
> +
>  static bool __kfunc_param_match_suffix(const struct btf *btf,
>                                        const struct btf_param *arg,
>                                        const char *suffix)
> @@ -11428,6 +11451,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>         if (env->cur_state->active_rcu_lock) {
>                 struct bpf_func_state *state;
>                 struct bpf_reg_state *reg;
> +               u32 clear_mask = (1 << STACK_SPILL) | (1 << STACK_ITER);
>
>                 if (in_rbtree_lock_required_cb(env) && (rcu_lock || rcu_unlock)) {
>                         verbose(env, "Calling bpf_rcu_read_{lock,unlock} in unnecessary rbtree callback\n");
> @@ -11438,7 +11462,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>                         verbose(env, "nested rcu read lock (kernel function %s)\n", func_name);
>                         return -EINVAL;
>                 } else if (rcu_unlock) {
> -                       bpf_for_each_reg_in_vstate(env->cur_state, state, reg, ({
> +                       bpf_for_each_reg_in_vstate_mask(env->cur_state, state, reg, clear_mask, ({
>                                 if (reg->type & MEM_RCU) {
>                                         reg->type &= ~(MEM_RCU | PTR_MAYBE_NULL);
>                                         reg->type |= PTR_UNTRUSTED;
> --
> 2.20.1
>
diff mbox series

Patch

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index a3236651ec64..b5cdcc332b0a 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -385,19 +385,18 @@  struct bpf_verifier_state {
 	u32 jmp_history_cnt;
 };
 
-#define bpf_get_spilled_reg(slot, frame)				\
+#define bpf_get_spilled_reg(slot, frame, mask)				\
 	(((slot < frame->allocated_stack / BPF_REG_SIZE) &&		\
-	  (frame->stack[slot].slot_type[0] == STACK_SPILL))		\
+	  ((1 << frame->stack[slot].slot_type[0]) & (mask))) \
 	 ? &frame->stack[slot].spilled_ptr : NULL)
 
 /* Iterate over 'frame', setting 'reg' to either NULL or a spilled register. */
-#define bpf_for_each_spilled_reg(iter, frame, reg)			\
-	for (iter = 0, reg = bpf_get_spilled_reg(iter, frame);		\
+#define bpf_for_each_spilled_reg(iter, frame, reg, mask)			\
+	for (iter = 0, reg = bpf_get_spilled_reg(iter, frame, mask);		\
 	     iter < frame->allocated_stack / BPF_REG_SIZE;		\
-	     iter++, reg = bpf_get_spilled_reg(iter, frame))
+	     iter++, reg = bpf_get_spilled_reg(iter, frame, mask))
 
-/* Invoke __expr over regsiters in __vst, setting __state and __reg */
-#define bpf_for_each_reg_in_vstate(__vst, __state, __reg, __expr)   \
+#define bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, __mask, __expr)   \
 	({                                                               \
 		struct bpf_verifier_state *___vstate = __vst;            \
 		int ___i, ___j;                                          \
@@ -409,7 +408,7 @@  struct bpf_verifier_state {
 				__reg = &___regs[___j];                  \
 				(void)(__expr);                          \
 			}                                                \
-			bpf_for_each_spilled_reg(___j, __state, __reg) { \
+			bpf_for_each_spilled_reg(___j, __state, __reg, __mask) { \
 				if (!__reg)                              \
 					continue;                        \
 				(void)(__expr);                          \
@@ -417,6 +416,10 @@  struct bpf_verifier_state {
 		}                                                        \
 	})
 
+/* Invoke __expr over regsiters in __vst, setting __state and __reg */
+#define bpf_for_each_reg_in_vstate(__vst, __state, __reg, __expr) \
+	bpf_for_each_reg_in_vstate_mask(__vst, __state, __reg, 1 << STACK_SPILL, __expr)
+
 /* linked list of verifier states used to prune search */
 struct bpf_verifier_state_list {
 	struct bpf_verifier_state state;
diff --git a/include/linux/btf.h b/include/linux/btf.h
index 928113a80a95..c2231c64d60b 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -74,6 +74,7 @@ 
 #define KF_ITER_NEW     (1 << 8) /* kfunc implements BPF iter constructor */
 #define KF_ITER_NEXT    (1 << 9) /* kfunc implements BPF iter next method */
 #define KF_ITER_DESTROY (1 << 10) /* kfunc implements BPF iter destructor */
+#define KF_RCU_PROTECTED (1 << 11) /* kfunc should be protected by rcu cs when they are invoked */
 
 /*
  * Tag marking a kernel function as a kfunc. This is meant to minimize the
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 9c3af36249a2..aa9e03fbfe1a 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2507,10 +2507,10 @@  BTF_ID_FLAGS(func, bpf_iter_num_destroy, KF_ITER_DESTROY)
 BTF_ID_FLAGS(func, bpf_iter_css_task_new, KF_ITER_NEW | KF_TRUSTED_ARGS)
 BTF_ID_FLAGS(func, bpf_iter_css_task_next, KF_ITER_NEXT | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_iter_css_task_destroy, KF_ITER_DESTROY)
-BTF_ID_FLAGS(func, bpf_iter_task_new, KF_ITER_NEW | KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_iter_task_new, KF_ITER_NEW | KF_TRUSTED_ARGS | KF_RCU_PROTECTED)
 BTF_ID_FLAGS(func, bpf_iter_task_next, KF_ITER_NEXT | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_iter_task_destroy, KF_ITER_DESTROY)
-BTF_ID_FLAGS(func, bpf_iter_css_new, KF_ITER_NEW | KF_TRUSTED_ARGS)
+BTF_ID_FLAGS(func, bpf_iter_css_new, KF_ITER_NEW | KF_TRUSTED_ARGS | KF_RCU_PROTECTED)
 BTF_ID_FLAGS(func, bpf_iter_css_next, KF_ITER_NEXT | KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_iter_css_destroy, KF_ITER_DESTROY)
 BTF_ID_FLAGS(func, bpf_dynptr_adjust)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 2367483bf4c2..a065e18a0b3a 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1172,7 +1172,12 @@  static bool is_dynptr_type_expected(struct bpf_verifier_env *env, struct bpf_reg
 
 static void __mark_reg_known_zero(struct bpf_reg_state *reg);
 
+static bool in_rcu_cs(struct bpf_verifier_env *env);
+
+static bool is_kfunc_rcu_protected(struct bpf_kfunc_call_arg_meta *meta);
+
 static int mark_stack_slots_iter(struct bpf_verifier_env *env,
+				 struct bpf_kfunc_call_arg_meta *meta,
 				 struct bpf_reg_state *reg, int insn_idx,
 				 struct btf *btf, u32 btf_id, int nr_slots)
 {
@@ -1193,6 +1198,12 @@  static int mark_stack_slots_iter(struct bpf_verifier_env *env,
 
 		__mark_reg_known_zero(st);
 		st->type = PTR_TO_STACK; /* we don't have dedicated reg type */
+		if (is_kfunc_rcu_protected(meta)) {
+			if (in_rcu_cs(env))
+				st->type |= MEM_RCU;
+			else
+				st->type |= PTR_UNTRUSTED;
+		}
 		st->live |= REG_LIVE_WRITTEN;
 		st->ref_obj_id = i == 0 ? id : 0;
 		st->iter.btf = btf;
@@ -1267,7 +1278,7 @@  static bool is_iter_reg_valid_uninit(struct bpf_verifier_env *env,
 	return true;
 }
 
-static bool is_iter_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
+static int is_iter_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
 				   struct btf *btf, u32 btf_id, int nr_slots)
 {
 	struct bpf_func_state *state = func(env, reg);
@@ -1275,26 +1286,28 @@  static bool is_iter_reg_valid_init(struct bpf_verifier_env *env, struct bpf_reg_
 
 	spi = iter_get_spi(env, reg, nr_slots);
 	if (spi < 0)
-		return false;
+		return -EINVAL;
 
 	for (i = 0; i < nr_slots; i++) {
 		struct bpf_stack_state *slot = &state->stack[spi - i];
 		struct bpf_reg_state *st = &slot->spilled_ptr;
 
+		if (st->type & PTR_UNTRUSTED)
+			return -EPERM;
 		/* only main (first) slot has ref_obj_id set */
 		if (i == 0 && !st->ref_obj_id)
-			return false;
+			return -EINVAL;
 		if (i != 0 && st->ref_obj_id)
-			return false;
+			return -EINVAL;
 		if (st->iter.btf != btf || st->iter.btf_id != btf_id)
-			return false;
+			return -EINVAL;
 
 		for (j = 0; j < BPF_REG_SIZE; j++)
 			if (slot->slot_type[j] != STACK_ITER)
-				return false;
+				return -EINVAL;
 	}
 
-	return true;
+	return 0;
 }
 
 /* Check if given stack slot is "special":
@@ -7503,15 +7516,20 @@  static int process_iter_arg(struct bpf_verifier_env *env, int regno, int insn_id
 				return err;
 		}
 
-		err = mark_stack_slots_iter(env, reg, insn_idx, meta->btf, btf_id, nr_slots);
+		err = mark_stack_slots_iter(env, meta, reg, insn_idx, meta->btf, btf_id, nr_slots);
 		if (err)
 			return err;
 	} else {
 		/* iter_next() or iter_destroy() expect initialized iter state*/
-		if (!is_iter_reg_valid_init(env, reg, meta->btf, btf_id, nr_slots)) {
-			verbose(env, "expected an initialized iter_%s as arg #%d\n",
+		err = is_iter_reg_valid_init(env, reg, meta->btf, btf_id, nr_slots);
+		switch (err) {
+		case -EINVAL:
+			verbose(env, "expected an initialized iter_%s as arg #%d or without bpf_rcu_read_lock()\n",
 				iter_type_str(meta->btf, btf_id), regno);
-			return -EINVAL;
+			return err;
+		case -EPERM:
+			verbose(env, "expected an RCU CS when using %s\n", meta->func_name);
+			return err;
 		}
 
 		spi = iter_get_spi(env, reg, nr_slots);
@@ -10092,6 +10110,11 @@  static bool is_kfunc_rcu(struct bpf_kfunc_call_arg_meta *meta)
 	return meta->kfunc_flags & KF_RCU;
 }
 
+static bool is_kfunc_rcu_protected(struct bpf_kfunc_call_arg_meta *meta)
+{
+	return meta->kfunc_flags & KF_RCU_PROTECTED;
+}
+
 static bool __kfunc_param_match_suffix(const struct btf *btf,
 				       const struct btf_param *arg,
 				       const char *suffix)
@@ -11428,6 +11451,7 @@  static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 	if (env->cur_state->active_rcu_lock) {
 		struct bpf_func_state *state;
 		struct bpf_reg_state *reg;
+		u32 clear_mask = (1 << STACK_SPILL) | (1 << STACK_ITER);
 
 		if (in_rbtree_lock_required_cb(env) && (rcu_lock || rcu_unlock)) {
 			verbose(env, "Calling bpf_rcu_read_{lock,unlock} in unnecessary rbtree callback\n");
@@ -11438,7 +11462,7 @@  static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 			verbose(env, "nested rcu read lock (kernel function %s)\n", func_name);
 			return -EINVAL;
 		} else if (rcu_unlock) {
-			bpf_for_each_reg_in_vstate(env->cur_state, state, reg, ({
+			bpf_for_each_reg_in_vstate_mask(env->cur_state, state, reg, clear_mask, ({
 				if (reg->type & MEM_RCU) {
 					reg->type &= ~(MEM_RCU | PTR_MAYBE_NULL);
 					reg->type |= PTR_UNTRUSTED;