Re: [PATCH RESEND bpf-next] bpf: Use common error handling code in bpf_is_state_visited()

Alexei Starovoitov <[email protected]> Sun, 21 Jun 2026 18:06:02 -0700
Newsgroups org.kernel.vger.kernel-janitors,org.kernel.vger.bpf,org.kernel.vger.linux-kernel
Message-ID <CAADnVQL060PFApJ++juj5S6H8OQGFUHCepavrRrizh0ciBtiqw@mail.gmail.com>
On Wed, Jun 17, 2026 at 12:55 PM Markus Elfring <[email protected]> wrote:
>
> From: Markus Elfring <[email protected]>
> Date: Wed, 17 Jun 2026 21:15:08 +0200
>
> Use an additional label so that a bit of exception handling can be better
> reused at the end of this function implementation.
>
> This issue was detected by using the Coccinelle software.
>
> Reviewed-by: Emil Tsalapatis <[email protected]>
> Signed-off-by: Markus Elfring <[email protected]>
> ---
>  kernel/bpf/states.c | 20 ++++++++++----------
>  1 file changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
> index 5945956a7573..09f87e2bb1e6 100644
> --- a/kernel/bpf/states.c
> +++ b/kernel/bpf/states.c
> @@ -1566,21 +1566,16 @@ int bpf_is_state_visited(struct bpf_verifier_env *env, int insn_idx)
>         /* add new state to the head of linked list */
>         new = &new_sl->state;
>         err = bpf_copy_verifier_state(new, cur);
> -       if (err) {
> -               bpf_free_verifier_state(new, false);
> -               kfree(new_sl);
> -               return err;
> -       }
> +       if (err)
> +               goto free_verifier_state;
> +
>         new->insn_idx = insn_idx;
>         verifier_bug_if(new->branches != 1, env,
>                         "%s:branches_to_explore=%d insn %d",
>                         __func__, new->branches, insn_idx);
>         err = maybe_enter_scc(env, new);
> -       if (err) {
> -               bpf_free_verifier_state(new, false);
> -               kfree(new_sl);
> -               return err;
> -       }
> +       if (err)
> +               goto free_verifier_state;
>
>         cur->parent = new;
>         cur->first_insn_idx = insn_idx;
> @@ -1588,4 +1583,9 @@ int bpf_is_state_visited(struct bpf_verifier_env *env, int insn_idx)
>         bpf_clear_jmp_history(cur);
>         list_add(&new_sl->node, head);
>         return 0;
> +
> +free_verifier_state:
> +       bpf_free_verifier_state(new, false);
> +       kfree(new_sl);
> +       return err;

This function is full of 'goto's already.
I prefer to keep the code as-is.