On 9/10/26 11:22 PM, [email protected] wrote:
diff --git a/kernel/bpf/states.c b/kernel/bpf/states.c
index 66fb11b6c6a7..d974baad37ee 100644
--- a/kernel/bpf/states.c
+++ b/kernel/bpf/states.c
[ ... ]

@@ -369,13 +369,6 @@ static bool check_ids(u32 old_id, u32 cur_id, struct 
bpf_idmap *idmap)
   * and r7.id=0 (both independent), without temp IDs both would map old_id=X
   * to cur_id=0 and pass. With temp IDs: r6 maps X->temp1, r7 tries to map
   * X->temp2, but X is already mapped to temp1, so the check fails correctly.
- *
- * When old_id has BPF_ADD_CONST set, the compound id (base | flag) and the
- * base id (flag stripped) must both map consistently. Example: old has
- * r2.id=A, r3.id=A|flag (r3 = r2 + delta), cur has r2.id=B, r3.id=C|flag
- * (r3 derived from unrelated r4). Without the base check, idmap gets two
- * independent entries A->B and A|flag->C|flag, missing that A->C conflicts
- * with A->B. The base ID cross-check catches this.
   */
  static bool check_scalar_ids(u32 old_id, u32 cur_id, struct bpf_idmap *idmap)
  {
@@ -384,15 +377,7 @@ static bool check_scalar_ids(u32 old_id, u32 cur_id, 
struct bpf_idmap *idmap)

        cur_id = cur_id ? cur_id : ++idmap->tmp_id_gen;

-       if (!check_ids(old_id, cur_id, idmap))
-               return false;
-       if (old_id & BPF_ADD_CONST) {
-               old_id &= ~BPF_ADD_CONST;
-               cur_id &= ~BPF_ADD_CONST;
-               if (!check_ids(old_id, cur_id, idmap))
-                       return false;
-       }
-       return true;
+       return check_ids(old_id, cur_id, idmap);
  }
[ ... ]

diff --git a/tools/testing/selftests/bpf/progs/verifier_linked_scalars.c 
b/tools/testing/selftests/bpf/progs/verifier_linked_scalars.c
index d571fbfc86a3..da6cb961a520 100644
--- a/tools/testing/selftests/bpf/progs/verifier_linked_scalars.c
+++ b/tools/testing/selftests/bpf/progs/verifier_linked_scalars.c
[ ... ]

@@ -648,10 +648,10 @@ l_exit_%=:                                                
        \
  }

  /*
- * Test that regsafe() verifies base_id consistency for BPF_ADD_CONST
+ * Test that regsafe() verifies base_id consistency for ADD_CONST_*
   * linked scalars during state pruning.
   *
- * The false branch (explored first) links R3 to R2 via ADD_CONST.
+ * The false branch (explored first) links R3 to R2 via ADD_CONST_64.
   * The true branch (runtime path) links R3 to R4 (unrelated base_id).
   * At the merge point, pruning must fail because the linkage topology
   * differs.
Does this comment still match what the code does? The patch removes the
"base_id" concept from kernel/bpf/states.c (deleted the paragraph that
defined it, removed the two-level compound/base id check from
check_scalar_ids), but the comment here still describes the test as
verifying "base_id consistency" and refers to "unrelated base_id" on the
next line.

Looking at the function name and __description below:

   */
  __description("add_const base_id must be consistent for pruning")
  __failure __msg("invalid variable-offset")
  __naked void add_const_base_id_pruning(void)
and later at line 695:

        /* Runtime: R2.id=A, R4.id=C: base_ids A vs C inconsistent,
         * pruning should have failed. */
Should these be updated to use current terminology, or does "base_id"
still have meaning in this context? The test itself still works correctly
after the patch, so this appears to be a documentation consistency
question rather than a functional issue.

Indeed base_id as a term is gone, but just id alone might be confusing as well.
How about link id as a replacement for base_id

-Vineet



---
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/34506184282


Reply via email to