diff mbox series

[bpf-next,v3,2/7] bpf: Refactor {acquire,release}_reference_state

Message ID 20241127165846.2001009-3-memxor@gmail.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series IRQ save/restore | expand

Checks

Context Check Description
bpf/vmtest-bpf-next-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-next-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-next-VM_Test-5 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-next-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-next-VM_Test-14 success Logs for s390x-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-15 success Logs for set-matrix
bpf/vmtest-bpf-next-VM_Test-4 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-16 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-9 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-13 success Logs for s390x-gcc / test
bpf/vmtest-bpf-next-VM_Test-11 fail Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-next-VM_Test-12 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-10 success Logs for aarch64-gcc / veristat
bpf/vmtest-bpf-next-VM_Test-17 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-next-VM_Test-18 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-22 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-23 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-25 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-26 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17-O2
bpf/vmtest-bpf-next-VM_Test-27 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-30 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-31 success Logs for x86_64-llvm-17 / veristat
bpf/vmtest-bpf-next-VM_Test-32 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-33 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18-O2
bpf/vmtest-bpf-next-VM_Test-34 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-38 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-39 success Logs for x86_64-llvm-18 / veristat
bpf/vmtest-bpf-next-VM_Test-6 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-19 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-20 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-21 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-24 success Logs for x86_64-gcc / veristat / veristat on x86_64 with gcc
bpf/vmtest-bpf-next-VM_Test-28 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-29 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-next-VM_Test-35 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-36 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-next-VM_Test-37 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-next-PR fail PR summary
bpf/vmtest-bpf-next-VM_Test-8 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-next-VM_Test-7 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for bpf-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 3 this patch: 3
netdev/build_tools success No tools touched, skip
netdev/cc_maintainers warning 8 maintainers not CCed: kpsingh@kernel.org jolsa@kernel.org song@kernel.org haoluo@google.com john.fastabend@gmail.com yonghong.song@linux.dev martin.lau@linux.dev sdf@fomichev.me
netdev/build_clang success Errors and warnings before: 3 this patch: 3
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 14 this patch: 14
netdev/checkpatch warning WARNING: line length of 115 exceeds 80 columns WARNING: line length of 81 exceeds 80 columns WARNING: line length of 83 exceeds 80 columns WARNING: line length of 85 exceeds 80 columns WARNING: line length of 86 exceeds 80 columns WARNING: line length of 88 exceeds 80 columns
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Kumar Kartikeya Dwivedi Nov. 27, 2024, 4:58 p.m. UTC 
In preparation for introducing support for more reference types which
have to add and remove reference state, refactor the
acquire_reference_state and release_reference_state functions to share
common logic.

The acquire_reference_state function simply handles growing the acquired
refs and returning the pointer to the new uninitialized element, which
can be filled in by the caller.

The release_reference_state function simply erases a reference state
entry in the acquired_refs array and shrinks it. The callers are
responsible for finding the suitable element by matching on various
fields of the reference state and requesting deletion through this
function. It is not supposed to be called directly.

Existing callers of release_reference_state were using it to find and
remove state for a given ref_obj_id without scrubbing the associated
registers in the verifier state. Introduce release_reference_nomark to
provide this functionality and convert callers. We now use this new
release_reference_nomark function within release_reference as well.
It needs to operate on a verifier state instead of taking verifier env
as mark_ptr_or_null_regs requires operating on verifier state of the
two branches of a NULL condition check, therefore env->cur_state cannot
be used directly.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/verifier.c | 113 +++++++++++++++++++++++-------------------
 1 file changed, 63 insertions(+), 50 deletions(-)

Comments

Eduard Zingerman Nov. 28, 2024, 4:13 a.m. UTC | #1 
On Wed, 2024-11-27 at 08:58 -0800, Kumar Kartikeya Dwivedi wrote:

Overall looks good, but please take a look at a few notes below.

[...]

> @@ -1349,77 +1350,69 @@ static int grow_stack_state(struct bpf_verifier_env *env, struct bpf_func_state
>   * On success, returns a valid pointer id to associate with the register
>   * On failure, returns a negative errno.
>   */
> -static int acquire_reference_state(struct bpf_verifier_env *env, int insn_idx)
> +static struct bpf_reference_state *acquire_reference_state(struct bpf_verifier_env *env, int insn_idx, bool gen_id)
>  {
>  	struct bpf_verifier_state *state = env->cur_state;
>  	int new_ofs = state->acquired_refs;
> -	int id, err;
> +	int err;
>  
>  	err = resize_reference_state(state, state->acquired_refs + 1);
>  	if (err)
> -		return err;
> -	id = ++env->id_gen;
> -	state->refs[new_ofs].type = REF_TYPE_PTR;
> -	state->refs[new_ofs].id = id;
> +		return NULL;
> +	if (gen_id)
> +		state->refs[new_ofs].id = ++env->id_gen;

Nit: state->refs[new_ods].id might end up with garbage value if 'gen_id' is false.
     The resize_reference_state() uses realloc_array(),
     which allocates memory with GFP_KERNEL, but without __GFP_ZERO flag.
     This is not a problem with current patch, as you always check
     reference type before checking id, but most of the data strucures
     in verifier are zero initialized just in case.

>  	state->refs[new_ofs].insn_idx = insn_idx;
>  
> -	return id;
> +	return &state->refs[new_ofs];
> +}

[...]

> -/* release function corresponding to acquire_reference_state(). Idempotent. */
> -static int release_reference_state(struct bpf_verifier_state *state, int ptr_id)
> +static void release_reference_state(struct bpf_verifier_state *state, int idx)
>  {
> -	int i, last_idx;
> +	int last_idx;
>  
>  	last_idx = state->acquired_refs - 1;
> -	for (i = 0; i < state->acquired_refs; i++) {
> -		if (state->refs[i].type != REF_TYPE_PTR)
> -			continue;
> -		if (state->refs[i].id == ptr_id) {
> -			if (last_idx && i != last_idx)
> -				memcpy(&state->refs[i], &state->refs[last_idx],
> -				       sizeof(*state->refs));
> -			memset(&state->refs[last_idx], 0, sizeof(*state->refs));
> -			state->acquired_refs--;
> -			return 0;
> -		}
> -	}
> -	return -EINVAL;
> +	if (last_idx && idx != last_idx)
> +		memcpy(&state->refs[idx], &state->refs[last_idx], sizeof(*state->refs));
> +	memset(&state->refs[last_idx], 0, sizeof(*state->refs));
> +	state->acquired_refs--;
> +	return;
>  }

Such implementation replaces element at 'idx' with element at 'last_idx'.
If the intention is to use 'state->refs' as a stack of acquired irq flags,
the stack property would be broken by this trick.
E.g. consider array [a, b, c, d] where 'idx' points to 'b',
after release_reference_state() the array would become [a, d, c].
You need to do 'memmove' instead.

[...]

> @@ -9666,21 +9659,41 @@ static void mark_pkt_end(struct bpf_verifier_state *vstate, int regn, bool range
>  		reg->range = AT_PKT_END;
>  }
>  
> +static int release_reference_nomark(struct bpf_verifier_state *state, int ref_obj_id)
> +{
> +	int i;
> +
> +	for (i = 0; i < state->acquired_refs; i++) {
> +		if (state->refs[i].type != REF_TYPE_PTR)
> +			continue;
> +		if (state->refs[i].id == ref_obj_id) {
> +			release_reference_state(state, i);
> +			return 0;
> +		}
> +	}
> +	return -EINVAL;
> +}
> +
>  /* The pointer with the specified id has released its reference to kernel
>   * resources. Identify all copies of the same pointer and clear the reference.
> + *
> + * This is the release function corresponding to acquire_reference(). Idempotent.
> + * The 'mark' boolean is used to optionally skip scrubbing registers matching
          ^^^^^^
Nit: this is probably a remnant of some older patch revision,
     function no longer takes 'mark' parameter.

> + * the ref_obj_id, in case they need to be switched to some other type instead
> + * of havoc scalar value.
>   */
> -static int release_reference(struct bpf_verifier_env *env,
> -			     int ref_obj_id)
> +static int release_reference(struct bpf_verifier_env *env, int ref_obj_id)
>  {

[...]
Kumar Kartikeya Dwivedi Nov. 28, 2024, 4:30 a.m. UTC | #2 
On Thu, 28 Nov 2024 at 05:13, Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Wed, 2024-11-27 at 08:58 -0800, Kumar Kartikeya Dwivedi wrote:
>
> Overall looks good, but please take a look at a few notes below.
>
> [...]
>
> > @@ -1349,77 +1350,69 @@ static int grow_stack_state(struct bpf_verifier_env *env, struct bpf_func_state
> >   * On success, returns a valid pointer id to associate with the register
> >   * On failure, returns a negative errno.
> >   */
> > -static int acquire_reference_state(struct bpf_verifier_env *env, int insn_idx)
> > +static struct bpf_reference_state *acquire_reference_state(struct bpf_verifier_env *env, int insn_idx, bool gen_id)
> >  {
> >       struct bpf_verifier_state *state = env->cur_state;
> >       int new_ofs = state->acquired_refs;
> > -     int id, err;
> > +     int err;
> >
> >       err = resize_reference_state(state, state->acquired_refs + 1);
> >       if (err)
> > -             return err;
> > -     id = ++env->id_gen;
> > -     state->refs[new_ofs].type = REF_TYPE_PTR;
> > -     state->refs[new_ofs].id = id;
> > +             return NULL;
> > +     if (gen_id)
> > +             state->refs[new_ofs].id = ++env->id_gen;
>
> Nit: state->refs[new_ods].id might end up with garbage value if 'gen_id' is false.
>      The resize_reference_state() uses realloc_array(),
>      which allocates memory with GFP_KERNEL, but without __GFP_ZERO flag.
>      This is not a problem with current patch, as you always check
>      reference type before checking id, but most of the data strucures
>      in verifier are zero initialized just in case.

We end up assigning to s->id if gen_id is false, e.g.
acquire_lock_state, so I think we'll be fine without __GFP_ZERO.

>
> >       state->refs[new_ofs].insn_idx = insn_idx;
> >
> > -     return id;
> > +     return &state->refs[new_ofs];
> > +}
>
> [...]
>
> > -/* release function corresponding to acquire_reference_state(). Idempotent. */
> > -static int release_reference_state(struct bpf_verifier_state *state, int ptr_id)
> > +static void release_reference_state(struct bpf_verifier_state *state, int idx)
> >  {
> > -     int i, last_idx;
> > +     int last_idx;
> >
> >       last_idx = state->acquired_refs - 1;
> > -     for (i = 0; i < state->acquired_refs; i++) {
> > -             if (state->refs[i].type != REF_TYPE_PTR)
> > -                     continue;
> > -             if (state->refs[i].id == ptr_id) {
> > -                     if (last_idx && i != last_idx)
> > -                             memcpy(&state->refs[i], &state->refs[last_idx],
> > -                                    sizeof(*state->refs));
> > -                     memset(&state->refs[last_idx], 0, sizeof(*state->refs));
> > -                     state->acquired_refs--;
> > -                     return 0;
> > -             }
> > -     }
> > -     return -EINVAL;
> > +     if (last_idx && idx != last_idx)
> > +             memcpy(&state->refs[idx], &state->refs[last_idx], sizeof(*state->refs));
> > +     memset(&state->refs[last_idx], 0, sizeof(*state->refs));
> > +     state->acquired_refs--;
> > +     return;
> >  }
>
> Such implementation replaces element at 'idx' with element at 'last_idx'.
> If the intention is to use 'state->refs' as a stack of acquired irq flags,
> the stack property would be broken by this trick.
> E.g. consider array [a, b, c, d] where 'idx' points to 'b',
> after release_reference_state() the array would become [a, d, c].
> You need to do 'memmove' instead.
>

Wow, great catch. Thanks for spotting this. I'll fix this and let me
see if I can add a selftest that would've triggered this particular
pattern.

> [...]
>
> > @@ -9666,21 +9659,41 @@ static void mark_pkt_end(struct bpf_verifier_state *vstate, int regn, bool range
> >               reg->range = AT_PKT_END;
> >  }
> >
> > +static int release_reference_nomark(struct bpf_verifier_state *state, int ref_obj_id)
> > +{
> > +     int i;
> > +
> > +     for (i = 0; i < state->acquired_refs; i++) {
> > +             if (state->refs[i].type != REF_TYPE_PTR)
> > +                     continue;
> > +             if (state->refs[i].id == ref_obj_id) {
> > +                     release_reference_state(state, i);
> > +                     return 0;
> > +             }
> > +     }
> > +     return -EINVAL;
> > +}
> > +
> >  /* The pointer with the specified id has released its reference to kernel
> >   * resources. Identify all copies of the same pointer and clear the reference.
> > + *
> > + * This is the release function corresponding to acquire_reference(). Idempotent.
> > + * The 'mark' boolean is used to optionally skip scrubbing registers matching
>           ^^^^^^
> Nit: this is probably a remnant of some older patch revision,
>      function no longer takes 'mark' parameter.

Yeah, this is a leftover. Sorry about that. Will fix.

>
> > + * the ref_obj_id, in case they need to be switched to some other type instead
> > + * of havoc scalar value.
> >   */
> > -static int release_reference(struct bpf_verifier_env *env,
> > -                          int ref_obj_id)
> > +static int release_reference(struct bpf_verifier_env *env, int ref_obj_id)
> >  {
>
> [...]
>
Eduard Zingerman Nov. 28, 2024, 4:36 a.m. UTC | #3 
On Thu, 2024-11-28 at 05:30 +0100, Kumar Kartikeya Dwivedi wrote:
> On Thu, 28 Nov 2024 at 05:13, Eduard Zingerman <eddyz87@gmail.com> wrote:
> > 
> > On Wed, 2024-11-27 at 08:58 -0800, Kumar Kartikeya Dwivedi wrote:
> > 
> > Overall looks good, but please take a look at a few notes below.
> > 
> > [...]
> > 
> > > @@ -1349,77 +1350,69 @@ static int grow_stack_state(struct bpf_verifier_env *env, struct bpf_func_state
> > >   * On success, returns a valid pointer id to associate with the register
> > >   * On failure, returns a negative errno.
> > >   */
> > > -static int acquire_reference_state(struct bpf_verifier_env *env, int insn_idx)
> > > +static struct bpf_reference_state *acquire_reference_state(struct bpf_verifier_env *env, int insn_idx, bool gen_id)
> > >  {
> > >       struct bpf_verifier_state *state = env->cur_state;
> > >       int new_ofs = state->acquired_refs;
> > > -     int id, err;
> > > +     int err;
> > > 
> > >       err = resize_reference_state(state, state->acquired_refs + 1);
> > >       if (err)
> > > -             return err;
> > > -     id = ++env->id_gen;
> > > -     state->refs[new_ofs].type = REF_TYPE_PTR;
> > > -     state->refs[new_ofs].id = id;
> > > +             return NULL;
> > > +     if (gen_id)
> > > +             state->refs[new_ofs].id = ++env->id_gen;
> > 
> > Nit: state->refs[new_ods].id might end up with garbage value if 'gen_id' is false.
> >      The resize_reference_state() uses realloc_array(),
> >      which allocates memory with GFP_KERNEL, but without __GFP_ZERO flag.
> >      This is not a problem with current patch, as you always check
> >      reference type before checking id, but most of the data strucures
> >      in verifier are zero initialized just in case.
> 
> We end up assigning to s->id if gen_id is false, e.g.
> acquire_lock_state, so I think we'll be fine without __GFP_ZERO.

Oh, I see, thank you for explaining.

[...]
diff mbox series

Patch

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index f8313e95eb8e..474cca3e8f66 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -196,7 +196,8 @@  struct bpf_verifier_stack_elem {
 
 #define BPF_PRIV_STACK_MIN_SIZE		64
 
-static int acquire_reference_state(struct bpf_verifier_env *env, int insn_idx);
+static int acquire_reference(struct bpf_verifier_env *env, int insn_idx);
+static int release_reference_nomark(struct bpf_verifier_state *state, int ref_obj_id);
 static int release_reference(struct bpf_verifier_env *env, int ref_obj_id);
 static void invalidate_non_owning_refs(struct bpf_verifier_env *env);
 static bool in_rbtree_lock_required_cb(struct bpf_verifier_env *env);
@@ -771,7 +772,7 @@  static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_
 		if (clone_ref_obj_id)
 			id = clone_ref_obj_id;
 		else
-			id = acquire_reference_state(env, insn_idx);
+			id = acquire_reference(env, insn_idx);
 
 		if (id < 0)
 			return id;
@@ -1033,7 +1034,7 @@  static int mark_stack_slots_iter(struct bpf_verifier_env *env,
 	if (spi < 0)
 		return spi;
 
-	id = acquire_reference_state(env, insn_idx);
+	id = acquire_reference(env, insn_idx);
 	if (id < 0)
 		return id;
 
@@ -1349,77 +1350,69 @@  static int grow_stack_state(struct bpf_verifier_env *env, struct bpf_func_state
  * On success, returns a valid pointer id to associate with the register
  * On failure, returns a negative errno.
  */
-static int acquire_reference_state(struct bpf_verifier_env *env, int insn_idx)
+static struct bpf_reference_state *acquire_reference_state(struct bpf_verifier_env *env, int insn_idx, bool gen_id)
 {
 	struct bpf_verifier_state *state = env->cur_state;
 	int new_ofs = state->acquired_refs;
-	int id, err;
+	int err;
 
 	err = resize_reference_state(state, state->acquired_refs + 1);
 	if (err)
-		return err;
-	id = ++env->id_gen;
-	state->refs[new_ofs].type = REF_TYPE_PTR;
-	state->refs[new_ofs].id = id;
+		return NULL;
+	if (gen_id)
+		state->refs[new_ofs].id = ++env->id_gen;
 	state->refs[new_ofs].insn_idx = insn_idx;
 
-	return id;
+	return &state->refs[new_ofs];
+}
+
+static int acquire_reference(struct bpf_verifier_env *env, int insn_idx)
+{
+	struct bpf_reference_state *s;
+
+	s = acquire_reference_state(env, insn_idx, true);
+	if (!s)
+		return -ENOMEM;
+	s->type = REF_TYPE_PTR;
+	return s->id;
 }
 
 static int acquire_lock_state(struct bpf_verifier_env *env, int insn_idx, enum ref_state_type type,
 			      int id, void *ptr)
 {
 	struct bpf_verifier_state *state = env->cur_state;
-	int new_ofs = state->acquired_refs;
-	int err;
+	struct bpf_reference_state *s;
 
-	err = resize_reference_state(state, state->acquired_refs + 1);
-	if (err)
-		return err;
-	state->refs[new_ofs].type = type;
-	state->refs[new_ofs].id = id;
-	state->refs[new_ofs].insn_idx = insn_idx;
-	state->refs[new_ofs].ptr = ptr;
+	s = acquire_reference_state(env, insn_idx, false);
+	s->type = type;
+	s->id = id;
+	s->ptr = ptr;
 
 	state->active_locks++;
 	return 0;
 }
 
-/* release function corresponding to acquire_reference_state(). Idempotent. */
-static int release_reference_state(struct bpf_verifier_state *state, int ptr_id)
+static void release_reference_state(struct bpf_verifier_state *state, int idx)
 {
-	int i, last_idx;
+	int last_idx;
 
 	last_idx = state->acquired_refs - 1;
-	for (i = 0; i < state->acquired_refs; i++) {
-		if (state->refs[i].type != REF_TYPE_PTR)
-			continue;
-		if (state->refs[i].id == ptr_id) {
-			if (last_idx && i != last_idx)
-				memcpy(&state->refs[i], &state->refs[last_idx],
-				       sizeof(*state->refs));
-			memset(&state->refs[last_idx], 0, sizeof(*state->refs));
-			state->acquired_refs--;
-			return 0;
-		}
-	}
-	return -EINVAL;
+	if (last_idx && idx != last_idx)
+		memcpy(&state->refs[idx], &state->refs[last_idx], sizeof(*state->refs));
+	memset(&state->refs[last_idx], 0, sizeof(*state->refs));
+	state->acquired_refs--;
+	return;
 }
 
 static int release_lock_state(struct bpf_verifier_state *state, int type, int id, void *ptr)
 {
-	int i, last_idx;
+	int i;
 
-	last_idx = state->acquired_refs - 1;
 	for (i = 0; i < state->acquired_refs; i++) {
 		if (state->refs[i].type != type)
 			continue;
 		if (state->refs[i].id == id && state->refs[i].ptr == ptr) {
-			if (last_idx && i != last_idx)
-				memcpy(&state->refs[i], &state->refs[last_idx],
-				       sizeof(*state->refs));
-			memset(&state->refs[last_idx], 0, sizeof(*state->refs));
-			state->acquired_refs--;
+			release_reference_state(state, i);
 			state->active_locks--;
 			return 0;
 		}
@@ -9666,21 +9659,41 @@  static void mark_pkt_end(struct bpf_verifier_state *vstate, int regn, bool range
 		reg->range = AT_PKT_END;
 }
 
+static int release_reference_nomark(struct bpf_verifier_state *state, int ref_obj_id)
+{
+	int i;
+
+	for (i = 0; i < state->acquired_refs; i++) {
+		if (state->refs[i].type != REF_TYPE_PTR)
+			continue;
+		if (state->refs[i].id == ref_obj_id) {
+			release_reference_state(state, i);
+			return 0;
+		}
+	}
+	return -EINVAL;
+}
+
 /* The pointer with the specified id has released its reference to kernel
  * resources. Identify all copies of the same pointer and clear the reference.
+ *
+ * This is the release function corresponding to acquire_reference(). Idempotent.
+ * The 'mark' boolean is used to optionally skip scrubbing registers matching
+ * the ref_obj_id, in case they need to be switched to some other type instead
+ * of havoc scalar value.
  */
-static int release_reference(struct bpf_verifier_env *env,
-			     int ref_obj_id)
+static int release_reference(struct bpf_verifier_env *env, int ref_obj_id)
 {
+	struct bpf_verifier_state *vstate = env->cur_state;
 	struct bpf_func_state *state;
 	struct bpf_reg_state *reg;
 	int err;
 
-	err = release_reference_state(env->cur_state, ref_obj_id);
+	err = release_reference_nomark(vstate, ref_obj_id);
 	if (err)
 		return err;
 
-	bpf_for_each_reg_in_vstate(env->cur_state, state, reg, ({
+	bpf_for_each_reg_in_vstate(vstate, state, reg, ({
 		if (reg->ref_obj_id == ref_obj_id)
 			mark_reg_invalid(env, reg);
 	}));
@@ -10774,7 +10787,7 @@  static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 			struct bpf_func_state *state;
 			struct bpf_reg_state *reg;
 
-			err = release_reference_state(env->cur_state, ref_obj_id);
+			err = release_reference_nomark(env->cur_state, ref_obj_id);
 			if (!err) {
 				bpf_for_each_reg_in_vstate(env->cur_state, state, reg, ({
 					if (reg->ref_obj_id == ref_obj_id) {
@@ -11107,7 +11120,7 @@  static int check_helper_call(struct bpf_verifier_env *env, struct bpf_insn *insn
 		/* For release_reference() */
 		regs[BPF_REG_0].ref_obj_id = meta.ref_obj_id;
 	} else if (is_acquire_function(func_id, meta.map_ptr)) {
-		int id = acquire_reference_state(env, insn_idx);
+		int id = acquire_reference(env, insn_idx);
 
 		if (id < 0)
 			return id;
@@ -13087,7 +13100,7 @@  static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
 		}
 		mark_btf_func_reg_size(env, BPF_REG_0, sizeof(void *));
 		if (is_kfunc_acquire(&meta)) {
-			int id = acquire_reference_state(env, insn_idx);
+			int id = acquire_reference(env, insn_idx);
 
 			if (id < 0)
 				return id;
@@ -15387,7 +15400,7 @@  static void mark_ptr_or_null_regs(struct bpf_verifier_state *vstate, u32 regno,
 		 * No one could have freed the reference state before
 		 * doing the NULL check.
 		 */
-		WARN_ON_ONCE(release_reference_state(vstate, id));
+		WARN_ON_ONCE(release_reference_nomark(vstate, id));
 
 	bpf_for_each_reg_in_vstate(vstate, state, reg, ({
 		mark_ptr_or_null_reg(state, reg, id, is_null);