* Re: [PATCH bpf-next v5 2/2] selftests/bpf: Add tests with stack ptr register in conditional jmp [not found] ` <20250524041340.4046304-1-yonghong.song@linux.dev> @ 2025-07-16 10:13 ` Shung-Hsi Yu 2025-07-16 16:05 ` Yonghong Song 0 siblings, 1 reply; 3+ messages in thread From: Shung-Hsi Yu @ 2025-07-16 10:13 UTC (permalink / raw) To: Andrii Nakryiko, Yonghong Song Cc: bpf, Alexei Starovoitov, Daniel Borkmann, kernel-team, Martin KaFai Lau, stable, Sasha Levin Hi Andrii and Yonghong, On Fri, May 23, 2025 at 09:13:40PM -0700, Yonghong Song wrote: > Add two tests: > - one test has 'rX <op> r10' where rX is not r10, and > - another test has 'rX <op> rY' where rX and rY are not r10 > but there is an early insn 'rX = r10'. > > Without previous verifier change, both tests will fail. > > Signed-off-by: Yonghong Song <yonghong.song@linux.dev> > --- > .../selftests/bpf/progs/verifier_precision.c | 53 +++++++++++++++++++ > 1 file changed, 53 insertions(+) I was looking this commit (5ffb537e416e) since it was a BPF selftest test for CVE-2025-38279, but upon looking I found that the commit differs from the patch, there is an extra hunk that changed kernel/bpf/verifier.c that wasn't found the Yonghong's original patch. I suppose it was meant to be squashed into the previous commit e2d2115e56c4 "bpf: Do not include stack ptr register in precision backtracking bookkeeping"? Since stable backports got only e2d2115e56c4, but not the 5ffb537e416e here with the extra change for kernel/bpf/verifier.c, I'd guess the backtracking logic in the stable kernel isn't correct at the moment, so I'll send 5ffb537e416e "selftests/bpf: Add tests with stack ptr register in conditional jmp" to stable as well. Let me know if that's not the right thing to do. Shung-Hsi diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 98c52829936e..a7d6e0c5928b 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -16456,6 +16456,8 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env, if (src_reg->type == PTR_TO_STACK) insn_flags |= INSN_F_SRC_REG_STACK; + if (dst_reg->type == PTR_TO_STACK) + insn_flags |= INSN_F_DST_REG_STACK; } else { if (insn->src_reg != BPF_REG_0) { verbose(env, "BPF_JMP/JMP32 uses reserved fields\n"); @@ -16465,10 +16467,11 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env, memset(src_reg, 0, sizeof(*src_reg)); src_reg->type = SCALAR_VALUE; __mark_reg_known(src_reg, insn->imm); + + if (dst_reg->type == PTR_TO_STACK) + insn_flags |= INSN_F_DST_REG_STACK; } - if (dst_reg->type == PTR_TO_STACK) - insn_flags |= INSN_F_DST_REG_STACK; if (insn_flags) { err = push_insn_history(env, this_branch, insn_flags, 0); if (err) > diff --git a/tools/testing/selftests/bpf/progs/verifier_precision.c b/tools/testing/selftests/bpf/progs/verifier_precision.c ... ^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next v5 2/2] selftests/bpf: Add tests with stack ptr register in conditional jmp 2025-07-16 10:13 ` [PATCH bpf-next v5 2/2] selftests/bpf: Add tests with stack ptr register in conditional jmp Shung-Hsi Yu @ 2025-07-16 16:05 ` Yonghong Song 2025-07-18 6:13 ` Shung-Hsi Yu 0 siblings, 1 reply; 3+ messages in thread From: Yonghong Song @ 2025-07-16 16:05 UTC (permalink / raw) To: Shung-Hsi Yu, Andrii Nakryiko Cc: bpf, Alexei Starovoitov, Daniel Borkmann, kernel-team, Martin KaFai Lau, stable, Sasha Levin On 7/16/25 3:13 AM, Shung-Hsi Yu wrote: > Hi Andrii and Yonghong, > > On Fri, May 23, 2025 at 09:13:40PM -0700, Yonghong Song wrote: >> Add two tests: >> - one test has 'rX <op> r10' where rX is not r10, and >> - another test has 'rX <op> rY' where rX and rY are not r10 >> but there is an early insn 'rX = r10'. >> >> Without previous verifier change, both tests will fail. >> >> Signed-off-by: Yonghong Song <yonghong.song@linux.dev> >> --- >> .../selftests/bpf/progs/verifier_precision.c | 53 +++++++++++++++++++ >> 1 file changed, 53 insertions(+) > I was looking this commit (5ffb537e416e) since it was a BPF selftest > test for CVE-2025-38279, but upon looking I found that the commit > differs from the patch, there is an extra hunk that changed > kernel/bpf/verifier.c that wasn't found the Yonghong's original patch. > > I suppose it was meant to be squashed into the previous commit > e2d2115e56c4 "bpf: Do not include stack ptr register in precision > backtracking bookkeeping"? Andrii made some change to my original patch for easy understanding. See https://lore.kernel.org/bpf/20250524041335.4046126-1-yonghong.song@linux.dev Quoted below: " I've moved it inside the preceding if/else (twice), so it's more obvious that BPF_X deal with both src_reg and dst_reg, and BPF_K case deals only with BPF_K. The end result is the same, but I found this way a bit easier to follow. Applied to bpf-next, thanks. diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 831c2eff56e1..c9a372ca7830 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -16471,6 +16471,8 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env, if (src_reg->type == PTR_TO_STACK) insn_flags |= INSN_F_SRC_REG_STACK; + if (dst_reg->type == PTR_TO_STACK) + insn_flags |= INSN_F_DST_REG_STACK; } else { if (insn->src_reg != BPF_REG_0) { verbose(env, "BPF_JMP/JMP32 uses reserved fields\n"); @@ -16480,10 +16482,11 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env, memset(src_reg, 0, sizeof(*src_reg)); src_reg->type = SCALAR_VALUE; __mark_reg_known(src_reg, insn->imm); + + if (dst_reg->type == PTR_TO_STACK) + insn_flags |= INSN_F_DST_REG_STACK; } - if (dst_reg->type == PTR_TO_STACK) - insn_flags |= INSN_F_DST_REG_STACK; if (insn_flags) { err = push_insn_history(env, this_branch, insn_flags, 0); if (err) ... " > > Since stable backports got only e2d2115e56c4, but not the 5ffb537e416e > here with the extra change for kernel/bpf/verifier.c, I'd guess the > backtracking logic in the stable kernel isn't correct at the moment, > so I'll send 5ffb537e416e "selftests/bpf: Add tests with stack ptr > register in conditional jmp" to stable as well. Let me know if that's > not the right thing to do. > > Shung-Hsi > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 98c52829936e..a7d6e0c5928b 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -16456,6 +16456,8 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env, > > if (src_reg->type == PTR_TO_STACK) > insn_flags |= INSN_F_SRC_REG_STACK; > + if (dst_reg->type == PTR_TO_STACK) > + insn_flags |= INSN_F_DST_REG_STACK; > } else { > if (insn->src_reg != BPF_REG_0) { > verbose(env, "BPF_JMP/JMP32 uses reserved fields\n"); > @@ -16465,10 +16467,11 @@ static int check_cond_jmp_op(struct bpf_verifier_env *env, > memset(src_reg, 0, sizeof(*src_reg)); > src_reg->type = SCALAR_VALUE; > __mark_reg_known(src_reg, insn->imm); > + > + if (dst_reg->type == PTR_TO_STACK) > + insn_flags |= INSN_F_DST_REG_STACK; > } > > - if (dst_reg->type == PTR_TO_STACK) > - insn_flags |= INSN_F_DST_REG_STACK; > if (insn_flags) { > err = push_insn_history(env, this_branch, insn_flags, 0); > if (err) > >> diff --git a/tools/testing/selftests/bpf/progs/verifier_precision.c b/tools/testing/selftests/bpf/progs/verifier_precision.c > ... > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH bpf-next v5 2/2] selftests/bpf: Add tests with stack ptr register in conditional jmp 2025-07-16 16:05 ` Yonghong Song @ 2025-07-18 6:13 ` Shung-Hsi Yu 0 siblings, 0 replies; 3+ messages in thread From: Shung-Hsi Yu @ 2025-07-18 6:13 UTC (permalink / raw) To: Yonghong Song Cc: Andrii Nakryiko, bpf, Alexei Starovoitov, Daniel Borkmann, kernel-team, Martin KaFai Lau, stable, Sasha Levin On Wed, Jul 16, 2025 at 09:05:05AM -0700, Yonghong Song wrote: > On 7/16/25 3:13 AM, Shung-Hsi Yu wrote: > > Hi Andrii and Yonghong, > > > > On Fri, May 23, 2025 at 09:13:40PM -0700, Yonghong Song wrote: > > > Add two tests: > > > - one test has 'rX <op> r10' where rX is not r10, and > > > - another test has 'rX <op> rY' where rX and rY are not r10 > > > but there is an early insn 'rX = r10'. > > > > > > Without previous verifier change, both tests will fail. > > > > > > Signed-off-by: Yonghong Song <yonghong.song@linux.dev> > > > --- > > > .../selftests/bpf/progs/verifier_precision.c | 53 +++++++++++++++++++ > > > 1 file changed, 53 insertions(+) > > I was looking this commit (5ffb537e416e) since it was a BPF selftest > > test for CVE-2025-38279, but upon looking I found that the commit > > differs from the patch, there is an extra hunk that changed > > kernel/bpf/verifier.c that wasn't found the Yonghong's original patch. > > > > I suppose it was meant to be squashed into the previous commit > > e2d2115e56c4 "bpf: Do not include stack ptr register in precision > > backtracking bookkeeping"? > > Andrii made some change to my original patch for easy understanding. > See > https://lore.kernel.org/bpf/20250524041335.4046126-1-yonghong.song@linux.dev > Quoted below: > " > I've moved it inside the preceding if/else (twice), so it's more > obvious that BPF_X deal with both src_reg and dst_reg, and BPF_K case > deals only with BPF_K. The end result is the same, but I found this > way a bit easier to follow. Applied to bpf-next, thanks. Argh, indeed I missed the sibling thread. Thanks for point that out. Shung-Hsi ... ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-07-18 6:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20250524041335.4046126-1-yonghong.song@linux.dev>
[not found] ` <20250524041340.4046304-1-yonghong.song@linux.dev>
2025-07-16 10:13 ` [PATCH bpf-next v5 2/2] selftests/bpf: Add tests with stack ptr register in conditional jmp Shung-Hsi Yu
2025-07-16 16:05 ` Yonghong Song
2025-07-18 6:13 ` Shung-Hsi Yu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox