[PATCH bpf v3 0/2] bpf, sockmap: Fix FIONREAD for sockets without a verdict program

[PATCH bpf v3 0/2] bpf, sockmap: Fix FIONREAD for sockets without a verdict program

[Mattia Meleleo via B4 Relay]

Sockets added to a sockmap/sockhash with no stream/skb verdict program
attached answer FIONREAD with 0 even when unread data is pending in
sk_receive_queue. Fix tcp_bpf_ioctl() to account for the receive queue
in that case, and add a selftest.

Changes in v3:
- Remove unused sk_psock_msg_inq()
- Link to v2: https://patch.msgid.link/20260708-fionread-no-verdict-v2-0-29dd293621c7@coralogix.com

Changes in v2:
- Split the fix and the selftest into separate patches
- Use READ_ONCE() to read the verdict program pointers
- Link to v1: https://patch.msgid.link/20260707-fionread-no-verdict-v1-1-ce94a72357ec@coralogix.com

Signed-off-by: Mattia Meleleo <mattia.meleleo@coralogix.com>
---
Mattia Meleleo (2):
      bpf, sockmap: Account for receive queue in FIONREAD without a verdict program
      selftests/bpf: Test FIONREAD on a sockmap socket without a verdict program

 include/linux/skmsg.h                              | 14 --------
 net/ipv4/tcp_bpf.c                                 | 17 +++++++++-
 .../selftests/bpf/prog_tests/sockmap_basic.c       | 39 ++++++++++++++++++++++
 3 files changed, 55 insertions(+), 15 deletions(-)
---
base-commit: d2c9a99135da931377240942d44f3dea104cedb8
change-id: 20260707-fionread-no-verdict-a4f8697ac9f9

Best regards,
--  
Mattia Meleleo <mattia.meleleo@coralogix.com>

[PATCH bpf v3 0/2] bpf, sockmap: Fix FIONREAD for sockets without a verdict program

[Mattia Meleleo]

Sockets added to a sockmap/sockhash with no stream/skb verdict program
attached answer FIONREAD with 0 even when unread data is pending in
sk_receive_queue. Fix tcp_bpf_ioctl() to account for the receive queue
in that case, and add a selftest.

Changes in v3:
- Remove unused sk_psock_msg_inq()
- Link to v2: https://patch.msgid.link/20260708-fionread-no-verdict-v2-0-29dd293621c7@coralogix.com

Changes in v2:
- Split the fix and the selftest into separate patches
- Use READ_ONCE() to read the verdict program pointers
- Link to v1: https://patch.msgid.link/20260707-fionread-no-verdict-v1-1-ce94a72357ec@coralogix.com

Signed-off-by: Mattia Meleleo <mattia.meleleo@coralogix.com>
---
Mattia Meleleo (2):
      bpf, sockmap: Account for receive queue in FIONREAD without a verdict program
      selftests/bpf: Test FIONREAD on a sockmap socket without a verdict program

 include/linux/skmsg.h                              | 14 --------
 net/ipv4/tcp_bpf.c                                 | 17 +++++++++-
 .../selftests/bpf/prog_tests/sockmap_basic.c       | 39 ++++++++++++++++++++++
 3 files changed, 55 insertions(+), 15 deletions(-)
---
base-commit: d2c9a99135da931377240942d44f3dea104cedb8
change-id: 20260707-fionread-no-verdict-a4f8697ac9f9

Best regards,
--  
Mattia Meleleo <mattia.meleleo@coralogix.com>

[PATCH bpf v3 1/2] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program

[Mattia Meleleo via B4 Relay]

From: Mattia Meleleo <mattia.meleleo@coralogix.com>

tcp_bpf_ioctl() answers SIOCINQ from psock->msg_tot_len, which only
counts bytes in ingress_msg. Without a stream/skb verdict program
nothing is diverted there: data stays in sk_receive_queue, so FIONREAD
returns 0 even though read() returns data.

Add tcp_inq() to the reported value when the psock has no verdict
program. The two queues are disjoint, so bytes redirected into
ingress_msg from other sockets stay correctly accounted through
msg_tot_len.

Remove unused sk_psock_msg_inq().

Fixes: 929e30f93125 ("bpf, sockmap: Fix FIONREAD for sockmap")
Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
Signed-off-by: Mattia Meleleo <mattia.meleleo@coralogix.com>
---
 include/linux/skmsg.h | 14 --------------
 net/ipv4/tcp_bpf.c    | 17 ++++++++++++++++-
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
index a8553401b..d5e35f247 100644
--- a/include/linux/skmsg.h
+++ b/include/linux/skmsg.h
@@ -551,20 +551,6 @@ static inline void psock_progs_drop(struct sk_psock_progs *progs)
 	psock_set_prog(&progs->skb_verdict, NULL);
 }
 
-/* for tcp only, sk is locked */
-static inline ssize_t sk_psock_msg_inq(struct sock *sk)
-{
-	struct sk_psock *psock;
-	ssize_t inq = 0;
-
-	psock = sk_psock_get(sk);
-	if (likely(psock)) {
-		inq = sk_psock_get_msg_len_nolock(psock);
-		sk_psock_put(sk, psock);
-	}
-	return inq;
-}
-
 /* for udp only, sk is not locked */
 static inline ssize_t sk_msg_first_len(struct sock *sk)
 {
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index cc0bd73f3..8e905b50d 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -334,6 +334,7 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
 
 static int tcp_bpf_ioctl(struct sock *sk, int cmd, int *karg)
 {
+	struct sk_psock *psock;
 	bool slow;
 
 	if (cmd != SIOCINQ)
@@ -344,7 +345,21 @@ static int tcp_bpf_ioctl(struct sock *sk, int cmd, int *karg)
 		return -EINVAL;
 
 	slow = lock_sock_fast(sk);
-	*karg = sk_psock_msg_inq(sk);
+	psock = sk_psock_get(sk);
+	if (unlikely(!psock)) {
+		unlock_sock_fast(sk, slow);
+		return tcp_ioctl(sk, cmd, karg);
+	}
+	*karg = sk_psock_get_msg_len_nolock(psock);
+	/* Without a verdict program, ingress data is never diverted to
+	 * ingress_msg: it stays in sk_receive_queue and is read through
+	 * the fallback to tcp_recvmsg(), so account for it like
+	 * tcp_ioctl() does.
+	 */
+	if (!READ_ONCE(psock->progs.stream_verdict) &&
+	    !READ_ONCE(psock->progs.skb_verdict))
+		*karg += tcp_inq(sk);
+	sk_psock_put(sk, psock);
 	unlock_sock_fast(sk, slow);
 
 	return 0;

-- 
Git-155)

[PATCH bpf v3 1/2] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program

[Mattia Meleleo]

tcp_bpf_ioctl() answers SIOCINQ from psock->msg_tot_len, which only
counts bytes in ingress_msg. Without a stream/skb verdict program
nothing is diverted there: data stays in sk_receive_queue, so FIONREAD
returns 0 even though read() returns data.

Add tcp_inq() to the reported value when the psock has no verdict
program. The two queues are disjoint, so bytes redirected into
ingress_msg from other sockets stay correctly accounted through
msg_tot_len.

Remove unused sk_psock_msg_inq().

Fixes: 929e30f93125 ("bpf, sockmap: Fix FIONREAD for sockmap")
Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
Signed-off-by: Mattia Meleleo <mattia.meleleo@coralogix.com>
---
 include/linux/skmsg.h | 14 --------------
 net/ipv4/tcp_bpf.c    | 17 ++++++++++++++++-
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
index a8553401b..d5e35f247 100644
--- a/include/linux/skmsg.h
+++ b/include/linux/skmsg.h
@@ -551,20 +551,6 @@ static inline void psock_progs_drop(struct sk_psock_progs *progs)
 	psock_set_prog(&progs->skb_verdict, NULL);
 }
 
-/* for tcp only, sk is locked */
-static inline ssize_t sk_psock_msg_inq(struct sock *sk)
-{
-	struct sk_psock *psock;
-	ssize_t inq = 0;
-
-	psock = sk_psock_get(sk);
-	if (likely(psock)) {
-		inq = sk_psock_get_msg_len_nolock(psock);
-		sk_psock_put(sk, psock);
-	}
-	return inq;
-}
-
 /* for udp only, sk is not locked */
 static inline ssize_t sk_msg_first_len(struct sock *sk)
 {
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index cc0bd73f3..8e905b50d 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -334,6 +334,7 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
 
 static int tcp_bpf_ioctl(struct sock *sk, int cmd, int *karg)
 {
+	struct sk_psock *psock;
 	bool slow;
 
 	if (cmd != SIOCINQ)
@@ -344,7 +345,21 @@ static int tcp_bpf_ioctl(struct sock *sk, int cmd, int *karg)
 		return -EINVAL;
 
 	slow = lock_sock_fast(sk);
-	*karg = sk_psock_msg_inq(sk);
+	psock = sk_psock_get(sk);
+	if (unlikely(!psock)) {
+		unlock_sock_fast(sk, slow);
+		return tcp_ioctl(sk, cmd, karg);
+	}
+	*karg = sk_psock_get_msg_len_nolock(psock);
+	/* Without a verdict program, ingress data is never diverted to
+	 * ingress_msg: it stays in sk_receive_queue and is read through
+	 * the fallback to tcp_recvmsg(), so account for it like
+	 * tcp_ioctl() does.
+	 */
+	if (!READ_ONCE(psock->progs.stream_verdict) &&
+	    !READ_ONCE(psock->progs.skb_verdict))
+		*karg += tcp_inq(sk);
+	sk_psock_put(sk, psock);
 	unlock_sock_fast(sk, slow);
 
 	return 0;

-- 
Git-155)

[PATCH bpf v3 2/2] selftests/bpf: Test FIONREAD on a sockmap socket without a verdict program

[Mattia Meleleo via B4 Relay]

From: Mattia Meleleo <mattia.meleleo@coralogix.com>

Add a test validating that FIONREAD on a TCP socket in a sockmap
without a verdict program reports data pending in sk_receive_queue.

Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
Signed-off-by: Mattia Meleleo <mattia.meleleo@coralogix.com>
---
 .../selftests/bpf/prog_tests/sockmap_basic.c       | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index cb3229711..f0f368201 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -1373,6 +1373,43 @@ static void test_sockmap_multi_channels(int sotype)
 	test_sockmap_pass_prog__destroy(skel);
 }
 
+/* A socket in a sockmap without a verdict program keeps its ingress data
+ * in sk_receive_queue: FIONREAD must account for it.
+ */
+static void test_sockmap_no_verdict_fionread(void)
+{
+	int err, map, zero = 0, sent, avail;
+	int c0 = -1, c1 = -1, p0 = -1, p1 = -1;
+	struct test_sockmap_pass_prog *skel;
+	char buf[256] = "0123456789";
+
+	skel = test_sockmap_pass_prog__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open_and_load"))
+		return;
+	map = bpf_map__fd(skel->maps.sock_map_rx);
+
+	err = create_socket_pairs(AF_INET, SOCK_STREAM, &c0, &c1, &p0, &p1);
+	if (!ASSERT_OK(err, "create_socket_pairs()"))
+		goto out;
+
+	err = bpf_map_update_elem(map, &zero, &c1, BPF_NOEXIST);
+	if (!ASSERT_OK(err, "bpf_map_update_elem(c1)"))
+		goto out_close;
+
+	sent = xsend(p1, &buf, sizeof(buf), 0);
+	ASSERT_EQ(sent, sizeof(buf), "xsend(p1)");
+	avail = wait_for_fionread(c1, sizeof(buf), IO_TIMEOUT_SEC);
+	ASSERT_EQ(avail, sizeof(buf), "ioctl(FIONREAD)");
+
+out_close:
+	close(c0);
+	close(p0);
+	close(c1);
+	close(p1);
+out:
+	test_sockmap_pass_prog__destroy(skel);
+}
+
 void test_sockmap_basic(void)
 {
 	if (test__start_subtest("sockmap create_update_free"))
@@ -1415,6 +1452,8 @@ void test_sockmap_basic(void)
 		test_sockmap_skb_verdict_shutdown();
 	if (test__start_subtest("sockmap skb_verdict fionread"))
 		test_sockmap_skb_verdict_fionread(true);
+	if (test__start_subtest("sockmap no_verdict fionread"))
+		test_sockmap_no_verdict_fionread();
 	if (test__start_subtest("sockmap skb_verdict fionread on drop"))
 		test_sockmap_skb_verdict_fionread(false);
 	if (test__start_subtest("sockmap skb_verdict change tail"))

-- 
Git-155)

[PATCH bpf v3 2/2] selftests/bpf: Test FIONREAD on a sockmap socket without a verdict program

[Mattia Meleleo]

Add a test validating that FIONREAD on a TCP socket in a sockmap
without a verdict program reports data pending in sk_receive_queue.

Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>
Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
Signed-off-by: Mattia Meleleo <mattia.meleleo@coralogix.com>
---
 .../selftests/bpf/prog_tests/sockmap_basic.c       | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index cb3229711..f0f368201 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -1373,6 +1373,43 @@ static void test_sockmap_multi_channels(int sotype)
 	test_sockmap_pass_prog__destroy(skel);
 }
 
+/* A socket in a sockmap without a verdict program keeps its ingress data
+ * in sk_receive_queue: FIONREAD must account for it.
+ */
+static void test_sockmap_no_verdict_fionread(void)
+{
+	int err, map, zero = 0, sent, avail;
+	int c0 = -1, c1 = -1, p0 = -1, p1 = -1;
+	struct test_sockmap_pass_prog *skel;
+	char buf[256] = "0123456789";
+
+	skel = test_sockmap_pass_prog__open_and_load();
+	if (!ASSERT_OK_PTR(skel, "open_and_load"))
+		return;
+	map = bpf_map__fd(skel->maps.sock_map_rx);
+
+	err = create_socket_pairs(AF_INET, SOCK_STREAM, &c0, &c1, &p0, &p1);
+	if (!ASSERT_OK(err, "create_socket_pairs()"))
+		goto out;
+
+	err = bpf_map_update_elem(map, &zero, &c1, BPF_NOEXIST);
+	if (!ASSERT_OK(err, "bpf_map_update_elem(c1)"))
+		goto out_close;
+
+	sent = xsend(p1, &buf, sizeof(buf), 0);
+	ASSERT_EQ(sent, sizeof(buf), "xsend(p1)");
+	avail = wait_for_fionread(c1, sizeof(buf), IO_TIMEOUT_SEC);
+	ASSERT_EQ(avail, sizeof(buf), "ioctl(FIONREAD)");
+
+out_close:
+	close(c0);
+	close(p0);
+	close(c1);
+	close(p1);
+out:
+	test_sockmap_pass_prog__destroy(skel);
+}
+
 void test_sockmap_basic(void)
 {
 	if (test__start_subtest("sockmap create_update_free"))
@@ -1415,6 +1452,8 @@ void test_sockmap_basic(void)
 		test_sockmap_skb_verdict_shutdown();
 	if (test__start_subtest("sockmap skb_verdict fionread"))
 		test_sockmap_skb_verdict_fionread(true);
+	if (test__start_subtest("sockmap no_verdict fionread"))
+		test_sockmap_no_verdict_fionread();
 	if (test__start_subtest("sockmap skb_verdict fionread on drop"))
 		test_sockmap_skb_verdict_fionread(false);
 	if (test__start_subtest("sockmap skb_verdict change tail"))

-- 
Git-155)

Re: [PATCH bpf v3 2/2] selftests/bpf: Test FIONREAD on a sockmap socket without a verdict program

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] The macro `IO_TIMEOUT_SEC` (30) is passed as the `timeout_ms` argument to `wait_for_fionread()`, resulting in a very short 30 millisecond timeout instead of 30 seconds.
- [Low] Multi-line comment does not follow the strict BPF subsystem comment style.
- [Low] Unnecessary creation and cleanup of an unused socket pair (`c0` and `p0`).
--

--- Patch [2]: [PATCH bpf v3 2/2] selftests/bpf: Test FIONREAD on a sockmap socket without a verdict program ---
commit ef6ce74d985414c924fd029034cc51c133f6ad26
Author: Mattia Meleleo <mattia.meleleo@coralogix.com>

selftests/bpf: Test FIONREAD on a sockmap socket without a verdict program

Add a test validating that FIONREAD on a TCP socket in a sockmap
without a verdict program reports data pending in sk_receive_queue.

> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> index cb3229711f93a..f0f368201ddad 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> @@ -1373,6 +1373,43 @@ static void test_sockmap_multi_channels(int sotype)
>  	test_sockmap_pass_prog__destroy(skel);
>  }
>  
> +/* A socket in a sockmap without a verdict program keeps its ingress data
> + * in sk_receive_queue: FIONREAD must account for it.
> + */

[Severity: Low]
This isn't a bug, but does this multi-line comment follow the strict BPF
subsystem style guidelines? It appears the opening /* is expected to be
on its own line.

> +static void test_sockmap_no_verdict_fionread(void)
> +{
> +	int err, map, zero = 0, sent, avail;
> +	int c0 = -1, c1 = -1, p0 = -1, p1 = -1;
> +	struct test_sockmap_pass_prog *skel;
> +	char buf[256] = "0123456789";
> +
> +	skel = test_sockmap_pass_prog__open_and_load();
> +	if (!ASSERT_OK_PTR(skel, "open_and_load"))
> +		return;
> +	map = bpf_map__fd(skel->maps.sock_map_rx);
> +
> +	err = create_socket_pairs(AF_INET, SOCK_STREAM, &c0, &c1, &p0, &p1);

[Severity: Low]
Is there a reason to allocate four socket file descriptors here?

It looks like the c0 and p0 sockets are created but never used in the test.
Could create_pair() be used instead to avoid unnecessarily creating and
closing the unused socket pair?

> +	if (!ASSERT_OK(err, "create_socket_pairs()"))
> +		goto out;
> +
> +	err = bpf_map_update_elem(map, &zero, &c1, BPF_NOEXIST);
> +	if (!ASSERT_OK(err, "bpf_map_update_elem(c1)"))
> +		goto out_close;
> +
> +	sent = xsend(p1, &buf, sizeof(buf), 0);
> +	ASSERT_EQ(sent, sizeof(buf), "xsend(p1)");
> +	avail = wait_for_fionread(c1, sizeof(buf), IO_TIMEOUT_SEC);

[Severity: Medium]
Will this result in a flaky test on slow or heavily loaded CI systems?

Looking at the wait_for_fionread() implementation, the third parameter is
timeout_ms, which dictates the number of 1 millisecond sleep iterations:

	while (elapsed < timeout_ms) {
		if (ioctl(fd, FIONREAD, &avail) < 0)
			return -errno;
		if (avail >= expected)
			return avail;
		usleep(1000);
		elapsed++;
	}

By passing IO_TIMEOUT_SEC (which is typically 30), it seems the test will only
wait 30 milliseconds instead of 30 seconds for the packet to traverse the local
network stack.

> +	ASSERT_EQ(avail, sizeof(buf), "ioctl(FIONREAD)");
> +
> +out_close:
> +	close(c0);
> +	close(p0);
> +	close(c1);
> +	close(p1);
> +out:
> +	test_sockmap_pass_prog__destroy(skel);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-fionread-no-verdict-v3-0-b4ee31b3af53@coralogix.com?part=2

Re: [PATCH bpf v3 1/2] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] FIONREAD (SIOCINQ) double-counts available bytes when a verdict program is detached, exposes negative values, and under-reports bytes when a verdict program is active.

Pre-existing issues:
- [High] udp_bpf_ioctl() suffers from the exact same bug that this patch fixes for tcp_bpf_ioctl(). Without a verdict program, data stays in sk_receive_queue and UDP FIONREAD returns 0.
- [High] Data race and sequence number corruption in psock->ingress_bytes during SK_REDIRECT.
--

--- Patch [1]: [PATCH bpf v3 1/2] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program ---
commit fc144ea49f61a51adcd404a526d68065c61d0452
Author: Mattia Meleleo <mattia.meleleo@coralogix.com>

bpf, sockmap: Account for receive queue in FIONREAD without a verdict program

This commit modifies tcp_bpf_ioctl() to add the output of tcp_inq() to the
reported FIONREAD value when no stream or skb verdict program is attached.
This correctly accounts for data resting in sk_receive_queue that has not
been diverted to the ingress_msg list.

> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index cc0bd73f36b6d..8e905b50deadb 100644
> --- a/net/ipv4/tcp_bpf.c
> +++ b/net/ipv4/tcp_bpf.c
[ ... ]
> @@ -344,7 +345,21 @@ static int tcp_bpf_ioctl(struct sock *sk, int cmd, int *karg)
>  		return -EINVAL;
>  
>  	slow = lock_sock_fast(sk);
> -	*karg = sk_psock_msg_inq(sk);
> +	psock = sk_psock_get(sk);
> +	if (unlikely(!psock)) {
> +		unlock_sock_fast(sk, slow);
> +		return tcp_ioctl(sk, cmd, karg);
> +	}
> +	*karg = sk_psock_get_msg_len_nolock(psock);
> +	/* Without a verdict program, ingress data is never diverted to
> +	 * ingress_msg: it stays in sk_receive_queue and is read through
> +	 * the fallback to tcp_recvmsg(), so account for it like
> +	 * tcp_ioctl() does.
> +	 */
> +	if (!READ_ONCE(psock->progs.stream_verdict) &&
> +	    !READ_ONCE(psock->progs.skb_verdict))
> +		*karg += tcp_inq(sk);

[Severity: High]
Does this conditional logic double-count available bytes or under-report them
in some situations?

If a stream verdict program is attached, data is queued (which delays
tp->copied_seq), and then the program is detached. The unread bytes sit in
both msg_tot_len (ingress_msg) and are also reflected in tcp_inq(sk) because
copied_seq was held back. Adding them together here would double-count the
available data.

Conversely, when a verdict program is active, tcp_inq(sk) is ignored. Could
this fail to account for unparsed data in sk_receive_queue, leading to an
under-reported value?

> +	sk_psock_put(sk, psock);
>  	unlock_sock_fast(sk, slow);
>  
>  	return 0;

[Severity: High]
This isn't a bug introduced by this patch, but udp_bpf_ioctl() in
net/ipv4/udp_bpf.c appears to suffer from the exact same bug that this patch
fixes for tcp_bpf_ioctl().

net/ipv4/udp_bpf.c:udp_bpf_ioctl() {
    ...
    *karg = sk_msg_first_len(sk);
    ...
}

Without a verdict program, data stays in sk_receive_queue, and UDP FIONREAD
constantly returns 0. Could a similar fallback to the native receive queue be
applied there to prevent user-space applications from hanging or dropping
packets when relying on ioctl(FIONREAD)?

[Severity: High]
This is a pre-existing issue, but is there a data race and sequence number
corruption in psock->ingress_bytes during SK_REDIRECT?

In sk_psock_skb_ingress_enqueue(), psock->ingress_bytes is incremented without
locks:

net/core/skmsg.c:sk_psock_skb_ingress_enqueue() {
    ...
    psock->ingress_bytes += len;
    ...
}

Concurrently, tcp_bpf_strp_read_sock() locklessly resets psock->ingress_bytes
to 0 and uses it to adjust tp->copied_seq:

net/ipv4/tcp_bpf.c:tcp_bpf_strp_read_sock() {
    ...
    psock->ingress_bytes = 0;
    ...
    tp->copied_seq = psock->copied_seq - psock->ingress_bytes;
    ...
}

If another socket redirects data to this socket, the target socket's
psock->ingress_bytes is incremented by the workqueue or softirq. Does the race
with tcp_bpf_strp_read_sock() cause redirected bytes to be incorrectly
subtracted from the target socket's tp->copied_seq, artificially holding back
the ACK sequence and corrupting the TCP state?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260708-fionread-no-verdict-v3-0-b4ee31b3af53@coralogix.com?part=1

Re: [PATCH bpf v3 1/2] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program

[John Fastabend]

On Wed, Jul 08, 2026 at 05:16:59PM +0000, sashiko-bot@kernel.org wrote:
>Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
>
>New issues:
>- [High] FIONREAD (SIOCINQ) double-counts available bytes when a verdict program is detached, exposes negative values, and under-reports bytes when a verdict program is active.
>
>Pre-existing issues:
>- [High] udp_bpf_ioctl() suffers from the exact same bug that this patch fixes for tcp_bpf_ioctl(). Without a verdict program, data stays in sk_receive_queue and UDP FIONREAD returns 0.
>- [High] Data race and sequence number corruption in psock->ingress_bytes during SK_REDIRECT.
>--
>
>--- Patch [1]: [PATCH bpf v3 1/2] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program ---
>commit fc144ea49f61a51adcd404a526d68065c61d0452
>Author: Mattia Meleleo <mattia.meleleo@coralogix.com>
>
>bpf, sockmap: Account for receive queue in FIONREAD without a verdict program
>
>This commit modifies tcp_bpf_ioctl() to add the output of tcp_inq() to the
>reported FIONREAD value when no stream or skb verdict program is attached.
>This correctly accounts for data resting in sk_receive_queue that has not
>been diverted to the ingress_msg list.
>
>> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
>> index cc0bd73f36b6d..8e905b50deadb 100644
>> --- a/net/ipv4/tcp_bpf.c
>> +++ b/net/ipv4/tcp_bpf.c
>[ ... ]
>> @@ -344,7 +345,21 @@ static int tcp_bpf_ioctl(struct sock *sk, int cmd, int *karg)
>>  		return -EINVAL;
>>
>>  	slow = lock_sock_fast(sk);
>> -	*karg = sk_psock_msg_inq(sk);
>> +	psock = sk_psock_get(sk);
>> +	if (unlikely(!psock)) {
>> +		unlock_sock_fast(sk, slow);
>> +		return tcp_ioctl(sk, cmd, karg);
>> +	}
>> +	*karg = sk_psock_get_msg_len_nolock(psock);
>> +	/* Without a verdict program, ingress data is never diverted to
>> +	 * ingress_msg: it stays in sk_receive_queue and is read through
>> +	 * the fallback to tcp_recvmsg(), so account for it like
>> +	 * tcp_ioctl() does.
>> +	 */
>> +	if (!READ_ONCE(psock->progs.stream_verdict) &&
>> +	    !READ_ONCE(psock->progs.skb_verdict))
>> +		*karg += tcp_inq(sk);
>
>[Severity: High]
>Does this conditional logic double-count available bytes or under-report them
>in some situations?
>
>If a stream verdict program is attached, data is queued (which delays
>tp->copied_seq), and then the program is detached. The unread bytes sit in
>both msg_tot_len (ingress_msg) and are also reflected in tcp_inq(sk) because
>copied_seq was held back. Adding them together here would double-count the
>available data.

This is fine lets ignore this for now. Every system we have adds sockets
to the sockmap on connect so this condition is not seen. And above 
solves common case.

>
>Conversely, when a verdict program is active, tcp_inq(sk) is ignored. Could
>this fail to account for unparsed data in sk_receive_queue, leading to an
>under-reported value?
>
>> +	sk_psock_put(sk, psock);
>>  	unlock_sock_fast(sk, slow);
>>
>>  	return 0;
>
>[Severity: High]
>This isn't a bug introduced by this patch, but udp_bpf_ioctl() in
>net/ipv4/udp_bpf.c appears to suffer from the exact same bug that this patch
>fixes for tcp_bpf_ioctl().

Sure but doesn't need to hold up this patch. Mattia would you want to
fix this as well? Otherwise I'll put it on my list and get to it at
some point.

>
>net/ipv4/udp_bpf.c:udp_bpf_ioctl() {
>    ...
>    *karg = sk_msg_first_len(sk);
>    ...
>}
>
>Without a verdict program, data stays in sk_receive_queue, and UDP FIONREAD
>constantly returns 0. Could a similar fallback to the native receive queue be
>applied there to prevent user-space applications from hanging or dropping
>packets when relying on ioctl(FIONREAD)?

yes we should do something here.

>
>[Severity: High]
>This is a pre-existing issue, but is there a data race and sequence number
>corruption in psock->ingress_bytes during SK_REDIRECT?

yep pre-existing needs another patch.

>
>In sk_psock_skb_ingress_enqueue(), psock->ingress_bytes is incremented without
>locks:
>
>net/core/skmsg.c:sk_psock_skb_ingress_enqueue() {
>    ...
>    psock->ingress_bytes += len;
>    ...
>}
>
>Concurrently, tcp_bpf_strp_read_sock() locklessly resets psock->ingress_bytes
>to 0 and uses it to adjust tp->copied_seq:
>
>net/ipv4/tcp_bpf.c:tcp_bpf_strp_read_sock() {
>    ...
>    psock->ingress_bytes = 0;
>    ...
>    tp->copied_seq = psock->copied_seq - psock->ingress_bytes;
>    ...
>}
>
>If another socket redirects data to this socket, the target socket's
>psock->ingress_bytes is incremented by the workqueue or softirq. Does the race
>with tcp_bpf_strp_read_sock() cause redirected bytes to be incorrectly
>subtracted from the target socket's tp->copied_seq, artificially holding back
>the ACK sequence and corrupting the TCP state?
>
>-- 
>Sashiko AI review · https://sashiko.dev/#/patchset/20260708-fionread-no-verdict-v3-0-b4ee31b3af53@coralogix.com?part=1

Re: [PATCH bpf v3 1/2] bpf, sockmap: Account for receive queue in FIONREAD without a verdict program

[John Fastabend]

On Wed, Jul 08, 2026 at 06:55:00PM +0200, Mattia Meleleo via B4 Relay wrote:
>From: Mattia Meleleo <mattia.meleleo@coralogix.com>
>
>tcp_bpf_ioctl() answers SIOCINQ from psock->msg_tot_len, which only
>counts bytes in ingress_msg. Without a stream/skb verdict program
>nothing is diverted there: data stays in sk_receive_queue, so FIONREAD
>returns 0 even though read() returns data.
>
>Add tcp_inq() to the reported value when the psock has no verdict
>program. The two queues are disjoint, so bytes redirected into
>ingress_msg from other sockets stay correctly accounted through
>msg_tot_len.
>
>Remove unused sk_psock_msg_inq().
>
>Fixes: 929e30f93125 ("bpf, sockmap: Fix FIONREAD for sockmap")
>Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>
>Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
>Signed-off-by: Mattia Meleleo <mattia.meleleo@coralogix.com>
>---

This would be good to merge. The bots had some good callouts but mostly 
they should be follow ups IMO. There is some concern that the FIONREAD
may not be accurate on transitioning add/remove of progs, but mostly
this is done at connect and then removed only done on socket tear down.
At the moment this is causing real application problems.

The most relevant bot callout would be to make this work for UDP progs
correctly as well.

Reviewed-by: John Fastabend <john.fastabend@gmail.com>

Re: [PATCH bpf v3 2/2] selftests/bpf: Test FIONREAD on a sockmap socket without a verdict program

[John Fastabend]

On Wed, Jul 08, 2026 at 06:55:01PM +0200, Mattia Meleleo via B4 Relay wrote:
>From: Mattia Meleleo <mattia.meleleo@coralogix.com>
>
>Add a test validating that FIONREAD on a TCP socket in a sockmap
>without a verdict program reports data pending in sk_receive_queue.
>
>Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>
>Reviewed-by: Emil Tsalapatis <emil@etsalapatis.com>
>Signed-off-by: Mattia Meleleo <mattia.meleleo@coralogix.com>
>---

LGTM we could improve IO_TIMEOUT_SEC across all the tests if we wanted
per bot suggestion.

Reviewed-by: John Fastabend <john.fastabend@gmail.com>