All of lore.kernel.org
 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 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.