mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Eduard Zingerman <eddyz87@gmail.com>
To: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>,
	ast@kernel.org,  daniel@iogearbox.net, andrii@kernel.org
Cc: John Fastabend <john.fastabend@gmail.com>,
	Martin KaFai Lau	 <martin.lau@linux.dev>,
	Kumar Kartikeya Dwivedi <memxor@gmail.com>,
	Song Liu	 <song@kernel.org>,
	Yonghong Song <yonghong.song@linux.dev>,
	Jiri Olsa	 <jolsa@kernel.org>,
	bpf@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH bpf-next v2 1/2] bpf, verifier: fold reg->var_off into PTR_TO_FLOW_KEYS bounds check
Date: Thu, 04 Jun 2026 19:22:03 -0700	[thread overview]
Message-ID: <f0163609ba2a533407a53ab4c754c559780d26f6.camel@gmail.com> (raw)
In-Reply-To: <20260604180730.2518088-2-gnq25@mails.tsinghua.edu.cn>

On Fri, 2026-06-05 at 02:07 +0800, Nuoqi Gui wrote:
> Constant pointer arithmetic on a PTR_TO_FLOW_KEYS register lands the
> constant in reg->var_off (e.g. flow_keys(imm=4096)), but the
> PTR_TO_FLOW_KEYS path in check_mem_access() passes only insn->off to
> check_flow_keys_access() and never folds reg->var_off.value.  The
> verifier therefore accepts an access that, at runtime, dereferences past
> struct bpf_flow_keys -- a verifier/runtime divergence that yields an
> out-of-bounds read and write of kernel stack memory.
> 
> Commit 022ac0750883 ("bpf: use reg->var_off instead of reg->off for
> pointers") removed the generic "off += reg->off" that check_mem_access()
> applied before the per-type dispatch and replaced it with per-path
> folding of reg->var_off.value (for example the ctx path now folds the
> register offset via check_ctx_access()).  The PTR_TO_FLOW_KEYS path was
> not given the equivalent fold, so a constant offset that used to be
> folded and rejected is now silently accepted:
> 
>   before 022ac0750883: the offset stays in reg->off and is folded
>     generically, so the access is checked with off=4096 and rejected.
>   after  022ac0750883: the offset lands in reg->var_off, the flow_keys
>     path checks off=0 and accepts; at runtime the access dereferences
>     base + 0x1000.
> 
> For a BPF_PROG_TYPE_FLOW_DISSECTOR program the following is accepted:
> 
>   r2 = *(u64 *)(r1 + 144)   ; R2=flow_keys (PTR_TO_FLOW_KEYS)
>   r2 += 0x1000              ; R2=flow_keys(imm=4096), accepted
>   r0 = *(u64 *)(r2 + 0)     ; accepted, var_off.value=0x1000 ignored
> 
> while the equivalent insn->off form
> 
>   r0 = *(u64 *)(r2 + 0x1000)
> 
> has the same effective offset but is correctly rejected with
> "invalid access to flow keys off=4096 size=8", which isolates the defect
> to the missing var_off fold.  Once attached as a flow dissector, the
> accepted program reads kernel stack past struct bpf_flow_keys (a
> kernel-stack / KASLR information leak) and can likewise write past it,
> corrupting kernel memory.
> 
> Fix it by folding reg->var_off.value into the offset before the bounds
> check and rejecting non-constant offsets, mirroring the other pointer
> types (e.g. check_ctx_access()).
> 
> No released kernel is affected; the regression is confined to the 7.1
> development cycle (reproduced on v7.1-rc1..rc5), and v7.0.x rejects the
> program above.
> 
> Fixes: 022ac0750883 ("bpf: use reg->var_off instead of reg->off for pointers")
> Signed-off-by: Nuoqi Gui <gnq25@mails.tsinghua.edu.cn>
> ---

This fixes a real issue, thank you for finding it.

>  kernel/bpf/verifier.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 8ed484cb1a8a..c04941636ef4 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -4728,9 +4728,22 @@ static int check_ctx_access(struct bpf_verifier_env *env, int insn_idx, struct b
>  	return err;
>  }
>  
> -static int check_flow_keys_access(struct bpf_verifier_env *env, int off,
> -				  int size)
> +static int check_flow_keys_access(struct bpf_verifier_env *env, u32 regno,
> +				  int off, int size)
>  {
> +	struct bpf_reg_state *reg = reg_state(env, regno);
> +
> +	/* Only a constant offset is allowed here; fold it into off. */
> +	if (!tnum_is_const(reg->var_off)) {
> +		char tn_buf[48];
> +
> +		tnum_strn(tn_buf, sizeof(tn_buf), reg->var_off);
> +		verbose(env, "R%d invalid variable offset to flow keys: off=%d, var_off=%s\n",
> +			regno, off, tn_buf);
> +		return -EACCES;
> +	}
> +	off += reg->var_off.value;
> +
>  	if (size < 0 || off < 0 ||
>  	    (u64)off + size > sizeof(struct bpf_flow_keys)) {
>  		verbose(env, "invalid access to flow keys off=%d size=%d\n",
> @@ -6239,7 +6252,7 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, struct b
>  			return -EACCES;
>  		}
>  
> -		err = check_flow_keys_access(env, off, size);
> +		err = check_flow_keys_access(env, regno, off, size);
                                                  ^^^^^
This variable is not defined, hence this patch leads to the compilation error.

>  		if (!err && t == BPF_READ && value_regno >= 0)
>  			mark_reg_unknown(env, regs, value_regno);
>  	} else if (type_is_sk_pointer(reg->type)) {

  parent reply	other threads:[~2026-06-05  2:22 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20260604180730.2518088-1-gnq25@mails.tsinghua.edu.cn>
2026-06-04 18:07 ` Nuoqi Gui
2026-06-04 18:45   ` bot+bpf-ci
2026-06-05  2:22   ` Eduard Zingerman [this message]
2026-06-05  4:36     ` gnq25
2026-06-04 18:07 ` [PATCH bpf-next v2 2/2] selftests/bpf: add tests for PTR_TO_FLOW_KEYS constant offset bounds Nuoqi Gui
2026-06-04 18:45   ` bot+bpf-ci
2026-06-05  2:23   ` Eduard Zingerman
2026-06-05  4:39     ` gnq25

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=f0163609ba2a533407a53ab4c754c559780d26f6.camel@gmail.com \
    --to=eddyz87@gmail.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=gnq25@mails.tsinghua.edu.cn \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=memxor@gmail.com \
    --cc=song@kernel.org \
    --cc=yonghong.song@linux.dev \
    /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

all inboxes | Powered by JetHome®