All of lore.kernel.org
 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,
	kernel-team@fb.com, martin.lau@kernel.org, eddyz87@gmail.com,
	clm@meta.com, ihor.solodrai@linux.dev
Subject: Re: [PATCH bpf-next v2 2/2] selftests/bpf: Add test for stack arg read without caller write
Date: Thu, 14 May 2026 23:31:55 -0700	[thread overview]
Message-ID: <0243b1a5-6a36-4a1a-9d50-e23d45faf1a9@linux.dev> (raw)
In-Reply-To: <eae0af1b3acf0a3f60cbdb86d1b43161f2241f80a07c27c3fae2e929a389e87a@mail.kernel.org>



On 5/15/26 4:31 AM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/tools/testing/selftests/bpf/progs/btf__verifier_stack_arg_order.c b/tools/testing/selftests/bpf/progs/btf__verifier_stack_arg_order.c
>> index da34e8456b6c..99bc115f8380 100644
>> --- a/tools/testing/selftests/bpf/progs/btf__verifier_stack_arg_order.c
>> +++ b/tools/testing/selftests/bpf/progs/btf__verifier_stack_arg_order.c
> [ ... ]
>
>> diff --git a/tools/testing/selftests/bpf/progs/verifier_stack_arg.c b/tools/testing/selftests/bpf/progs/verifier_stack_arg.c
>> index d43a9b42034c..d45339b83795 100644
>> --- a/tools/testing/selftests/bpf/progs/verifier_stack_arg.c
>> +++ b/tools/testing/selftests/bpf/progs/verifier_stack_arg.c
>> @@ -152,7 +152,7 @@ __naked void stack_arg_pruning_type_mismatch(void)
>>   SEC("tc")
>>   __description("stack_arg: release_reference invalidates stack arg slot")
>>   __failure
>> -__msg("R{{[0-9]}} !read_ok")
>> +__msg("callee expects 6 args, stack arg1 is not initialized")
>>   __naked void stack_arg_release_ref(void)
> [ ... ]
>
>>   SEC("tc")
>>   __description("stack_arg: pkt pointer in stack arg slot invalidated after pull_data")
>>   __failure
>> -__msg("R{{[0-9]}} !read_ok")
>> +__msg("callee expects 6 args, stack arg1 is not initialized")
>>   __naked void stack_arg_stale_pkt_ptr(void)
> [ ... ]
>
>> diff --git a/tools/testing/selftests/bpf/progs/verifier_stack_arg_order.c b/tools/testing/selftests/bpf/progs/verifier_stack_arg_order.c
>> index 1240cf8a40d6..c9fe4857da3f 100644
>> --- a/tools/testing/selftests/bpf/progs/verifier_stack_arg_order.c
>> +++ b/tools/testing/selftests/bpf/progs/verifier_stack_arg_order.c
>> @@ -112,6 +112,64 @@ __naked void stack_arg_pruning_load_after_call(void)
>>   	);
>>   }
>>
>> +/*
>> + * "bad_ptr": the first arg is 'long *', which is not a recognized pointer
>> + * type for static subprogs (not ctx, dynptr, or tagged).  btf_prepare_func_args()
>> + * sets arg_cnt = 7 / stack_arg_cnt = 2, then fails with -EINVAL.  The subprog
>> + * is marked unreliable but the call still proceeds for static subprogs.
>> + */
>> +__noinline __used __naked
>> +static void subprog_bad_ptr_7args(long *a, int b, int c, int d, int e, int f, int g)
>> +{
>> +	asm volatile (
>> +		"r0 = *(u64 *)(r11 + 8);"
>> +		"r1 = *(u64 *)(r11 + 16);"
>> +		"exit;"
>> +		::: __clobber_all
>> +	);
>> +}
>> +
>> +SEC("tc")
>> +__description("stack_arg: read without caller write")
>> +__failure
>> +__msg("callee expects 7 args, stack arg1 is not initialized")
>> +__btf_func_path("btf__verifier_stack_arg_order.bpf.o")
>> +__naked void stack_arg_read_without_write_1(void)
>> +{
>> +	asm volatile (
>> +		"r1 = 0;"
>> +		"r2 = 0;"
>> +		"r3 = 0;"
>> +		"r4 = 0;"
>> +		"r5 = 0;"
>> +		"call subprog_bad_ptr_7args;"
>> +		"exit;"
>> +		::: __clobber_all
>> +	);
>> +}
>> +
>> +SEC("tc")
>> +__description("stack_arg: read with not-initialized caller write")
>> +__failure
>> +__msg("R0 !read_ok")
> Is the expected error message correct here? Looking at
> stack_arg_read_without_write_1 (the test just above), it expects the
> new-style message "callee expects 7 args, stack arg1 is not
> initialized" when calling subprog_bad_ptr_7args without writing to the
> stack. This test (stack_arg_read_without_write_2) also calls
> subprog_bad_ptr_7args without properly initializing the stack args, but
> expects the old-style message "R0 !read_ok" instead.
>
> The commit message explains that invalidated stack arg slots are now
> caught by check_outgoing_stack_args() at the call site instead of at
> the callee's dereference, which led to updating the expected messages
> in stack_arg_release_ref and stack_arg_stale_pkt_ptr from "R0
> !read_ok" to "callee expects 6 args, stack arg1 is not initialized".
>
> This test writes to r11-8 and r11-16 (stack args for the 6th and 7th
> parameters), then calls subprog_bad_ptr_7args twice without rewriting
> the stack between calls. The pattern matches stack_arg_sequential_calls
> in verifier_stack_arg.c which expects the new-style error message.
>
> Should this test expect "callee expects 7 args, stack arg1 is not
> initialized" to be consistent with the other tests and the commit's
> stated behavior change?

I think it should be okay. The first subprog_bad_ptr_7args()
will go through arg checking in btf_check_func_arg_match().

But the second call subprog_bad_ptr_7args() will not go through
btf_check_func_arg_match() since the subprog btf_id has been
calculated in the first subprog_bad_ptr_7args() and the verifier
simply reuse the btf_id. Due to this, the validation
check_outgoing_stack_args() inside btf_check_func_arg_match()
is not triggered.

Since there is no intermediate r11-based store, the callee
gets a 'not initialized' value for r0 and eventually gets
an error message 'R0 !read_ok' indicating the return value
is not valid.

>
>> +__btf_func_path("btf__verifier_stack_arg_order.bpf.o")
>> +__naked void stack_arg_read_without_write_2(void)
>> +{
>> +	asm volatile (
>> +		"r1 = 0;"
>> +		"r2 = 0;"
>> +		"r3 = 0;"
>> +		"r4 = 0;"
>> +		"r5 = 0;"
>> +		"*(u64 *)(r11 - 8) = 0;"
>> +		"*(u64 *)(r11 - 16) = 0;"
>> +		"call subprog_bad_ptr_7args;"
>> +		"call subprog_bad_ptr_7args;"
>> +		"exit;"
>> +		::: __clobber_all
>> +	);
>> +}
>
> ---
> 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/25896151874


      reply	other threads:[~2026-05-15  6:32 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-15  1:49 [PATCH bpf-next v2 1/2] bpf: Validate outgoing stack args when btf_prepare_func_args fails Yonghong Song
2026-05-15  1:50 ` [PATCH bpf-next v2 2/2] selftests/bpf: Add test for stack arg read without caller write Yonghong Song
2026-05-15  2:31   ` bot+bpf-ci
2026-05-15  6:31     ` Yonghong Song [this message]

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=0243b1a5-6a36-4a1a-9d50-e23d45faf1a9@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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.