From: sashiko-bot@kernel.org
To: "Emil Tsalapatis" <emil@etsalapatis.com>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH bpf v2 04/11] selftests/bpf: Add selftests for rx_queue_mapping context access
Date: Tue, 22 Sep 2026 17:36:11 +0000 [thread overview]
Message-ID: <20260922173612.4B7191F000FF@smtp.kernel.org> (raw)
In-Reply-To: <20260922172028.6269-5-emil@etsalapatis.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: selftests fail to verify bounds tracking of rx_queue_mapping
--
commit 90d4583595dcbfcce26a9758d799968a757439e5
Author: Emil Tsalapatis <emil@etsalapatis.com>
selftests/bpf: Add selftests for rx_queue_mapping context access
This commit adds selftests for the rx_queue_mapping field read from
struct sock to ensure the verifier properly tracks its 0 bit state and
width.
> diff --git a/tools/testing/selftests/bpf/progs/verifier_sock.c b/tools/testing/selftests/bpf/progs/verifier_sock.c
> index 2a136c917680f..565c4d8bc3fa3 100644
> --- a/tools/testing/selftests/bpf/progs/verifier_sock.c
> +++ b/tools/testing/selftests/bpf/progs/verifier_sock.c
[ ... ]
> +SEC("socket")
> +__description("skb->sk: sk->rx_queue_mapping [no sign extension]")
> +__success __success_unpriv __retval(0)
> +__naked void sk_rx_queue_mapping_no_sign_ext(void)
> +{
> + asm volatile (" \
> + r1 = *(u64*)(r1 + %[__sk_buff_sk]); \
> + if r1 != 0 goto l0_%=; \
> + r0 = 0xdead; \
> + exit; \
> +l0_%=: r0 = *(u32*)(r1 + %[bpf_sock_rx_queue_mapping]); \
> + r0 >>= 32; \
> + exit; \
[Severity: Medium]
Does this sequence actually verify the verifier's 0-bit state tracking
of rx_queue_mapping?
The program shifts the loaded value right by 32 and exits. The verifier
always accepts returning an unknown initialized scalar in this context. At
runtime, the CPU zero-extends the value, so it always evaluates to 0 and
passes the __retval(0) check, regardless of what the verifier's static
analysis concluded about the bounds.
> +" :
> + : __imm_const(__sk_buff_sk, offsetof(struct __sk_buff, sk)),
> + __imm_const(bpf_sock_rx_queue_mapping, offsetof(struct bpf_sock, rx_queue_mapping))
> + : __clobber_all);
> +}
> +
> +SEC("socket")
> +__description("skb->sk: sk->rx_queue_mapping [narrow load mask]")
> +__success __success_unpriv __retval(0)
> +__naked void sk_rx_queue_mapping_narrow_load_mask(void)
> +{
> + asm volatile (" \
> + r1 = *(u64*)(r1 + %[__sk_buff_sk]); \
> + if r1 != 0 goto l0_%=; \
> + r0 = 0xdead; \
> + exit; \
> +l0_%=: r0 = *(u16*)(r1 + %[bpf_sock_rx_queue_mapping]); \
> + r0 >>= 16; \
> + exit; \
[Severity: Medium]
Similarly, does this narrow load test pass at runtime without strictly
asserting the verifier's internal bounds tracking?
Like the previous function, this shifts the 16-bit load right by 16 and
exits. It seems this will also implicitly pass at runtime rather than
causing a verification failure if bounds tracking regressions occur.
> +" :
> + : __imm_const(__sk_buff_sk, offsetof(struct __sk_buff, sk)),
> + __imm_const(bpf_sock_rx_queue_mapping, offsetof(struct bpf_sock, rx_queue_mapping))
> + : __clobber_all);
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260922172028.6269-1-emil@etsalapatis.com?part=4
next prev parent reply other threads:[~2026-09-22 17:36 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-22 17:20 [PATCH bpf v2 00/11] skb/arena bugfixes Emil Tsalapatis
2026-09-22 17:20 ` [PATCH bpf v2 01/11] bpf: Fix bounds check for skb-backed dynptrs Emil Tsalapatis
2026-09-22 17:27 ` sashiko-bot
2026-09-22 20:15 ` Amery Hung
2026-09-22 17:20 ` [PATCH bpf v2 02/11] selftests/bpf: Test dynptr slices past end of skb Emil Tsalapatis
2026-09-22 17:28 ` sashiko-bot
2026-09-22 20:16 ` Amery Hung
2026-09-22 17:20 ` [PATCH bpf v2 03/11] bpf: Fix bpf_sock context code generation Emil Tsalapatis
2026-09-22 17:20 ` [PATCH bpf v2 04/11] selftests/bpf: Add selftests for rx_queue_mapping context access Emil Tsalapatis
2026-09-22 17:36 ` sashiko-bot [this message]
2026-09-22 17:20 ` [PATCH bpf v2 05/11] bpf: Reject pkt arguments in mutating subprogs Emil Tsalapatis
2026-09-22 20:32 ` Amery Hung
2026-09-22 17:20 ` [PATCH bpf v2 06/11] selftests/bpf: Test rejection of pkt args to " Emil Tsalapatis
2026-09-22 17:20 ` [PATCH bpf v2 07/11] bpf: Prevent variable arena/non-arena register contents Emil Tsalapatis
2026-09-22 17:20 ` [PATCH bpf v2 08/11] selftests/bpf: Test for mixed arena/nonarena code paths Emil Tsalapatis
2026-09-22 17:20 ` [PATCH bpf v2 09/11] bpf: Track whether dynptr type is known Emil Tsalapatis
2026-09-22 18:46 ` Alexei Starovoitov
2026-09-22 20:23 ` Amery Hung
2026-09-22 20:28 ` Emil Tsalapatis
2026-09-22 17:20 ` [PATCH bpf v2 10/11] bpf: Track skb memory invalidation by packet-backed dynptrs Emil Tsalapatis
2026-09-22 20:06 ` Amery Hung
2026-09-22 20:29 ` Emil Tsalapatis
2026-09-22 17:20 ` [PATCH bpf v2 11/11] selftests/bpf: Test dynptr slice invalidation on skb clobber Emil Tsalapatis
2026-09-22 19:40 ` [PATCH bpf v2 00/11] skb/arena bugfixes patchwork-bot+netdevbpf
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=20260922173612.4B7191F000FF@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=emil@etsalapatis.com \
--cc=sashiko-reviews@lists.linux.dev \
/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