BPF List
 help / color / mirror / Atom feed
From: Yonghong Song <yonghong.song@linux.dev>
To: bot+bpf-ci@kernel.org, bpf@vger.kernel.org
Cc: ast@kernel.org, andrii@kernel.org, daniel@iogearbox.net,
	eddyz87@gmail.com, kernel-team@fb.com, martin.lau@kernel.org,
	clm@meta.com, ihor.solodrai@linux.dev
Subject: Re: [PATCH bpf-next v5 04/11] bpf: Track R2 of register-pair returns in precision backtracking
Date: Sun, 16 Aug 2026 20:32:44 -0700	[thread overview]
Message-ID: <d14e1165-609c-4b54-b703-2a182615399c@linux.dev> (raw)
In-Reply-To: <5e43d273b5933f72fb0d45f3dcd67c149a371860391837680739df6555477e77@mail.kernel.org>



On 8/13/26 1:49 PM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/kernel/bpf/backtrack.c b/kernel/bpf/backtrack.c
>> index a2b18a9f1694..0c8e05a7e175 100644
>> --- a/kernel/bpf/backtrack.c
>> +++ b/kernel/bpf/backtrack.c
>> @@ -423,6 +423,16 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
>>   				 */
>>   				verifier_bug_if(idx + 1 != subseq_idx, env,
>>   						"extra insn from subprog");
>> +				/* a global subprog returning more than 8 bytes
>> +				 * sets R2 as well. R2 is part of the args mask
>> +				 * checked just below, so clear it here rather
>> +				 * than next to R0. Only a subprog that does
>> +				 * return a pair defines R2, so leave the mask
>> +				 * alone otherwise and let the check below catch
>> +				 * an R2 that has no business being set.
>> +				 */
>> +				if (bpf_ret_reg_pair(env, subprog))
>> +					bt_clear_reg(bt, BPF_REG_2);
> [ ... ]
>
>> @@ -506,6 +516,17 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
>>   				return -ENOTSUPP;
>>   			/* regular helper call sets R0 */
>>   			bt_clear_reg(bt, BPF_REG_0);
>> +			/* a kfunc returning more than 8 bytes also sets R2.
>> +			 * R2 is part of the args mask checked just below, so
>> +			 * clear it here rather than next to R0. The prototype
>> +			 * lookup is only worth doing when R2 is requested at
>> +			 * all; any other call leaves R2 uninitialized, so a
>> +			 * request for it is caught by the check below.
>> +			 */
>> +			if (bt_is_reg_set(bt, BPF_REG_2) &&
>> +			    insn->src_reg == BPF_PSEUDO_KFUNC_CALL &&
>> +			    bpf_kfunc_ret_reg_pair(env, insn))
>> +				bt_clear_reg(bt, BPF_REG_2);
> [ ... ]
>
>> @@ -520,7 +541,41 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
>>   					return -EFAULT;
>>   			}
>>   		} else if (opcode == BPF_EXIT) {
> [ ... ]
>
>> +			r0_precise = from_subprog_call && bt_is_reg_set(bt, BPF_REG_0);
>> +			r2_precise = false;
>> +			if (from_subprog_call && bt_is_reg_set(bt, BPF_REG_2)) {
>> +				call = &env->prog->insnsi[subseq_idx - 1];
>> +				subprog = bpf_find_subprog(env, subseq_idx + call->imm);
>> +				if (subprog < 0)
>> +					return -EFAULT;
>> +				/* Only a callee that does return a pair defines
>> +				 * R2. Leave the mask alone otherwise, so that
>> +				 * the check below still catches an R2 that has
>> +				 * no business being set.
>> +				 */
>> +				r2_precise = bpf_ret_reg_pair(env, subprog);
>> +			}
> [ ... ]
>
>> @@ -533,30 +588,27 @@ static int backtrack_insn(struct bpf_verifier_env *env, int idx, int subseq_idx,
>>   			if (subseq_idx >= 0 && bpf_calls_callback(env, subseq_idx))
>>   				for (i = BPF_REG_1; i <= BPF_REG_5; i++)
>>   					bt_clear_reg(bt, i);
>> +
>> +			/* a callee returning more than 8 bytes sets R2 as well;
>> +			 * R2 is part of the args mask checked just below, so
>> +			 * clear it here rather than next to R0.
>> +			 */
>> +			if (r2_precise)
>> +				bt_clear_reg(bt, BPF_REG_2);
> This isn't a bug, but the "R2 is part of the args mask checked just
> below, so clear it here rather than next to R0" rationale appears at
> each of the three call boundaries (global subprog, kfunc, and BPF_EXIT).
> Would stating it once - say at the first site, or in a note above the
> BPF_EXIT block - and keeping the other two to a one-liner read more
> easily?

Eduard suggested to remove the condition "r2_precise" for bt_clear_reg(bt, BPF_REG_2)
which should solve the problem.

>
>
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
>
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31740414277


  reply	other threads:[~2026-08-17  3:32 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-13 20:02 [PATCH bpf-next v5 00/11] bpf: Support aggregate return values up to 16 bytes Yonghong Song
2026-08-13 20:02 ` [PATCH bpf-next v5 01/11] bpf: Factor check_global_ret_scalar_reg() out of the global return check Yonghong Song
2026-08-14 22:24   ` Eduard Zingerman
2026-08-13 20:02 ` [PATCH bpf-next v5 02/11] bpf: Add helpers to describe the R0:R2 return register pair Yonghong Song
2026-08-13 21:11   ` bot+bpf-ci
2026-08-17  3:25     ` Yonghong Song
2026-08-14 22:43   ` Eduard Zingerman
2026-08-17  3:28     ` Yonghong Song
2026-08-13 20:02 ` [PATCH bpf-next v5 03/11] bpf: Wire up JIT support for 16-byte kfunc returns Yonghong Song
2026-08-14  1:20   ` sashiko-bot
2026-08-17  3:30     ` Yonghong Song
2026-08-13 20:02 ` [PATCH bpf-next v5 04/11] bpf: Track R2 of register-pair returns in precision backtracking Yonghong Song
2026-08-13 20:49   ` bot+bpf-ci
2026-08-17  3:32     ` Yonghong Song [this message]
2026-08-14 23:37   ` Eduard Zingerman
2026-08-17  3:33     ` Yonghong Song
2026-08-13 20:02 ` [PATCH bpf-next v5 05/11] bpf: Account R2 of register-pair returns in live register analysis Yonghong Song
2026-08-14  1:50   ` sashiko-bot
2026-08-17  3:35     ` Yonghong Song
2026-08-13 20:02 ` [PATCH bpf-next v5 06/11] bpf: Add verifier support for 16-byte returns in R0:R2 Yonghong Song
2026-08-13 21:11   ` bot+bpf-ci
2026-08-17  3:43     ` Yonghong Song
2026-08-14  2:26   ` sashiko-bot
2026-08-17  3:44     ` Yonghong Song
2026-08-14 23:53   ` Eduard Zingerman
2026-08-17  3:44     ` Yonghong Song
2026-08-13 20:02 ` [PATCH bpf-next v5 07/11] bpf: Enable aggregate return types up to 16 bytes Yonghong Song
2026-08-13 20:02 ` [PATCH bpf-next v5 08/11] bpf: Reject register-pair returns when the subprog BTF is unreliable Yonghong Song
2026-08-13 20:49   ` bot+bpf-ci
2026-08-17  3:47     ` Yonghong Song
2026-08-14  3:02   ` sashiko-bot
2026-08-17  3:49     ` Yonghong Song
2026-08-13 20:02 ` [PATCH bpf-next v5 09/11] selftests/bpf: Add C tests for 16-byte returns in R0:R2 Yonghong Song
2026-08-13 21:11   ` bot+bpf-ci
2026-08-17  3:50     ` Yonghong Song
2026-08-15  0:47   ` Eduard Zingerman
2026-08-17  3:53     ` Yonghong Song
2026-08-13 20:03 ` [PATCH bpf-next v5 10/11] selftests/bpf: Add inline-asm and subprog tests for R0:R2 returns Yonghong Song
2026-08-13 21:11   ` bot+bpf-ci
2026-08-17  3:59     ` Yonghong Song
2026-08-15  1:03   ` Eduard Zingerman
2026-08-17  4:00     ` Yonghong Song
2026-08-13 20:03 ` [PATCH bpf-next v5 11/11] Documentation/bpf: Document up to 16-byte kfunc return values in R0:R2 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=d14e1165-609c-4b54-b703-2a182615399c@linux.dev \
    --to=yonghong.song@linux.dev \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bot+bpf-ci@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=clm@meta.com \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=ihor.solodrai@linux.dev \
    --cc=kernel-team@fb.com \
    --cc=martin.lau@kernel.org \
    /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