From: Shung-Hsi Yu <shung-hsi.yu@suse.com>
To: Yiyang Chen <chenyy23@mails.tsinghua.edu.cn>
Cc: Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andrii@kernel.org>,
Eduard Zingerman <eddyz87@gmail.com>,
Kumar Kartikeya Dwivedi <memxor@gmail.com>,
John Fastabend <john.fastabend@gmail.com>,
Martin KaFai Lau <martin.lau@linux.dev>,
Song Liu <song@kernel.org>,
Yonghong Song <yonghong.song@linux.dev>,
Jiri Olsa <jolsa@kernel.org>, Shuah Khan <shuah@kernel.org>,
Emil Tsalapatis <emil@etsalapatis.com>,
Ihor Solodrai <ihor.solodrai@linux.dev>,
bpf@vger.kernel.org, linux-kselftest@vger.kernel.org,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH bpf-next v3 1/3] bpf: Preserve pointer state for commuted arithmetic
Date: Thu, 23 Jul 2026 12:25:20 +0800 [thread overview]
Message-ID: <amGUjFu1MN__jM4c@u94a> (raw)
In-Reply-To: <004a83de52a36e9f3acd6c3fa2d0dfd0a013460d.1784696372.git.chenyy23@mails.tsinghua.edu.cn>
On Wed, Jul 22, 2026 at 05:27:31AM +0000, Yiyang Chen wrote:
[...]
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -13726,11 +13726,14 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
> s64 smin_val = reg_smin(off_reg), smax_val = reg_smax(off_reg);
> u64 umin_val = reg_umin(off_reg), umax_val = reg_umax(off_reg);
> struct bpf_sanitize_info info = {};
> + const struct bpf_reg_state *orig_off_reg = off_reg;
> + bool ptr_is_dst_reg;
>
> u8 opcode = BPF_OP(insn->code);
> u32 dst = insn->dst_reg;
> int ret, bounds_ret;
>
> dst_reg = ®s[dst];
> + ptr_is_dst_reg = ptr_reg == dst_reg;
>
> if ((known && (smin_val != smax_val || umin_val != umax_val)) ||
> smin_val > smax_val || umin_val > umax_val) {
[...]
> @@ -13813,7 +13820,7 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
> ret = sanitize_ptr_alu(env, insn, ptr_reg, off_reg, dst_reg,
> &info, false);
> if (ret < 0)
> - return sanitize_err(env, insn, ret, off_reg, dst_reg);
> + return sanitize_err(env, insn, ret, orig_off_reg, dst_reg);
> }
>
> switch (opcode) {
[...]
> @@ -13906,7 +13913,7 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
> return -EFAULT;
> }
> if (ret < 0)
> - return sanitize_err(env, insn, ret, off_reg, dst_reg);
> + return sanitize_err(env, insn, ret, orig_off_reg, dst_reg);
> }
>
> return 0;
The sole purpose of having orig_off_reg seem to be for satisfying the
'off_reg == dst_reg' conditional in sanitize_err(). If that's the case,
maybe it is better to change sanitize_err to accept an ptr_is_dst_reg
argument too.
And given now that we are starting to make more use of scratch register
states, it seems better to get rid the likes of 'off_reg == dst_reg' all
together, and simply have ptr_is_dst_reg passed down by
adjust_scalar_min_max_vals().
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 52be0a118cce..72d49f2a57a3 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -13505,13 +13505,13 @@ static int sanitize_ptr_alu(struct bpf_verifier_env *env,
const struct bpf_reg_state *off_reg,
struct bpf_reg_state *dst_reg,
struct bpf_sanitize_info *info,
- const bool commit_window)
+ const bool commit_window,
+ const bool ptr_is_dst_reg)
{
struct bpf_insn_aux_data *aux = commit_window ? cur_aux(env) : &info->aux;
struct bpf_verifier_state *vstate = env->cur_state;
bool off_is_imm = tnum_is_const(off_reg->var_off);
bool off_is_neg = reg_smin(off_reg) < 0;
- bool ptr_is_dst_reg = ptr_reg == dst_reg;
u8 opcode = BPF_OP(insn->code);
u32 alu_state, alu_limit;
struct bpf_reg_state tmp;
@@ -13611,7 +13611,8 @@ static void sanitize_mark_insn_seen(struct bpf_verifier_env *env)
static int sanitize_err(struct bpf_verifier_env *env,
const struct bpf_insn *insn, int reason,
const struct bpf_reg_state *off_reg,
- const struct bpf_reg_state *dst_reg)
+ const struct bpf_reg_state *dst_reg,
+ const bool ptr_is_dst_reg)
{
static const char *err = "pointer arithmetic with it prohibited for !root";
const char *op = BPF_OP(insn->code) == BPF_ADD ? "add" : "sub";
@@ -13620,11 +13621,11 @@ static int sanitize_err(struct bpf_verifier_env *env,
switch (reason) {
case REASON_BOUNDS:
verbose(env, "R%d has unknown scalar with mixed signed bounds, %s\n",
- off_reg == dst_reg ? dst : src, err);
+ !ptr_is_dst_reg ? dst : src, err);
break;
case REASON_TYPE:
verbose(env, "R%d has pointer with unsupported alu operation, %s\n",
- off_reg == dst_reg ? src : dst, err);
+ !ptr_is_dst_reg ? src : dst, err);
break;
case REASON_PATHS:
verbose(env, "R%d tried to %s from different maps, paths or scalars, %s\n",
@@ -13717,7 +13718,8 @@ static int sanitize_check_bounds(struct bpf_verifier_env *env,
static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
struct bpf_insn *insn,
const struct bpf_reg_state *ptr_reg,
- const struct bpf_reg_state *off_reg)
+ const struct bpf_reg_state *off_reg,
+ const bool ptr_is_dst_reg)
{
struct bpf_verifier_state *vstate = env->cur_state;
struct bpf_func_state *state = vstate->frame[vstate->curframe];
@@ -13811,9 +13813,9 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
if (sanitize_needed(opcode)) {
ret = sanitize_ptr_alu(env, insn, ptr_reg, off_reg, dst_reg,
- &info, false);
+ &info, false, ptr_is_dst_reg);
if (ret < 0)
- return sanitize_err(env, insn, ret, off_reg, dst_reg);
+ return sanitize_err(env, insn, ret, off_reg, dst_reg, ptr_is_dst_reg);
}
switch (opcode) {
@@ -13843,7 +13845,7 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
}
break;
case BPF_SUB:
- if (dst_reg == off_reg) {
+ if (!ptr_is_dst_reg) {
/* scalar -= pointer. Creates an unknown scalar */
verbose(env, "R%d tried to subtract pointer from scalar\n",
dst);
@@ -13897,7 +13899,7 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
return bounds_ret;
if (sanitize_needed(opcode)) {
ret = sanitize_ptr_alu(env, insn, dst_reg, off_reg, dst_reg,
- &info, true);
+ &info, true, ptr_is_dst_reg);
if (verifier_bug_if(!can_skip_alu_sanitation(env, insn)
&& !env->cur_state->speculative
&& bounds_ret
@@ -13906,7 +13908,7 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
return -EFAULT;
}
if (ret < 0)
- return sanitize_err(env, insn, ret, off_reg, dst_reg);
+ return sanitize_err(env, insn, ret, off_reg, dst_reg, ptr_is_dst_reg);
}
return 0;
@@ -14658,7 +14660,7 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
if (sanitize_needed(opcode)) {
ret = sanitize_val_alu(env, insn);
if (ret < 0)
- return sanitize_err(env, insn, ret, NULL, NULL);
+ return sanitize_err(env, insn, ret, NULL, NULL, true /* does it matter here? */);
}
/* Calculate sign/unsigned bounds and tnum for alu32 and alu64 bit ops.
@@ -14862,7 +14864,7 @@ static int adjust_reg_min_max_vals(struct bpf_verifier_env *env,
if (err)
return err;
return adjust_ptr_min_max_vals(env, insn,
- src_reg, dst_reg);
+ src_reg, dst_reg, false);
}
} else if (ptr_reg) {
/* pointer += scalar */
@@ -14870,7 +14872,7 @@ static int adjust_reg_min_max_vals(struct bpf_verifier_env *env,
if (err)
return err;
return adjust_ptr_min_max_vals(env, insn,
- dst_reg, src_reg);
+ dst_reg, src_reg, true);
} else if (dst_reg->precise) {
/* if dst_reg is precise, src_reg should be precise as well */
err = mark_chain_precision(env, insn->src_reg);
@@ -14886,7 +14888,7 @@ static int adjust_reg_min_max_vals(struct bpf_verifier_env *env,
src_reg = &off_reg;
if (ptr_reg) /* pointer += K */
return adjust_ptr_min_max_vals(env, insn,
- ptr_reg, src_reg);
+ ptr_reg, src_reg, true);
}
/* Got here implies adding two SCALAR_VALUEs */
next prev parent reply other threads:[~2026-07-23 4:25 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-22 5:27 [PATCH bpf-next v3 0/3] bpf: Preserve pointer state for commuted arithmetic Yiyang Chen
2026-07-22 5:27 ` [PATCH bpf-next v3 1/3] " Yiyang Chen
2026-07-23 3:45 ` Shung-Hsi Yu
2026-07-23 4:25 ` Shung-Hsi Yu [this message]
2026-07-22 5:27 ` [PATCH bpf-next v3 2/3] bpf: Propagate untrusted pointer state in " Yiyang Chen
2026-07-22 5:27 ` [PATCH bpf-next v3 3/3] selftests/bpf: Cover commuted pointer state propagation Yiyang Chen
2026-07-22 11:38 ` [PATCH bpf-next v3 0/3] bpf: Preserve pointer state for commuted arithmetic Daniel Wade
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=amGUjFu1MN__jM4c@u94a \
--to=shung-hsi.yu@suse.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=chenyy23@mails.tsinghua.edu.cn \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=emil@etsalapatis.com \
--cc=ihor.solodrai@linux.dev \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=martin.lau@linux.dev \
--cc=memxor@gmail.com \
--cc=shuah@kernel.org \
--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