All of lore.kernel.org
 help / color / mirror / Atom feed
From: <emil@etsalapatis.com>
To: "Yazhou Tang" <tangyazhou@zju.edu.cn>, <bpf@vger.kernel.org>
Cc: <ast@kernel.org>, <daniel@iogearbox.net>,
	<john.fastabend@gmail.com>, <andrii@kernel.org>,
	<martin.lau@linux.dev>, <eddyz87@gmail.com>, <song@kernel.org>,
	<yonghong.song@linux.dev>, <kpsingh@kernel.org>,
	<sdf@fomichev.me>, <haoluo@google.com>, <jolsa@kernel.org>,
	<tangyazhou518@outlook.com>, <shenghaoyuan0928@163.com>,
	<ziye@zju.edu.cn>
Subject: Re: [PATCH bpf 2/2] selftests/bpf: Add test for large offset bpf-to-bpf call
Date: Mon, 16 Mar 2026 16:18:52 -0400	[thread overview]
Message-ID: <DH4HGGKLQ2EG.33P3WH3OF05RX@etsalapatis.com> (raw)
In-Reply-To: <20260316190220.113417-3-tangyazhou@zju.edu.cn>

On Mon Mar 16, 2026 at 3:02 PM EDT, Yazhou Tang wrote:
> From: Yazhou Tang <tangyazhou518@outlook.com>
>
> The test utilizes an inline assembly block with `.rept 200000` to generate
> a massive dummy subprogram. By placing this padding between the main program
> and the target subprogram, it forces the verifier to process a bpf-to-bpf call
> with the `imm` field exceeding the s16 range.
>
> The user-space test driver dynamically adapts to the host's JIT configuration:
> - When JIT is enabled, it asserts that the program is successfully loaded,
>   JIT-compiled, and executes correctly (returning the expected value).
> - When JIT is disabled, it asserts that the program is cleanly rejected by
>   the verifier with -EINVAL (or -ENOSPC if the verifier log buffer is truncated),
>   and strictly matches the expected error log.
>
> Co-developed-by: Tianci Cao <ziye@zju.edu.cn>
> Signed-off-by: Tianci Cao <ziye@zju.edu.cn>
> Co-developed-by: Shenghao Yuan <shenghaoyuan0928@163.com>
> Signed-off-by: Shenghao Yuan <shenghaoyuan0928@163.com>
> Signed-off-by: Yazhou Tang <tangyazhou518@outlook.com>
> ---
>  .../selftests/bpf/prog_tests/call_large_imm.c | 49 +++++++++++++++++++
>  .../selftests/bpf/progs/call_large_imm.c      | 38 ++++++++++++++
>  2 files changed, 87 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/call_large_imm.c
>  create mode 100644 tools/testing/selftests/bpf/progs/call_large_imm.c
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/call_large_imm.c b/tools/testing/selftests/bpf/prog_tests/call_large_imm.c
> new file mode 100644
> index 000000000000..ba4a2e27fa5d
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/call_large_imm.c
> @@ -0,0 +1,49 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <test_progs.h>
> +#include "call_large_imm.skel.h"
> +
> +void test_call_large_imm(void)
> +{
> +	struct call_large_imm *skel;
> +	int err, prog_fd;
> +	char log_buf[4096] = {};

No need to init the log_buf, it will be NUL-terminated if load runs.

> +	char dummy_data[64] = {};

Nit: Why define dummy_data? You can make the function you're calling an
int (void) syscall to avoid this.

> +
> +	LIBBPF_OPTS(bpf_test_run_opts, opts,
> +		.data_in = dummy_data,
> +		.data_size_in = sizeof(dummy_data),
> +	);
> +
> +	skel = call_large_imm__open();
> +	if (!ASSERT_OK_PTR(skel, "skel_open"))
> +		return;
> +
> +	if (!env.jit_enabled)
> +		bpf_program__set_log_buf(skel->progs.call_large_imm_test,
> +					 log_buf, sizeof(log_buf));
> +
> +	err = call_large_imm__load(skel);
> +
> +	if (env.jit_enabled) {

So we only test one path at a time, correct? I understand there's no
good way to turn JIT compilation on and off, but as it stands

> +		if (!ASSERT_OK(err, "load_should_succeed_with_jit"))
> +			goto cleanup;
> +
> +		prog_fd = bpf_program__fd(skel->progs.call_large_imm_test);
> +		err = bpf_prog_test_run_opts(prog_fd, &opts);
> +
> +		if (ASSERT_OK(err, "prog_run_success"))
> +			ASSERT_EQ(opts.retval, 3, "prog_retval");
> +
> +	} else {
> +		ASSERT_ERR(err, "load_should_fail_in_interpreter");
> +		ASSERT_TRUE(err == -EINVAL || err == -ENOSPC, "err_should_be_einval_or_enospc");
> +
> +		if (!ASSERT_OK_PTR(strstr(log_buf, "bpf-to-bpf call offset out of range for interpreter"),
> +				   "check_verifier_log_msg")) {
> +			printf("Actual verifier log:\n%s\n", log_buf);
> +		}
> +	}
> +
> +cleanup:
> +	call_large_imm__destroy(skel);
> +}
> diff --git a/tools/testing/selftests/bpf/progs/call_large_imm.c b/tools/testing/selftests/bpf/progs/call_large_imm.c
> new file mode 100644
> index 000000000000..d0057f88b48b
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/call_large_imm.c
> @@ -0,0 +1,38 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#include <linux/bpf.h>
> +#include <bpf/bpf_helpers.h>
> +
> +struct __sk_buff;

Can be removed if you use a syscall prog instead of a socket.

> +
> +static __attribute__((noinline)) void padding_subprog(void)
> +{
> +	asm volatile ("					\
> +		r0 = 0;					\
> +		.rept 200000;				\
> +		r0 += 0;				\
> +		.endr;					\
> +	" ::: "r0");
> +}
> +
> +static __attribute__((noinline)) int target_subprog(struct __sk_buff *ctx)
> +{
> +	volatile int magic_ret = 3;
> +	return magic_ret;


If we remove the printk below, we can just leave this empty or have it
return 3 directly. Is there a reason to define the constant we're
returning indirectly? If there is there should be a comment about it
because right now it's not readily apparent.

> +}
> +
> +SEC("socket")
> +int call_large_imm_test(struct __sk_buff *ctx)
> +{
> +	int ret = 0;
> +
> +	if (ctx == (void *)0)

Nit: Would if (!ctx) work? What is this condition's purpose in the first
place? If it's deliberate, again can we add a comment?

> +		padding_subprog();
> +
> +	et = target_subprog(ctx);
> +
> +	bpf_printk("Target subprog returned: %d\n", ret);

Let's remove this, there's no reason to emit to the trace pipe even on
success.

> +
> +	return ret;
> +}
> +
> +char LICENSE[] SEC("license") = "GPL";


  reply	other threads:[~2026-03-16 20:18 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-16 19:02 [PATCH bpf 0/2] bpf: reject bpf-to-bpf call with large offset in interpreter Yazhou Tang
2026-03-16 19:02 ` [PATCH bpf 1/2] " Yazhou Tang
2026-03-16 19:33   ` bot+bpf-ci
2026-03-16 20:32   ` Emil Tsalapatis
2026-03-17  3:18     ` Yazhou Tang
2026-03-16 20:45   ` Puranjay Mohan
2026-03-17  3:27     ` Yazhou Tang
2026-03-16 19:02 ` [PATCH bpf 2/2] selftests/bpf: Add test for large offset bpf-to-bpf call Yazhou Tang
2026-03-16 20:18   ` emil [this message]
2026-03-17  5:32     ` Yazhou Tang

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=DH4HGGKLQ2EG.33P3WH3OF05RX@etsalapatis.com \
    --to=emil@etsalapatis.com \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=haoluo@google.com \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=sdf@fomichev.me \
    --cc=shenghaoyuan0928@163.com \
    --cc=song@kernel.org \
    --cc=tangyazhou518@outlook.com \
    --cc=tangyazhou@zju.edu.cn \
    --cc=yonghong.song@linux.dev \
    --cc=ziye@zju.edu.cn \
    /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.