Re: [PATCH bpf v2 2/2] selftests/bpf: Exercise negative optlen in cgroup getsockopt hook

[bot+bpf-ci@kernel.org]

[-- Attachment #1: Type: text/plain, Size: 3352 bytes --]

> diff --git a/tools/testing/selftests/bpf/prog_tests/sockopt.c b/tools/testing/selftests/bpf/prog_tests/sockopt.c
> index eaac83a7f388..6c96f2d9fccf 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockopt.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockopt.c

[ ... ]

> @@ -283,6 +284,27 @@ static struct sockopt_test {
>  		.error = EFAULT_GETSOCKOPT,
>  		.io_uring_support = true,
>  	},
> +	{
> +		.descr = "getsockopt: deny negative ctx->optlen in TCP_ZEROCOPY_RECEIVE",
> +		.insns = {
> +			/* ctx->optlen = -1 */
> +			BPF_MOV64_IMM(BPF_REG_0, -1),
> +			BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_0,
> +				    offsetof(struct bpf_sockopt, optlen)),
> +
> +			/* return 1 */
> +			BPF_MOV64_IMM(BPF_REG_0, 1),
> +			BPF_EXIT_INSN(),
> +		},
> +		.attach_type = BPF_CGROUP_GETSOCKOPT,
> +		.expected_attach_type = BPF_CGROUP_GETSOCKOPT,
> +
> +		.get_level = IPPROTO_TCP,
> +		.get_optname = TCP_ZEROCOPY_RECEIVE,
> +		.get_optlen = sizeof(struct tcp_zerocopy_receive),
> +
> +		.error = EFAULT_GETSOCKOPT,
> +	},
>  	{
>  		.descr = "getsockopt: ignore >PAGE_SIZE optlen",
>  		.insns = {

A subsystem pattern flags this as potentially concerning: the test asserts
-EFAULT for negative ctx->optlen, but both a fixed kernel and an unfixed
kernel can return -EFAULT, so does the test provide regression protection?

The commit message notes this honestly: "the return value alone cannot
distinguish the bug." The test exercises the kernel-buffer getsockopt path
with ctx->optlen == -1, which triggers a hardened-usercopy WARN on
unpatched kernels.

Tracing it through:

  run_test() creates socket(AF_INET, SOCK_STREAM, 0), state is TCP_CLOSE
  and tcp_inq(sk) == 0.

  do_tcp_getsockopt() accepts len (40) and calls tcp_zerocopy_receive().
  With inq == 0 < PAGE_SIZE and SOCK_DONE unset:

    zc->length = 0;
    zc->recv_skip_hint = inq;
    return 0;

  so err == 0 going into the BPF hook.

  Without the fix: ctx.optlen becomes -1, back in tcp.c, switch (len) with
  len == -1 falls to default: goto zerocopy_rcv_out. The if (!err &&
  copy_to_sockptr(optval, &zc, len)) runs with size (size_t)-1.
  check_copy_size() WARNs and returns false, copy_to_user() returns n != 0,
  so err = -EFAULT.

  With the fix: ctx.optlen < 0 returns -EFAULT directly, all the !err
  guards skip the copy, and -EFAULT is returned.

Userspace sees errno == EFAULT in both cases; run_test() returns 0 (PASS)
either way. The only difference between kernels is a WARN in dmesg, and
test_progs has no dmesg/taint/WARN detection (no references to dmesg,
kmsg, tainted, or panic_on_warn in test_progs.c/.h).

Strengthening it by making the handler fail first doesn't work either,
because __cgroup_bpf_run_filter_getsockopt_kern() does if (ret < 0) return
ret; before applying ctx.optlen, so a non-zero retval short-circuits the
buggy assignment. The buggy copy is only reachable when err == 0, and tcp.c
always converts it into EFAULT.

Is the intent that this test relies on WARN/taint detection or
panic_on_warn, or is the value of the test primarily as a WARN-only
reproducer rather than a pass/fail assertion?


---
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/31501873750