| Message ID | 20220830172759.4069786-2-davemarchevsky@fb.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | BPF |
| Headers | show |
| Series | bpf: Introduce rbtree map | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-next-PR | fail | merge-conflict |
| netdev/tree_selection | success | Clearly marked for bpf-next, async |
| netdev/apply | fail | Patch does not apply to bpf-next |
On Tue, Aug 30, 2022 at 11:03 AM Dave Marchevsky <davemarchevsky@fb.com> wrote: > > Verifier logic to confirm that a callback function returns 0 or 1 was > added in commit 69c087ba6225b ("bpf: Add bpf_for_each_map_elem() helper"). > At the time, callback return value was only used to continue or stop > iteration. > > In order to support callbacks with a broader return value range, such as > those added further in this series, add a callback_ret_range to > bpf_func_state. Verifier's helpers which set in_callback_fn will also > set the new field, which the verifier will later use to check return > value bounds. > > Default to tnum_range(0, 1) instead of using tnum_unknown as a sentinel > value as the latter would prevent the valid range (0, U64_MAX) being > used. > > Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> > --- > include/linux/bpf_verifier.h | 1 + > kernel/bpf/verifier.c | 4 +++- > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h > index 2e3bad8640dc..9c017575c034 100644 > --- a/include/linux/bpf_verifier.h > +++ b/include/linux/bpf_verifier.h > @@ -237,6 +237,7 @@ struct bpf_func_state { > */ > u32 async_entry_cnt; > bool in_callback_fn; > + struct tnum callback_ret_range; > bool in_async_callback_fn; > > /* The following fields should be last. See copy_func_state() */ > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 9bef8b41e737..68bfa7c28048 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -1745,6 +1745,7 @@ static void init_func_state(struct bpf_verifier_env *env, > state->callsite = callsite; > state->frameno = frameno; > state->subprogno = subprogno; > + state->callback_ret_range = tnum_range(0, 1); > init_reg_state(env, state); > mark_verifier_state_scratched(env); > } > @@ -6879,6 +6880,7 @@ static int set_find_vma_callback_state(struct bpf_verifier_env *env, > __mark_reg_not_init(env, &callee->regs[BPF_REG_4]); > __mark_reg_not_init(env, &callee->regs[BPF_REG_5]); > callee->in_callback_fn = true; > + callee->callback_ret_range = tnum_range(0, 1); Thanks for removing this restriction for callback functions! One quick question: is this line above needed? I think in __check_func_call, we always call init_func_state() first before calling set_find_vma_callback_state(), so after the init_func_state() call, the callee->callback_ret_range will already be set to tnum_range(0,1). > return 0; > } > > @@ -6906,7 +6908,7 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx) > caller = state->frame[state->curframe]; > if (callee->in_callback_fn) { > /* enforce R0 return value range [0, 1]. */ > - struct tnum range = tnum_range(0, 1); > + struct tnum range = callee->callback_ret_range; > > if (r0->type != SCALAR_VALUE) { > verbose(env, "R0 not a scalar value\n"); > -- > 2.30.2 >
On 9/1/22 5:01 PM, Joanne Koong wrote: > On Tue, Aug 30, 2022 at 11:03 AM Dave Marchevsky <davemarchevsky@fb.com> wrote: >> >> Verifier logic to confirm that a callback function returns 0 or 1 was >> added in commit 69c087ba6225b ("bpf: Add bpf_for_each_map_elem() helper"). >> At the time, callback return value was only used to continue or stop >> iteration. >> >> In order to support callbacks with a broader return value range, such as >> those added further in this series, add a callback_ret_range to >> bpf_func_state. Verifier's helpers which set in_callback_fn will also >> set the new field, which the verifier will later use to check return >> value bounds. >> >> Default to tnum_range(0, 1) instead of using tnum_unknown as a sentinel >> value as the latter would prevent the valid range (0, U64_MAX) being >> used. >> >> Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> >> --- >> include/linux/bpf_verifier.h | 1 + >> kernel/bpf/verifier.c | 4 +++- >> 2 files changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h >> index 2e3bad8640dc..9c017575c034 100644 >> --- a/include/linux/bpf_verifier.h >> +++ b/include/linux/bpf_verifier.h >> @@ -237,6 +237,7 @@ struct bpf_func_state { >> */ >> u32 async_entry_cnt; >> bool in_callback_fn; >> + struct tnum callback_ret_range; >> bool in_async_callback_fn; >> >> /* The following fields should be last. See copy_func_state() */ >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c >> index 9bef8b41e737..68bfa7c28048 100644 >> --- a/kernel/bpf/verifier.c >> +++ b/kernel/bpf/verifier.c >> @@ -1745,6 +1745,7 @@ static void init_func_state(struct bpf_verifier_env *env, >> state->callsite = callsite; >> state->frameno = frameno; >> state->subprogno = subprogno; >> + state->callback_ret_range = tnum_range(0, 1); >> init_reg_state(env, state); >> mark_verifier_state_scratched(env); >> } >> @@ -6879,6 +6880,7 @@ static int set_find_vma_callback_state(struct bpf_verifier_env *env, >> __mark_reg_not_init(env, &callee->regs[BPF_REG_4]); >> __mark_reg_not_init(env, &callee->regs[BPF_REG_5]); >> callee->in_callback_fn = true; >> + callee->callback_ret_range = tnum_range(0, 1); > > Thanks for removing this restriction for callback functions! > > One quick question: is this line above needed? I think in > __check_func_call, we always call init_func_state() first before > calling set_find_vma_callback_state(), so after the init_func_state() > call, the callee->callback_ret_range will already be set to > tnum_range(0,1). > You're right, it's not strictly necessary. I think that the default range being tnum_range(0, 1) - although necessary for backwards compat - is unintuitive. So decided to be explicit with existing callbacks so folks didn't have to go searching for the default to understand what the ret_range is, and it's more obvious that callback_ret_range should be changed if existing helper code is reused. >> return 0; >> } >> >> @@ -6906,7 +6908,7 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx) >> caller = state->frame[state->curframe]; >> if (callee->in_callback_fn) { >> /* enforce R0 return value range [0, 1]. */ >> - struct tnum range = tnum_range(0, 1); >> + struct tnum range = callee->callback_ret_range; >> >> if (r0->type != SCALAR_VALUE) { >> verbose(env, "R0 not a scalar value\n"); >> -- >> 2.30.2 >>
On 9/6/22 4:42 PM, Dave Marchevsky wrote: > On 9/1/22 5:01 PM, Joanne Koong wrote: >> On Tue, Aug 30, 2022 at 11:03 AM Dave Marchevsky <davemarchevsky@fb.com> wrote: >>> >>> Verifier logic to confirm that a callback function returns 0 or 1 was >>> added in commit 69c087ba6225b ("bpf: Add bpf_for_each_map_elem() helper"). >>> At the time, callback return value was only used to continue or stop >>> iteration. >>> >>> In order to support callbacks with a broader return value range, such as >>> those added further in this series, add a callback_ret_range to >>> bpf_func_state. Verifier's helpers which set in_callback_fn will also >>> set the new field, which the verifier will later use to check return >>> value bounds. >>> >>> Default to tnum_range(0, 1) instead of using tnum_unknown as a sentinel >>> value as the latter would prevent the valid range (0, U64_MAX) being >>> used. >>> >>> Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> >>> --- >>> include/linux/bpf_verifier.h | 1 + >>> kernel/bpf/verifier.c | 4 +++- >>> 2 files changed, 4 insertions(+), 1 deletion(-) >>> >>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h >>> index 2e3bad8640dc..9c017575c034 100644 >>> --- a/include/linux/bpf_verifier.h >>> +++ b/include/linux/bpf_verifier.h >>> @@ -237,6 +237,7 @@ struct bpf_func_state { >>> */ >>> u32 async_entry_cnt; >>> bool in_callback_fn; >>> + struct tnum callback_ret_range; >>> bool in_async_callback_fn; >>> >>> /* The following fields should be last. See copy_func_state() */ >>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c >>> index 9bef8b41e737..68bfa7c28048 100644 >>> --- a/kernel/bpf/verifier.c >>> +++ b/kernel/bpf/verifier.c >>> @@ -1745,6 +1745,7 @@ static void init_func_state(struct bpf_verifier_env *env, >>> state->callsite = callsite; >>> state->frameno = frameno; >>> state->subprogno = subprogno; >>> + state->callback_ret_range = tnum_range(0, 1); >>> init_reg_state(env, state); >>> mark_verifier_state_scratched(env); >>> } >>> @@ -6879,6 +6880,7 @@ static int set_find_vma_callback_state(struct bpf_verifier_env *env, >>> __mark_reg_not_init(env, &callee->regs[BPF_REG_4]); >>> __mark_reg_not_init(env, &callee->regs[BPF_REG_5]); >>> callee->in_callback_fn = true; >>> + callee->callback_ret_range = tnum_range(0, 1); >> >> Thanks for removing this restriction for callback functions! >> >> One quick question: is this line above needed? I think in >> __check_func_call, we always call init_func_state() first before >> calling set_find_vma_callback_state(), so after the init_func_state() >> call, the callee->callback_ret_range will already be set to >> tnum_range(0,1). >> > > You're right, it's not strictly necessary. I think that the default range being > tnum_range(0, 1) - although necessary for backwards compat - is unintuitive. So > decided to be explicit with existing callbacks so folks didn't have to go > searching for the default to understand what the ret_range is, and it's more > obvious that callback_ret_range should be changed if existing helper code is > reused. Maybe then it's better to keep callback_ret_range as range(0,0) in init_func_state() to nudge/force other places to set it explicitly ?
On 9/6/22 9:53 PM, Alexei Starovoitov wrote: > On 9/6/22 4:42 PM, Dave Marchevsky wrote: >> On 9/1/22 5:01 PM, Joanne Koong wrote: >>> On Tue, Aug 30, 2022 at 11:03 AM Dave Marchevsky <davemarchevsky@fb.com> wrote: >>>> >>>> Verifier logic to confirm that a callback function returns 0 or 1 was >>>> added in commit 69c087ba6225b ("bpf: Add bpf_for_each_map_elem() helper"). >>>> At the time, callback return value was only used to continue or stop >>>> iteration. >>>> >>>> In order to support callbacks with a broader return value range, such as >>>> those added further in this series, add a callback_ret_range to >>>> bpf_func_state. Verifier's helpers which set in_callback_fn will also >>>> set the new field, which the verifier will later use to check return >>>> value bounds. >>>> >>>> Default to tnum_range(0, 1) instead of using tnum_unknown as a sentinel >>>> value as the latter would prevent the valid range (0, U64_MAX) being >>>> used. >>>> >>>> Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> >>>> --- >>>> include/linux/bpf_verifier.h | 1 + >>>> kernel/bpf/verifier.c | 4 +++- >>>> 2 files changed, 4 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h >>>> index 2e3bad8640dc..9c017575c034 100644 >>>> --- a/include/linux/bpf_verifier.h >>>> +++ b/include/linux/bpf_verifier.h >>>> @@ -237,6 +237,7 @@ struct bpf_func_state { >>>> */ >>>> u32 async_entry_cnt; >>>> bool in_callback_fn; >>>> + struct tnum callback_ret_range; >>>> bool in_async_callback_fn; >>>> >>>> /* The following fields should be last. See copy_func_state() */ >>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c >>>> index 9bef8b41e737..68bfa7c28048 100644 >>>> --- a/kernel/bpf/verifier.c >>>> +++ b/kernel/bpf/verifier.c >>>> @@ -1745,6 +1745,7 @@ static void init_func_state(struct bpf_verifier_env *env, >>>> state->callsite = callsite; >>>> state->frameno = frameno; >>>> state->subprogno = subprogno; >>>> + state->callback_ret_range = tnum_range(0, 1); >>>> init_reg_state(env, state); >>>> mark_verifier_state_scratched(env); >>>> } >>>> @@ -6879,6 +6880,7 @@ static int set_find_vma_callback_state(struct bpf_verifier_env *env, >>>> __mark_reg_not_init(env, &callee->regs[BPF_REG_4]); >>>> __mark_reg_not_init(env, &callee->regs[BPF_REG_5]); >>>> callee->in_callback_fn = true; >>>> + callee->callback_ret_range = tnum_range(0, 1); >>> >>> Thanks for removing this restriction for callback functions! >>> >>> One quick question: is this line above needed? I think in >>> __check_func_call, we always call init_func_state() first before >>> calling set_find_vma_callback_state(), so after the init_func_state() >>> call, the callee->callback_ret_range will already be set to >>> tnum_range(0,1). >>> >> >> You're right, it's not strictly necessary. I think that the default range being >> tnum_range(0, 1) - although necessary for backwards compat - is unintuitive. So >> decided to be explicit with existing callbacks so folks didn't have to go >> searching for the default to understand what the ret_range is, and it's more >> obvious that callback_ret_range should be changed if existing helper code is >> reused. > > Maybe then it's better to keep callback_ret_range as range(0,0) > in init_func_state() to nudge/force other places to set it explicitly ? tnum_range(0, 0) sounds good to me. Would you like me to send this separately with that change, so it can be applied independently of rest of these changes?
On Thu, Sep 8, 2022 at 2:37 PM Dave Marchevsky <davemarchevsky@fb.com> wrote: > > On 9/6/22 9:53 PM, Alexei Starovoitov wrote: > > On 9/6/22 4:42 PM, Dave Marchevsky wrote: > >> On 9/1/22 5:01 PM, Joanne Koong wrote: > >>> On Tue, Aug 30, 2022 at 11:03 AM Dave Marchevsky <davemarchevsky@fb.com> wrote: > >>>> > >>>> Verifier logic to confirm that a callback function returns 0 or 1 was > >>>> added in commit 69c087ba6225b ("bpf: Add bpf_for_each_map_elem() helper"). > >>>> At the time, callback return value was only used to continue or stop > >>>> iteration. > >>>> > >>>> In order to support callbacks with a broader return value range, such as > >>>> those added further in this series, add a callback_ret_range to > >>>> bpf_func_state. Verifier's helpers which set in_callback_fn will also > >>>> set the new field, which the verifier will later use to check return > >>>> value bounds. > >>>> > >>>> Default to tnum_range(0, 1) instead of using tnum_unknown as a sentinel > >>>> value as the latter would prevent the valid range (0, U64_MAX) being > >>>> used. > >>>> > >>>> Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> > >>>> --- > >>>> include/linux/bpf_verifier.h | 1 + > >>>> kernel/bpf/verifier.c | 4 +++- > >>>> 2 files changed, 4 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h > >>>> index 2e3bad8640dc..9c017575c034 100644 > >>>> --- a/include/linux/bpf_verifier.h > >>>> +++ b/include/linux/bpf_verifier.h > >>>> @@ -237,6 +237,7 @@ struct bpf_func_state { > >>>> */ > >>>> u32 async_entry_cnt; > >>>> bool in_callback_fn; > >>>> + struct tnum callback_ret_range; > >>>> bool in_async_callback_fn; > >>>> > >>>> /* The following fields should be last. See copy_func_state() */ > >>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > >>>> index 9bef8b41e737..68bfa7c28048 100644 > >>>> --- a/kernel/bpf/verifier.c > >>>> +++ b/kernel/bpf/verifier.c > >>>> @@ -1745,6 +1745,7 @@ static void init_func_state(struct bpf_verifier_env *env, > >>>> state->callsite = callsite; > >>>> state->frameno = frameno; > >>>> state->subprogno = subprogno; > >>>> + state->callback_ret_range = tnum_range(0, 1); > >>>> init_reg_state(env, state); > >>>> mark_verifier_state_scratched(env); > >>>> } > >>>> @@ -6879,6 +6880,7 @@ static int set_find_vma_callback_state(struct bpf_verifier_env *env, > >>>> __mark_reg_not_init(env, &callee->regs[BPF_REG_4]); > >>>> __mark_reg_not_init(env, &callee->regs[BPF_REG_5]); > >>>> callee->in_callback_fn = true; > >>>> + callee->callback_ret_range = tnum_range(0, 1); > >>> > >>> Thanks for removing this restriction for callback functions! > >>> > >>> One quick question: is this line above needed? I think in > >>> __check_func_call, we always call init_func_state() first before > >>> calling set_find_vma_callback_state(), so after the init_func_state() > >>> call, the callee->callback_ret_range will already be set to > >>> tnum_range(0,1). > >>> > >> > >> You're right, it's not strictly necessary. I think that the default range being > >> tnum_range(0, 1) - although necessary for backwards compat - is unintuitive. So > >> decided to be explicit with existing callbacks so folks didn't have to go > >> searching for the default to understand what the ret_range is, and it's more > >> obvious that callback_ret_range should be changed if existing helper code is > >> reused. > > > > Maybe then it's better to keep callback_ret_range as range(0,0) > > in init_func_state() to nudge/force other places to set it explicitly ? > > tnum_range(0, 0) sounds good to me. > > Would you like me to send this separately with that change, so it can be applied > independently of rest of these changes? Whichever way is faster. We can always apply a patch or a few patches out of a bigger set.
On 9/8/22 5:40 PM, Alexei Starovoitov wrote: > On Thu, Sep 8, 2022 at 2:37 PM Dave Marchevsky <davemarchevsky@fb.com> wrote: >> >> On 9/6/22 9:53 PM, Alexei Starovoitov wrote: >>> On 9/6/22 4:42 PM, Dave Marchevsky wrote: >>>> On 9/1/22 5:01 PM, Joanne Koong wrote: >>>>> On Tue, Aug 30, 2022 at 11:03 AM Dave Marchevsky <davemarchevsky@fb.com> wrote: >>>>>> >>>>>> Verifier logic to confirm that a callback function returns 0 or 1 was >>>>>> added in commit 69c087ba6225b ("bpf: Add bpf_for_each_map_elem() helper"). >>>>>> At the time, callback return value was only used to continue or stop >>>>>> iteration. >>>>>> >>>>>> In order to support callbacks with a broader return value range, such as >>>>>> those added further in this series, add a callback_ret_range to >>>>>> bpf_func_state. Verifier's helpers which set in_callback_fn will also >>>>>> set the new field, which the verifier will later use to check return >>>>>> value bounds. >>>>>> >>>>>> Default to tnum_range(0, 1) instead of using tnum_unknown as a sentinel >>>>>> value as the latter would prevent the valid range (0, U64_MAX) being >>>>>> used. >>>>>> >>>>>> Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> >>>>>> --- >>>>>> include/linux/bpf_verifier.h | 1 + >>>>>> kernel/bpf/verifier.c | 4 +++- >>>>>> 2 files changed, 4 insertions(+), 1 deletion(-) >>>>>> >>>>>> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h >>>>>> index 2e3bad8640dc..9c017575c034 100644 >>>>>> --- a/include/linux/bpf_verifier.h >>>>>> +++ b/include/linux/bpf_verifier.h >>>>>> @@ -237,6 +237,7 @@ struct bpf_func_state { >>>>>> */ >>>>>> u32 async_entry_cnt; >>>>>> bool in_callback_fn; >>>>>> + struct tnum callback_ret_range; >>>>>> bool in_async_callback_fn; >>>>>> >>>>>> /* The following fields should be last. See copy_func_state() */ >>>>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c >>>>>> index 9bef8b41e737..68bfa7c28048 100644 >>>>>> --- a/kernel/bpf/verifier.c >>>>>> +++ b/kernel/bpf/verifier.c >>>>>> @@ -1745,6 +1745,7 @@ static void init_func_state(struct bpf_verifier_env *env, >>>>>> state->callsite = callsite; >>>>>> state->frameno = frameno; >>>>>> state->subprogno = subprogno; >>>>>> + state->callback_ret_range = tnum_range(0, 1); >>>>>> init_reg_state(env, state); >>>>>> mark_verifier_state_scratched(env); >>>>>> } >>>>>> @@ -6879,6 +6880,7 @@ static int set_find_vma_callback_state(struct bpf_verifier_env *env, >>>>>> __mark_reg_not_init(env, &callee->regs[BPF_REG_4]); >>>>>> __mark_reg_not_init(env, &callee->regs[BPF_REG_5]); >>>>>> callee->in_callback_fn = true; >>>>>> + callee->callback_ret_range = tnum_range(0, 1); >>>>> >>>>> Thanks for removing this restriction for callback functions! >>>>> >>>>> One quick question: is this line above needed? I think in >>>>> __check_func_call, we always call init_func_state() first before >>>>> calling set_find_vma_callback_state(), so after the init_func_state() >>>>> call, the callee->callback_ret_range will already be set to >>>>> tnum_range(0,1). >>>>> >>>> >>>> You're right, it's not strictly necessary. I think that the default range being >>>> tnum_range(0, 1) - although necessary for backwards compat - is unintuitive. So >>>> decided to be explicit with existing callbacks so folks didn't have to go >>>> searching for the default to understand what the ret_range is, and it's more >>>> obvious that callback_ret_range should be changed if existing helper code is >>>> reused. >>> >>> Maybe then it's better to keep callback_ret_range as range(0,0) >>> in init_func_state() to nudge/force other places to set it explicitly ? >> >> tnum_range(0, 0) sounds good to me. >> >> Would you like me to send this separately with that change, so it can be applied >> independently of rest of these changes? > > Whichever way is faster. > We can always apply a patch or a few patches out of a bigger set. Updated + rebased and sent as https://lore.kernel.org/bpf/20220908230716.2751723-1-davemarchevsky@fb.com/
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h index 2e3bad8640dc..9c017575c034 100644 --- a/include/linux/bpf_verifier.h +++ b/include/linux/bpf_verifier.h @@ -237,6 +237,7 @@ struct bpf_func_state { */ u32 async_entry_cnt; bool in_callback_fn; + struct tnum callback_ret_range; bool in_async_callback_fn; /* The following fields should be last. See copy_func_state() */ diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 9bef8b41e737..68bfa7c28048 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -1745,6 +1745,7 @@ static void init_func_state(struct bpf_verifier_env *env, state->callsite = callsite; state->frameno = frameno; state->subprogno = subprogno; + state->callback_ret_range = tnum_range(0, 1); init_reg_state(env, state); mark_verifier_state_scratched(env); } @@ -6879,6 +6880,7 @@ static int set_find_vma_callback_state(struct bpf_verifier_env *env, __mark_reg_not_init(env, &callee->regs[BPF_REG_4]); __mark_reg_not_init(env, &callee->regs[BPF_REG_5]); callee->in_callback_fn = true; + callee->callback_ret_range = tnum_range(0, 1); return 0; } @@ -6906,7 +6908,7 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx) caller = state->frame[state->curframe]; if (callee->in_callback_fn) { /* enforce R0 return value range [0, 1]. */ - struct tnum range = tnum_range(0, 1); + struct tnum range = callee->callback_ret_range; if (r0->type != SCALAR_VALUE) { verbose(env, "R0 not a scalar value\n");
Verifier logic to confirm that a callback function returns 0 or 1 was added in commit 69c087ba6225b ("bpf: Add bpf_for_each_map_elem() helper"). At the time, callback return value was only used to continue or stop iteration. In order to support callbacks with a broader return value range, such as those added further in this series, add a callback_ret_range to bpf_func_state. Verifier's helpers which set in_callback_fn will also set the new field, which the verifier will later use to check return value bounds. Default to tnum_range(0, 1) instead of using tnum_unknown as a sentinel value as the latter would prevent the valid range (0, U64_MAX) being used. Signed-off-by: Dave Marchevsky <davemarchevsky@fb.com> --- include/linux/bpf_verifier.h | 1 + kernel/bpf/verifier.c | 4 +++- 2 files changed, 4 insertions(+), 1 deletion(-)