BPF List
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Sun Jian" <sun.jian.kdev@gmail.com>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH bpf v4] selftests/bpf: Cover partial copy of non-linear test_run output
Date: Tue, 23 Jun 2026 01:53:24 +0000	[thread overview]
Message-ID: <20260623015325.2F2D31F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260623014027.402820-1-sun.jian.kdev@gmail.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The newly added selftests `test_skb_nonlinear_data_out_partial` and `test_xdp_nonlinear_data_out_partial` will unconditionally fail due to an unaddressed arithmetic bug in the kernel's `bpf_test_finish` function, breaking the selftest suite.
--

commit 99284be8e441ba5ead251137597e96ebcee998f4
Author: Sun Jian <sun.jian.kdev@gmail.com>

selftests/bpf: Cover partial copy of non-linear test_run output

This commit adds selftests to cover the partial copy of non-linear test_run
output for both TC and XDP programs. It uses a 9000-byte packet with a 64-byte
linear data area and a 100-byte output buffer.

> diff --git a/tools/testing/selftests/bpf/prog_tests/prog_run_opts.c b/tools/testing/selftests/bpf/prog_tests/prog_run_opts.c
> index 01f1d1b6715a6..beb6fa78fd949 100644
> --- a/tools/testing/selftests/bpf/prog_tests/prog_run_opts.c
> +++ b/tools/testing/selftests/bpf/prog_tests/prog_run_opts.c

[ ... ]

> @@ -20,6 +24,69 @@ static void check_run_cnt(int prog_fd, __u64 run_cnt)
>  	      "incorrect number of repetitions, want %llu have %llu\n", run_cnt, info.run_cnt);
>  }
>  
> +static void init_pkt(__u8 *pkt, size_t len)
> +{
> +	size_t i;
> +
> +	for (i = 0; i < len; i++)
> +		pkt[i] = i & 0xff;
> +}
> +
> +static void test_skb_nonlinear_data_out_partial(struct test_pkt_access *skel)
> +{
> +	LIBBPF_OPTS(bpf_test_run_opts, topts);
> +	__u8 pkt[NONLINEAR_PKT_LEN];
> +	__u8 out[SHORT_OUT_LEN] = {};
> +	struct __sk_buff skb = {};
> +	int prog_fd, err;
> +
> +	init_pkt(pkt, sizeof(pkt));
> +
> +	skb.data_end = NONLINEAR_LINEAR_DATA_LEN;
> +
> +	topts.data_in = pkt;
> +	topts.data_size_in = sizeof(pkt);
> +	topts.data_out = out;
> +	topts.data_size_out = sizeof(out);
> +	topts.ctx_in = &skb;
> +	topts.ctx_size_in = sizeof(skb);
> +
> +	prog_fd = bpf_program__fd(skel->progs.tc_pass_prog);
> +	err = bpf_prog_test_run_opts(prog_fd, &topts);
> +
> +	ASSERT_EQ(err, -ENOSPC, "skb_partial_err");
> +	ASSERT_EQ(topts.data_size_out, sizeof(pkt), "skb_partial_size");
> +	ASSERT_OK(memcmp(out, pkt, sizeof(out)), "skb_partial_data");

[Severity: High]
Will these assertions fail unconditionally due to an existing arithmetic
bug in the kernel?

In net/bpf/test_run.c:bpf_test_finish(), the length is calculated as:

    int len = sinfo ? copy_size - frag_size : copy_size;

    if (len < 0) {
        err = -ENOSPC;
        goto out;
    }

Since the packet length is 9000 bytes with a 64 byte linear area, frag_size
is 8936. When copy_size is 100, this yields a negative length of -8836.

This triggers an early error exit, completely bypassing both the partial
data copy to the userspace out buffer and the update of data_size_out.

> +}

[ ... ]

> +static void test_xdp_nonlinear_data_out_partial(struct test_pkt_access *skel)
> +{
> +	LIBBPF_OPTS(bpf_test_run_opts, topts);
> +	__u8 pkt[NONLINEAR_PKT_LEN];
> +	__u8 out[SHORT_OUT_LEN] = {};
> +	struct xdp_md ctx = {};
> +	int prog_fd, err;
> +
> +	init_pkt(pkt, sizeof(pkt));
> +
> +	ctx.data = 0;
> +	ctx.data_end = NONLINEAR_LINEAR_DATA_LEN;
> +
> +	topts.data_in = pkt;
> +	topts.data_size_in = sizeof(pkt);
> +	topts.data_out = out;
> +	topts.data_size_out = sizeof(out);
> +	topts.ctx_in = &ctx;
> +	topts.ctx_size_in = sizeof(ctx);
> +
> +	prog_fd = bpf_program__fd(skel->progs.xdp_frags_pass_prog);
> +	err = bpf_prog_test_run_opts(prog_fd, &topts);
> +
> +	ASSERT_EQ(err, -ENOSPC, "xdp_partial_err");
> +	ASSERT_EQ(topts.data_size_out, sizeof(pkt), "xdp_partial_size");
> +	ASSERT_OK(memcmp(out, pkt, sizeof(out)), "xdp_partial_data");

[Severity: High]
Will this XDP test also unconditionally fail for the same reason?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260623014027.402820-1-sun.jian.kdev@gmail.com?part=1

  reply	other threads:[~2026-06-23  1:53 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-23  1:40 [PATCH bpf v4] selftests/bpf: Cover partial copy of non-linear test_run output Sun Jian
2026-06-23  1:53 ` sashiko-bot [this message]
2026-06-23  2:03   ` sun jian

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=20260623015325.2F2D31F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=sun.jian.kdev@gmail.com \
    /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