Re: [PATCH bpf-next v3 4/9] bpf: Refactor object relationship tracking and fix dynptr UAF bug

On Fri, Apr 24, 2026 at 3:48 PM Eduard Zingerman [off-list ref] wrote:

On Tue, 2026-04-21 at 15:10 -0700, Amery Hung wrote:

Tbh, I find current state of affairs with id/ref_obj_id/parent_id hard
to follow. The release_reference() is an improvement, but the means by
which the fields are propagated to bpf_reg_state objects are convoluted.
I wonder if having a separate "object table" in bpf_verifier_state and
having bpf_reg_state->id refer to objects within this table would make
things more straight forward.

I think this is a good idea in the long term. First, we need to make
id a stable and unique object identifier (i.e., always assign id to
objects; different objects have different ids). Then, we can create
the object table indexed by id and each entry contains the ref_obj_id
and parent_id moved from bpf_reg_state. Then, there will be helpers
managing the lifetime, relationship (e.g., bpf_obj_create,
bpf_obj_release, bpf_obj_clone) with clear semantics to centralize how
id,parent_id,ref_obj_id are manipulated.

If this makes sense, I can send this as a follow up patchset.

A few nits below.

[...]

quoted

diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h

[...]

quoted

@@ -1323,6 +1335,7 @@ struct bpf_dynptr_desc {
      enum bpf_dynptr_type type;
      u32 id;
      u32 ref_obj_id;
+     u32 parent_id;
 };

Nit: would be nice to have a comment describing when the above fields
     are populated.

Ack

quoted

 struct bpf_kfunc_call_arg_meta {

@@ -1334,6 +1347,7 @@ struct bpf_kfunc_call_arg_meta {
      const char *func_name;
      /* Out parameters */
      u32 ref_obj_id;
+     u32 id;

Nit: would be nice to have a comment here too.

Ack

[...]

quoted

diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
index 8478d2c6ed5b..72bd3bcda5fb 100644
--- a/kernel/bpf/states.c
+++ b/kernel/bpf/states.c

@@ -494,7 +494,8 @@ static bool regs_exact(const struct bpf_reg_state *rold,
 {
      return memcmp(rold, rcur, offsetof(struct bpf_reg_state, id)) == 0 &&
             check_ids(rold->id, rcur->id, idmap) &&
-            check_ids(rold->ref_obj_id, rcur->ref_obj_id, idmap);
+            check_ids(rold->ref_obj_id, rcur->ref_obj_id, idmap) &&
+            check_ids(rold->parent_id, rcur->parent_id, idmap);

Nit: these check_ids() become repetitive, maybe add a utility function
     checking id/ref_obj_id/parent_id?

Will add a utility function.

quoted

 }

 enum exact_level {

@@ -619,7 +620,8 @@ static bool regsafe(struct bpf_verifier_env *env, struct bpf_reg_state *rold,
                     range_within(rold, rcur) &&
                     tnum_in(rold->var_off, rcur->var_off) &&
                     check_ids(rold->id, rcur->id, idmap) &&
-                    check_ids(rold->ref_obj_id, rcur->ref_obj_id, idmap);
+                    check_ids(rold->ref_obj_id, rcur->ref_obj_id, idmap) &&
+                    check_ids(rold->parent_id, rcur->parent_id, idmap);
      case PTR_TO_PACKET_META:
      case PTR_TO_PACKET:
              /* We must have at least as much range as the old ptr

@@ -799,7 +801,8 @@ static bool stacksafe(struct bpf_verifier_env *env, struct bpf_func_state *old,
                      cur_reg = &cur->stack[spi].spilled_ptr;
                      if (old_reg->dynptr.type != cur_reg->dynptr.type ||
                          old_reg->dynptr.first_slot != cur_reg->dynptr.first_slot ||
-                         !check_ids(old_reg->ref_obj_id, cur_reg->ref_obj_id, idmap))
+                         !check_ids(old_reg->ref_obj_id, cur_reg->ref_obj_id, idmap) ||
+                         !check_ids(old_reg->parent_id, cur_reg->parent_id, idmap))

Not something changed by the current patch, but still a question:
this path ignores old_reg->id, is it a bug?

Yes. I will also check id.

quoted

                              return false;
                      break;
              case STACK_ITER:

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0313b7d5f6c9..908a3af0e7c4 100644

[...]

quoted

@@ -241,6 +241,7 @@ struct bpf_call_arg_meta {
      int mem_size;
      u64 msize_max_value;
      int ref_obj_id;
+     u32 id;

Nit: please add a comment here as well.

Ack

[...]

quoted

@@ -635,11 +636,12 @@ static int destroy_if_dynptr_stack_slot(struct bpf_verifier_env *env,
                                      struct bpf_func_state *state, int spi);

 static int mark_stack_slots_dynptr(struct bpf_verifier_env *env, struct bpf_reg_state *reg,
-                                enum bpf_arg_type arg_type, int insn_idx, int clone_ref_obj_id)
+                                enum bpf_arg_type arg_type, int insn_idx, int parent_id,
+                                struct bpf_dynptr_desc *dynptr)

Having both parent_id and dynptr->parent_id as parameters of this
function is very confusing, but I don't have a suggestion on how to
better deal with it.

Does tweaking the argument name and reorder the if-else block make it
more obvious how these two arguments are used?

if (clone_dynptr->type != BPF_DYNPTR_TYPE_INVALID) { /* bpf_dynptr_clone() */
        ref_obj_id = clone_dynptr->ref_obj_id;
        parent_id = clone_dynptr->parent_id;
} else { /* dynptr constructors */
        if (dynptr_type_referenced(type)) {
                ref_obj_id = acquire_reference(env, insn_idx);
                if (ref_obj_id < 0)
                        return ref_obj_id;
        }
}

[...]

quoted

@@ -8489,6 +8444,7 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 arg,
                      return -EACCES;
              }
              meta->ref_obj_id = reg->ref_obj_id;
+             meta->id = reg->id;

Could you please leave a comment here explaining when e.g.
meta->ref_obj_id and meta->id would have same or different ids here?
Maybe a few lines of BPF C code.

Ack

quoted

      }

      switch (base_type(arg_type)) {

@@ -9111,26 +9067,75 @@ static int release_reference_nomark(struct bpf_verifier_state *state, int ref_ob

[...]

quoted

+/* Release id and objects referencing the id iteratively in a DFS manner */
+static int release_reference(struct bpf_verifier_env *env, int id)
+{
+     u32 mask = (1 << STACK_SPILL) | (1 << STACK_DYNPTR);
      struct bpf_verifier_state *vstate = env->cur_state;
+     struct bpf_idmap *idstack = &env->idmap_scratch;
+     struct bpf_stack_state *stack;
      struct bpf_func_state *state;
      struct bpf_reg_state *reg;
-     int err;
+     int root_id = id, err;

-     err = release_reference_nomark(vstate, ref_obj_id);
-     if (err)
-             return err;
+     idstack->cnt = 0;
+     idstack_push(idstack, id);

-     bpf_for_each_reg_in_vstate(vstate, state, reg, ({
-             if (reg->ref_obj_id == ref_obj_id)
-                     mark_reg_invalid(env, reg);
-     }));
+     if (find_reference_state(vstate, id))
+             WARN_ON_ONCE(release_reference_nomark(vstate, id));
+
+     while ((id = idstack_pop(idstack))) {
+             bpf_for_each_reg_in_vstate_mask(vstate, state, reg, stack, mask, ({
+                     if (reg->id != id && reg->parent_id != id && reg->ref_obj_id != id)
+                             continue;
+
+                     if (reg->ref_obj_id && id != root_id) {
+                             struct bpf_reference_state *ref_state;
+
+                             ref_state = find_reference_state(env->cur_state, reg->ref_obj_id);
+                             verbose(env, "Unreleased reference id=%d alloc_insn=%d when releasing id=%d\n",
+                                     ref_state->id, ref_state->insn_idx, root_id);
+                             return -EINVAL;
+                     }
+
+                     if (reg->id != id) {
+                             err = idstack_push(idstack, reg->id);
+                             if (err)
+                                     return err;
+                     }
+
+                     if (!stack || stack->slot_type[BPF_REG_SIZE - 1] == STACK_SPILL)
+                             mark_reg_invalid(env, reg);
+                     else if (stack->slot_type[BPF_REG_SIZE - 1] == STACK_DYNPTR)
+                             invalidate_dynptr(env, state, stack);

invalidate_dynptr() rewrites to stack slots, can it be the case that
this body of bpf_for_each_reg_in_vstate_mask() is computed for first
and second dynptr stack slots, hence triggering invalidate_dynptr() to
rewrite three slots instead of two?

invalidate_dynptr() will mark the two stack slots as STACK_INVALID so
I didn't bother to check whether it is the first slot or not. Am I
missing anything?

quoted

+             }));
+     }

      return 0;
 }

[...]

quoted

@@ -12009,6 +12009,7 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
                              return -EFAULT;
                      }
                      meta->ref_obj_id = reg->ref_obj_id;
+                     meta->id = reg->id;

And here a comment describing when this happens would be helpful.

Ack

[...]

quoted

@@ -12171,15 +12171,10 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
                              }

                              dynptr_arg_type |= (unsigned int)get_dynptr_type_flag(parent_type);
-                             clone_ref_obj_id = meta->dynptr.ref_obj_id;
-                             if (dynptr_type_refcounted(parent_type) && !clone_ref_obj_id) {
-                                     verifier_bug(env, "missing ref obj id for parent of clone");
-                                     return -EFAULT;
-                             }
                      }

-                     ret = process_dynptr_func(env, regno, insn_idx, dynptr_arg_type, clone_ref_obj_id,
-                                               &meta->dynptr);
+                     ret = process_dynptr_func(env, regno, insn_idx, dynptr_arg_type,
+                                               meta->ref_obj_id ? meta->id : 0, &meta->dynptr);

And an example here as well.

Ack

quoted

                      if (ret < 0)
                              return ret;
                      break;

[...]