NACK/Cmnt: [SRU][N][PATCH 1/1] Revert "selftests/bpf: validate fake register spill/fill precision backtracking logic"
Andrei Gherzan
andrei.gherzan at canonical.com
Mon Sep 21 11:13:47 UTC 2026
On 2026/09/18 02:17PM, Aaron Jauregui via kernel-team wrote:
> BugLink: https://bugs.launchpad.net/bugs/2160493
>
> This reverts commit d8bdc8a63f97c861d6606dae3e3dcc571eb82d87.
>
> Upstream commit 7d8ed51bcb32 was backported to v6.6.140 and picked into noble
> via the upstream stable updates as d8bdc8a63f97. However, 7d8ed51bcb32 was
> merged in v6.8-rc1 and is therefore already present in the noble 6.8 base.
> Applying the backport adds a second copy of
> stack_load_preserves_const_precision tests, which breaks the selftests
> build.
>
> Signed-off-by: Aaron Jauregui <aaron.jauregui at canonical.com>
> ---
> .../selftests/bpf/progs/verifier_spill_fill.c | 154 ------------------
> 1 file changed, 154 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/progs/verifier_spill_fill.c b/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
> index 0a3a11a619b9..0feb18a3364b 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_spill_fill.c
> @@ -737,158 +737,4 @@ __naked void stack_load_preserves_const_precision_subreg(void)
> : __clobber_common);
> }
>
> -char two_byte_buf[2] SEC(".data.two_byte_buf");
> -
> -SEC("raw_tp")
> -__log_level(2) __flag(BPF_F_TEST_STATE_FREQ)
> -__success
> -/* make sure fp-8 is IMPRECISE fake register spill */
> -__msg("3: (7a) *(u64 *)(r10 -8) = 1 ; R10=fp0 fp-8_w=1")
> -/* and fp-16 is spilled IMPRECISE const reg */
> -__msg("5: (7b) *(u64 *)(r10 -16) = r0 ; R0_w=1 R10=fp0 fp-16_w=1")
> -/* validate load from fp-8, which was initialized using BPF_ST_MEM */
> -__msg("8: (79) r2 = *(u64 *)(r10 -8) ; R2_w=1 R10=fp0 fp-8=1")
> -__msg("9: (0f) r1 += r2")
> -__msg("mark_precise: frame0: last_idx 9 first_idx 7 subseq_idx -1")
> -__msg("mark_precise: frame0: regs=r2 stack= before 8: (79) r2 = *(u64 *)(r10 -8)")
> -__msg("mark_precise: frame0: regs= stack=-8 before 7: (bf) r1 = r6")
> -/* note, fp-8 is precise, fp-16 is not yet precise, we'll get there */
> -__msg("mark_precise: frame0: parent state regs= stack=-8: R0_w=1 R1=ctx(off=0,imm=0) R6_r=map_value(off=0,ks=4,vs=2,imm=0) R10=fp0 fp-8_rw=P1 fp-16_w=1")
> -__msg("mark_precise: frame0: last_idx 6 first_idx 3 subseq_idx 7")
> -__msg("mark_precise: frame0: regs= stack=-8 before 6: (05) goto pc+0")
> -__msg("mark_precise: frame0: regs= stack=-8 before 5: (7b) *(u64 *)(r10 -16) = r0")
> -__msg("mark_precise: frame0: regs= stack=-8 before 4: (b7) r0 = 1")
> -__msg("mark_precise: frame0: regs= stack=-8 before 3: (7a) *(u64 *)(r10 -8) = 1")
> -__msg("10: R1_w=map_value(off=1,ks=4,vs=2,imm=0) R2_w=1")
> -/* validate load from fp-16, which was initialized using BPF_STX_MEM */
> -__msg("12: (79) r2 = *(u64 *)(r10 -16) ; R2_w=1 R10=fp0 fp-16=1")
> -__msg("13: (0f) r1 += r2")
> -__msg("mark_precise: frame0: last_idx 13 first_idx 7 subseq_idx -1")
> -__msg("mark_precise: frame0: regs=r2 stack= before 12: (79) r2 = *(u64 *)(r10 -16)")
> -__msg("mark_precise: frame0: regs= stack=-16 before 11: (bf) r1 = r6")
> -__msg("mark_precise: frame0: regs= stack=-16 before 10: (73) *(u8 *)(r1 +0) = r2")
> -__msg("mark_precise: frame0: regs= stack=-16 before 9: (0f) r1 += r2")
> -__msg("mark_precise: frame0: regs= stack=-16 before 8: (79) r2 = *(u64 *)(r10 -8)")
> -__msg("mark_precise: frame0: regs= stack=-16 before 7: (bf) r1 = r6")
> -/* now both fp-8 and fp-16 are precise, very good */
> -__msg("mark_precise: frame0: parent state regs= stack=-16: R0_w=1 R1=ctx(off=0,imm=0) R6_r=map_value(off=0,ks=4,vs=2,imm=0) R10=fp0 fp-8_rw=P1 fp-16_rw=P1")
> -__msg("mark_precise: frame0: last_idx 6 first_idx 3 subseq_idx 7")
> -__msg("mark_precise: frame0: regs= stack=-16 before 6: (05) goto pc+0")
> -__msg("mark_precise: frame0: regs= stack=-16 before 5: (7b) *(u64 *)(r10 -16) = r0")
> -__msg("mark_precise: frame0: regs=r0 stack= before 4: (b7) r0 = 1")
> -__msg("14: R1_w=map_value(off=1,ks=4,vs=2,imm=0) R2_w=1")
> -__naked void stack_load_preserves_const_precision(void)
> -{
> - asm volatile (
> - /* establish checkpoint with state that has no stack slots;
> - * if we bubble up to this state without finding desired stack
> - * slot, then it's a bug and should be caught
> - */
> - "goto +0;"
> -
> - /* fp-8 is const 1 *fake* register */
> - ".8byte %[fp8_st_one];" /* LLVM-18+: *(u64 *)(r10 -8) = 1; */
> -
> - /* fp-16 is const 1 register */
> - "r0 = 1;"
> - "*(u64 *)(r10 -16) = r0;"
> -
> - /* force checkpoint to check precision marks preserved in parent states */
> - "goto +0;"
> -
> - /* load single U64 from aligned FAKE_REG=1 slot */
> - "r1 = %[two_byte_buf];"
> - "r2 = *(u64 *)(r10 -8);"
> - "r1 += r2;"
> - "*(u8 *)(r1 + 0) = r2;" /* this should be fine */
> -
> - /* load single U64 from aligned REG=1 slot */
> - "r1 = %[two_byte_buf];"
> - "r2 = *(u64 *)(r10 -16);"
> - "r1 += r2;"
> - "*(u8 *)(r1 + 0) = r2;" /* this should be fine */
> -
> - "r0 = 0;"
> - "exit;"
> - :
> - : __imm_ptr(two_byte_buf),
> - __imm_insn(fp8_st_one, BPF_ST_MEM(BPF_DW, BPF_REG_FP, -8, 1))
> - : __clobber_common);
> -}
> -
> -SEC("raw_tp")
> -__log_level(2) __flag(BPF_F_TEST_STATE_FREQ)
> -__success
> -/* make sure fp-8 is 32-bit FAKE subregister spill */
> -__msg("3: (62) *(u32 *)(r10 -8) = 1 ; R10=fp0 fp-8=1")
> -/* but fp-16 is spilled IMPRECISE zero const reg */
> -__msg("5: (63) *(u32 *)(r10 -16) = r0 ; R0_w=1 R10=fp0 fp-16=1")
> -/* validate load from fp-8, which was initialized using BPF_ST_MEM */
> -__msg("8: (61) r2 = *(u32 *)(r10 -8) ; R2_w=1 R10=fp0 fp-8=1")
> -__msg("9: (0f) r1 += r2")
> -__msg("mark_precise: frame0: last_idx 9 first_idx 7 subseq_idx -1")
> -__msg("mark_precise: frame0: regs=r2 stack= before 8: (61) r2 = *(u32 *)(r10 -8)")
> -__msg("mark_precise: frame0: regs= stack=-8 before 7: (bf) r1 = r6")
> -__msg("mark_precise: frame0: parent state regs= stack=-8: R0_w=1 R1=ctx(off=0,imm=0) R6_r=map_value(off=0,ks=4,vs=2,imm=0) R10=fp0 fp-8_r=P1 fp-16=1")
> -__msg("mark_precise: frame0: last_idx 6 first_idx 3 subseq_idx 7")
> -__msg("mark_precise: frame0: regs= stack=-8 before 6: (05) goto pc+0")
> -__msg("mark_precise: frame0: regs= stack=-8 before 5: (63) *(u32 *)(r10 -16) = r0")
> -__msg("mark_precise: frame0: regs= stack=-8 before 4: (b7) r0 = 1")
> -__msg("mark_precise: frame0: regs= stack=-8 before 3: (62) *(u32 *)(r10 -8) = 1")
> -__msg("10: R1_w=map_value(off=1,ks=4,vs=2,imm=0) R2_w=1")
> -/* validate load from fp-16, which was initialized using BPF_STX_MEM */
> -__msg("12: (61) r2 = *(u32 *)(r10 -16) ; R2_w=1 R10=fp0 fp-16=1")
> -__msg("13: (0f) r1 += r2")
> -__msg("mark_precise: frame0: last_idx 13 first_idx 7 subseq_idx -1")
> -__msg("mark_precise: frame0: regs=r2 stack= before 12: (61) r2 = *(u32 *)(r10 -16)")
> -__msg("mark_precise: frame0: regs= stack=-16 before 11: (bf) r1 = r6")
> -__msg("mark_precise: frame0: regs= stack=-16 before 10: (73) *(u8 *)(r1 +0) = r2")
> -__msg("mark_precise: frame0: regs= stack=-16 before 9: (0f) r1 += r2")
> -__msg("mark_precise: frame0: regs= stack=-16 before 8: (61) r2 = *(u32 *)(r10 -8)")
> -__msg("mark_precise: frame0: regs= stack=-16 before 7: (bf) r1 = r6")
> -__msg("mark_precise: frame0: parent state regs= stack=-16: R0_w=1 R1=ctx(off=0,imm=0) R6_r=map_value(off=0,ks=4,vs=2,imm=0) R10=fp0 fp-8_r=P1 fp-16_r=P1")
> -__msg("mark_precise: frame0: last_idx 6 first_idx 3 subseq_idx 7")
> -__msg("mark_precise: frame0: regs= stack=-16 before 6: (05) goto pc+0")
> -__msg("mark_precise: frame0: regs= stack=-16 before 5: (63) *(u32 *)(r10 -16) = r0")
> -__msg("mark_precise: frame0: regs=r0 stack= before 4: (b7) r0 = 1")
> -__msg("14: R1_w=map_value(off=1,ks=4,vs=2,imm=0) R2_w=1")
> -__naked void stack_load_preserves_const_precision_subreg(void)
> -{
> - asm volatile (
> - /* establish checkpoint with state that has no stack slots;
> - * if we bubble up to this state without finding desired stack
> - * slot, then it's a bug and should be caught
> - */
> - "goto +0;"
> -
> - /* fp-8 is const 1 *fake* SUB-register */
> - ".8byte %[fp8_st_one];" /* LLVM-18+: *(u32 *)(r10 -8) = 1; */
> -
> - /* fp-16 is const 1 SUB-register */
> - "r0 = 1;"
> - "*(u32 *)(r10 -16) = r0;"
> -
> - /* force checkpoint to check precision marks preserved in parent states */
> - "goto +0;"
> -
> - /* load single U32 from aligned FAKE_REG=1 slot */
> - "r1 = %[two_byte_buf];"
> - "r2 = *(u32 *)(r10 -8);"
> - "r1 += r2;"
> - "*(u8 *)(r1 + 0) = r2;" /* this should be fine */
> -
> - /* load single U32 from aligned REG=1 slot */
> - "r1 = %[two_byte_buf];"
> - "r2 = *(u32 *)(r10 -16);"
> - "r1 += r2;"
> - "*(u8 *)(r1 + 0) = r2;" /* this should be fine */
> -
> - "r0 = 0;"
> - "exit;"
> - :
> - : __imm_ptr(two_byte_buf),
> - __imm_insn(fp8_st_one, BPF_ST_MEM(BPF_W, BPF_REG_FP, -8, 1)) /* 32-bit spill */
> - : __clobber_common);
> -}
> -
> char _license[] SEC("license") = "GPL";
This is an Ubuntu-originated revert, not a revert that came from upstream. That
means it should be marked accordingly as "UBUNTU SAUCE".
The patch looks fine otherwise.
--
Andrei Gherzan
gpg: rsa4096/D4D94F67AD0E9640
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20260921/a98aec1a/attachment.sig>
More information about the kernel-team
mailing list