On Wed, 2023-11-08 at 15:11 -0800, Andrii Nakryiko wrote:
Acked-by: Eduard Zingerman <eddyz87@xxxxxxxxx>
(given that I understood check in push_insn correctly).
[...]
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index edca7f1ad335..35065cae98b7 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -15433,8 +15433,9 @@ static int check_return_code(struct bpf_verifier_env *env, int regno)
Nitpick: there is a comment right above this enum which has to be
updated after changes to the enum.
> enum {
> DISCOVERED = 0x10,
> EXPLORED = 0x20,
> - FALLTHROUGH = 1,
> - BRANCH = 2,
> + CONDITIONAL = 0x01,
> + FALLTHROUGH = 0x02,
> + BRANCH = 0x04,
> };
>
> static void mark_prune_point(struct bpf_verifier_env *env, int idx)
> @@ -15468,16 +15469,15 @@ enum {
> * w - next instruction
> * e - edge
> */
> -static int push_insn(int t, int w, int e, struct bpf_verifier_env *env,
> - bool loop_ok)
> +static int push_insn(int t, int w, int e, struct bpf_verifier_env *env)
> {
> int *insn_stack = env->cfg.insn_stack;
> int *insn_state = env->cfg.insn_state;
>
> - if (e == FALLTHROUGH && insn_state[t] >= (DISCOVERED | FALLTHROUGH))
> + if ((e & FALLTHROUGH) && insn_state[t] >= (DISCOVERED | FALLTHROUGH))
> return DONE_EXPLORING;
This not related to your changes, but '>=' here is so confusing.
The intent is to check:
((insn_state[t] & (DISCOVERED | FALLTHROUGH)) == (DISCOVERED | FALLTHROUGH))
i.e. DONE_EXPLORING if fall-through branch of 't' had been explored already,
right?
[...]
