bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Yonghong Song <yonghong.song@linux.dev>
To: Paul Chaignon <paul.chaignon@gmail.com>, bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
	Daniel Borkmann <daniel@iogearbox.net>,
	Andrii Nakryiko <andrii@kernel.org>,
	Eduard Zingerman <eddyz87@gmail.com>
Subject: Re: [PATCH bpf-next 1/2] bpf: Forget ranges when refining tnum after JSET
Date: Wed, 9 Jul 2025 16:57:28 -0700	[thread overview]
Message-ID: <ba1b52e3-a938-4ead-943c-267e4c06b1ae@linux.dev> (raw)
In-Reply-To: <75b3af3d315d60c1c5bfc8e3929ac69bb57d5cea.1752099022.git.paul.chaignon@gmail.com>



On 7/9/25 3:26 PM, Paul Chaignon wrote:
> Syzbot reported a kernel warning due to a range invariant violation on
> the following BPF program.
>
>    0: call bpf_get_netns_cookie
>    1: if r0 == 0 goto <exit>
>    2: if r0 & Oxffffffff goto <exit>
>
> The issue is on the path where we fall through both jumps.
>
> That path is unreachable at runtime: after insn 1, we know r0 != 0, but
> with the sign extension on the jset, we would only fallthrough insn 2
> if r0 == 0. Unfortunately, is_branch_taken() isn't currently able to
> figure this out, so the verifier walks all branches. The verifier then
> refines the register bounds using the second condition and we end
> up with inconsistent bounds on this unreachable path:
>
>    1: if r0 == 0 goto <exit>
>      r0: u64=[0x1, 0xffffffffffffffff] var_off=(0, 0xffffffffffffffff)
>    2: if r0 & 0xffffffff goto <exit>
>      r0 before reg_bounds_sync: u64=[0x1, 0xffffffffffffffff] var_off=(0, 0)
>      r0 after reg_bounds_sync:  u64=[0x1, 0] var_off=(0, 0)
>
> Improving the range refinement for JSET to cover all cases is tricky. We
> also don't expect many users to rely on JSET given LLVM doesn't generate
> those instructions. So instead of reducing false positives due to JSETs,
> Eduard suggested we forget the ranges whenever we're narrowing tnums
> after a JSET. This patch implements that approach.
>
> Reported-by: syzbot+c711ce17dd78e5d4fdcf@syzkaller.appspotmail.com
> Suggested-by: Eduard Zingerman <eddyz87@gmail.com>
> Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> ---
>   kernel/bpf/verifier.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 53007182b46b..e2fcea860755 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -16208,6 +16208,10 @@ static void regs_refine_cond_op(struct bpf_reg_state *reg1, struct bpf_reg_state
>   		if (!is_reg_const(reg2, is_jmp32))
>   			break;
>   		val = reg_const_value(reg2, is_jmp32);
> +		/* Forget the ranges before narrowing tnums, to avoid invariant
> +		 * violations if we're on a dead branch.
> +		 */
> +		__mark_reg_unbounded(reg1);
>   		if (is_jmp32) {
>   			t = tnum_and(tnum_subreg(reg1->var_off), tnum_const(~val));
>   			reg1->var_off = tnum_with_subreg(reg1->var_off, t);

The CI reports some invariant violation:
   https://github.com/kernel-patches/bpf/actions/runs/16182458904/job/45681940946?pr=9283

[ 283.030177] ------------[ cut here ]------------ [ 283.030517] 
verifier bug: REG INVARIANTS VIOLATION (false_reg2): range bounds 
violation u64=[0x8000000000000010, 0x800000000000000f] 
s64=[0x8000000000000010, 0x800000000000000f] u32=[0x10, 0xf] s32=[0x10, 
0xf] var_off=(0x8000000000000000, 0x1f)(1)
[ 283.032139] WARNING: CPU: 0 PID: 103 at kernel/bpf/verifier.c:2689 
reg_bounds_sanity_check+0x1dd/0x1f0 ... Probably this change triggered 
some other violations. Please take a look.


  parent reply	other threads:[~2025-07-09 23:57 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-07-09 22:26 [PATCH bpf-next 1/2] bpf: Forget ranges when refining tnum after JSET Paul Chaignon
2025-07-09 22:27 ` [PATCH bpf-next 2/2] selftests/bpf: Range analysis test case for JSET Paul Chaignon
2025-07-09 23:09   ` Yonghong Song
2025-07-10 14:38     ` Paul Chaignon
2025-07-09 23:26 ` [PATCH bpf-next 1/2] bpf: Forget ranges when refining tnum after JSET Eduard Zingerman
2025-07-09 23:57 ` Yonghong Song [this message]
2025-07-10 14:51   ` Paul Chaignon
2025-07-10 17:09     ` Yonghong Song

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ba1b52e3-a938-4ead-943c-267e4c06b1ae@linux.dev \
    --to=yonghong.song@linux.dev \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=paul.chaignon@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).