[PATCH] bpf: Use common error handling code in bpf_is_state_visited()

[PATCH] bpf: Use common error handling code in bpf_is_state_visited()

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
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.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 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;
 }
-- 
2.54.0

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

[Emil Tsalapatis]

On Wed Jun 17, 2026 at 3:30 PM EDT, Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> 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.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>

Can you resend with [PATCH bpf-next] in the title? Right now it's not
celar which tree this is targeting so Patchwork is treating this as
a netdev patch.

> ---
>  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;
>  }

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

[Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
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 <emil@etsalapatis.com>
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 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;
 }
-- 
2.54.0

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

[Alexei Starovoitov]

On Wed, Jun 17, 2026 at 12:55 PM Markus Elfring <Markus.Elfring@web.de> wrote:
>
> From: Markus Elfring <elfring@users.sourceforge.net>
> 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 <emil@etsalapatis.com>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  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.

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

[Markus Elfring]

>> 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.
…
>> +++ b/kernel/bpf/states.c
…
>> @@ -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.

How does such a view fit to a known coding style recommendation?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?h=v7.1#n526


How do you think about to increase the application of scope-based resource management?

Regards,
Markus