Thread (23 messages) 23 messages, 3 authors, 2026-05-22

Re: [PATCH bpf-next v5 03/14] bpf: Assign reg->id when getting referenced kptr from ctx

From: Amery Hung <hidden>
Date: 2026-05-19 20:17:57
Also in: bpf

On Tue, May 19, 2026 at 11:56 AM [off-list ref] wrote:
quoted
bpf: Assign reg->id when getting referenced kptr from ctx

Assign reg->id when getting referenced kptr from read program context
to be consistent with R0 of KF_ACQUIRE kfunc. skb dynptr will track the
referenced skb in qdisc programs using a new field reg->parent_id in
a later patch.

Acked-by: Andrii Nakryiko <andrii@kernel.org>
Acked-by: Eduard Zingerman <eddyz87@gmail.com>
Signed-off-by: Amery Hung <redacted>
During the v3 review on lore, Eduard Zingerman noted that this patch is
mandatory for the series because the .id link is needed to propagate
invalidation triggered by free_skb() to child objects. Specifically, the
test bpf_qdisc_fail__invalid_dynptr_cross_frame.c:invalid_dynptr_cross_frame
fails if the line "regs[value_regno].id = info.ref_obj_id;" is commented
out. This invalidation propagation aspect is not mentioned in the current
commit message.

Reference:
  https://lore.kernel.org/bpf/c84a7beccd4ae527981797a6e5a6af840ba9c312.camel@gmail.com/ (local)
  Date: Wed, 22 Apr 2026 15:50:52 -0700
  Reviewer: Eduard Zingerman

Could the commit message be updated to mention the invalidation propagation
requirement?
quoted
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 564679f70afd..441950b43a4c 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -6209,8 +6209,6 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, struct b
                      } else {
                              mark_reg_known_zero(env, regs,
                                                  value_regno);
-                             if (type_may_be_null(info.reg_type))
-                                     regs[value_regno].id = ++env->id_gen;
                              /* A load of ctx field could have different
                               * actual load size with the one encoded in the
                               * insn. When the dst is PTR, it is for sure not
@@ -6220,8 +6218,11 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, struct b
                              if (base_type(info.reg_type) == PTR_TO_BTF_ID) {
                                      regs[value_regno].btf = info.btf;
                                      regs[value_regno].btf_id = info.btf_id;
+                                     regs[value_regno].id = info.ref_obj_id;
                                      regs[value_regno].ref_obj_id = info.ref_obj_id;
                              }
+                             if (type_may_be_null(info.reg_type) && !regs[value_regno].id)
+                                     regs[value_regno].id = ++env->id_gen;
                      }
                      regs[value_regno].type = info.reg_type;
              }
This looks like a bug fix for the id generation logic introduced when
support for may-be-null types was added to the check_ctx_access path. The
issue is that when getting a referenced kptr from program context (PTR_TO_BTF_ID
with ref_obj_id), the code was generating a fresh id for nullable types before
assigning ref_obj_id, causing inconsistency with KF_ACQUIRE kfunc behavior
where reg->id is set to info.ref_obj_id.

Should this include:

  Fixes: 93c230e3f5bd ("bpf: Enforce id generation for all may-be-null register type")
The inconsistency is not a bug. It is only needed later when I added
reg->parent_id that track reg->id. We do not need a fix tag here.

---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26116680637
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help