BPF List
 help / color / mirror / Atom feed
* [PATCH v5 0/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup
@ 2026-07-17 15:53 Nnamdi Onyeyiri
  2026-07-17 15:53 ` [PATCH v5 1/2] " Nnamdi Onyeyiri
  2026-07-17 15:53 ` [PATCH v5 2/2] selftest/bpf: added selftest sockmap_basic/sockmap recvfrom eagain Nnamdi Onyeyiri
  0 siblings, 2 replies; 6+ messages in thread
From: Nnamdi Onyeyiri @ 2026-07-17 15:53 UTC (permalink / raw)
  To: nnamdio
  Cc: bpf, davem, edumazet, horms, jakub, jiayuan.chen, john.fastabend,
	kuba, kuniyu, ncardwell, netdev, pabeni, sashiko-reviews,
	linux-kernel

Spurious wakeups in tcp_msg_wait_data() isn't being handled by
tcp_bpf_recvmsg() and tcp_bpf_recvmsg_parser(), leading to unexpected
EAGAIN errors returned by recvfrom()/recv().  Adding handling for the
wakeup and a selftest.

This issue was first discovered in an application that adds sockets to
a sockmap as a way to view the received data.  No redirects or any
other operations are performed.

Sashiko has noted a few other pre-existing issues in the same area
(https://patch.msgid.link/20260714205118.17DB11F000E9@smtp.kernel.org)
that can lead to EAGAIN in tcp_bpf_recvmsg() and
tcp_bpf_recvmsg_parser().  A local run of Sashiko also identified a
potential issue in tcp_bpf_recvmsg() handling zero-data FIN packets when
the MSG_PEEK flag is set.  To prevent this patchset from growing too
large, I intend to submit follow up patches to address these once this
one has been accepted.

Changes in v5:
- Move selftest from net into bpf/prog_tests/sockmap_basic.c
- Link to v4: https://patch.msgid.link/20260715213538.37229-1-nnamdio@gmail.com

Changes in v4:
- Fix potential data loss in tcp_bpf_recvmsg() when a FIN or RST has
  been received.
- Check the return code of pthread_create() in the selftest.
- Fix race caused by using EXPECT macros in a thread in selftest.
- Link to v3: https://patch.msgid.link/20260714203927.32289-1-nnamdio@gmail.com

Changes in v3:
- Added the sockmap_recvfrom selftest.
- Link to v2: https://patch.msgid.link/alFRK66z45eDNZA7@localhost.localdomain

Changes in v2:
- In tcp_bpf_recvmsg, handle signals and the socket closing in the loop.
- Fix spurious wakeups when SO_RCVTIMEO has been set on the socket.
- Link to v1: https://patch.msgid.link/ak_rR-Skd8Mvn4mH@localhost.localdomain

Signed-off-by: Nnamdi Onyeyiri <nnamdio@gmail.com>
---
Nnamdi Onyeyiri (2):
  bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup
  selftest/bpf: added selftest sockmap_basic/sockmap recvfrom eagain

 net/ipv4/tcp_bpf.c                            |  69 +++++++++--
 .../selftests/bpf/prog_tests/sockmap_basic.c  | 115 ++++++++++++++++++
 2 files changed, 175 insertions(+), 9 deletions(-)

-- 
2.52.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v5 1/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup
  2026-07-17 15:53 [PATCH v5 0/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup Nnamdi Onyeyiri
@ 2026-07-17 15:53 ` Nnamdi Onyeyiri
  2026-07-17 17:01   ` sashiko-bot
  2026-07-17 15:53 ` [PATCH v5 2/2] selftest/bpf: added selftest sockmap_basic/sockmap recvfrom eagain Nnamdi Onyeyiri
  1 sibling, 1 reply; 6+ messages in thread
From: Nnamdi Onyeyiri @ 2026-07-17 15:53 UTC (permalink / raw)
  To: nnamdio
  Cc: bpf, davem, edumazet, horms, jakub, jiayuan.chen, john.fastabend,
	kuba, kuniyu, ncardwell, netdev, pabeni, sashiko-reviews,
	linux-kernel

recvfrom()/recv() are documented as only returning EAGAIN for blocking sockets
when they have a receive timeout configured.  However, adding a blocking
ipv4 tcp socket without a receive timeout to a sockmap will cause EAGAIN errors
sporadically.  A socket with a receive timeout may return EAGAIN before the
timeout expires.

There are 2 code paths affected by this:

  1. tcp_bpf_recvmsg() - Used when the socket has been added to a sockmap
     that has no verdict program attached.

  2. tcp_bpf_recvmsg_parser() - Used when the socket has been added to a
     sockmap that has a verdict program.  To reproduce this issue, it is
     enough for the verdict program to do nothing but return SK_PASS.

In both cases this happens when tcp_msg_wait_data() wakes spuriously
(returning 0).  To fix it, we now loop back to msg_bytes_ready instead
of returning -EAGAIN on spurious wakeup.

To ensure the looping does not cause sockets with a SO_RCVTIMEO set to
wait excessively long, tcp_msg_wait_data() now takes a pointer to timeo,
allowing sk_wait_event() to update it as appropriate.

The logic in tcp_bpf_recvmsg_parser() that allow it to handle signals,
socket errors and closuers in its loop was also added to tcp_bpf_recvmsg().

Signed-off-by: Nnamdi Onyeyiri <nnamdio@gmail.com>
---
 net/ipv4/tcp_bpf.c | 69 ++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 60 insertions(+), 9 deletions(-)

diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index cc0bd73f36b6..aa5c5d741599 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -179,7 +179,7 @@ EXPORT_SYMBOL_GPL(tcp_bpf_sendmsg_redir);
 
 #ifdef CONFIG_BPF_SYSCALL
 static int tcp_msg_wait_data(struct sock *sk, struct sk_psock *psock,
-			     long timeo)
+			     long *timeo)
 {
 	DEFINE_WAIT_FUNC(wait, woken_wake_function);
 	int ret = 0;
@@ -187,12 +187,12 @@ static int tcp_msg_wait_data(struct sock *sk, struct sk_psock *psock,
 	if (sk->sk_shutdown & RCV_SHUTDOWN)
 		return 1;
 
-	if (!timeo)
+	if (!*timeo)
 		return ret;
 
 	add_wait_queue(sk_sleep(sk), &wait);
 	sk_set_bit(SOCKWQ_ASYNC_WAITDATA, sk);
-	ret = sk_wait_event(sk, &timeo,
+	ret = sk_wait_event(sk, timeo,
 			    !list_empty(&psock->ingress_msg) ||
 			    !skb_queue_empty_lockless(&sk->sk_receive_queue), &wait);
 	sk_clear_bit(SOCKWQ_ASYNC_WAITDATA, sk);
@@ -229,6 +229,7 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
 	int copied_from_self = 0;
 	int copied = 0;
 	u32 seq;
+	long timeo;
 
 	if (unlikely(flags & MSG_ERRQUEUE))
 		return inet_recv_error(sk, msg, len);
@@ -262,6 +263,8 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
 		}
 	}
 
+	timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
+
 msg_bytes_ready:
 	copied = __sk_msg_recvmsg(sk, psock, msg, len, flags, &copied_from_self);
 	/* The typical case for EFAULT is the socket was gracefully
@@ -280,7 +283,6 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
 	}
 	seq += copied_from_self;
 	if (!copied) {
-		long timeo;
 		int data;
 
 		if (sock_flag(sk, SOCK_DONE))
@@ -299,7 +301,6 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
 			goto out;
 		}
 
-		timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
 		if (!timeo) {
 			copied = -EAGAIN;
 			goto out;
@@ -310,13 +311,15 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
 			goto out;
 		}
 
-		data = tcp_msg_wait_data(sk, psock, timeo);
+		data = tcp_msg_wait_data(sk, psock, &timeo);
 		if (data < 0) {
 			copied = data;
 			goto unlock;
 		}
 		if (data && !sk_psock_queue_empty(psock))
 			goto msg_bytes_ready;
+		if (!data && timeo > 0)
+			goto msg_bytes_ready;
 		copied = -EAGAIN;
 	}
 out:
@@ -355,6 +358,7 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 {
 	struct sk_psock *psock;
 	int copied, ret;
+	long timeo;
 
 	if (unlikely(flags & MSG_ERRQUEUE))
 		return inet_recv_error(sk, msg, len);
@@ -371,14 +375,59 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 		return tcp_recvmsg(sk, msg, len, flags);
 	}
 	lock_sock(sk);
+
+	timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
+
 msg_bytes_ready:
 	copied = sk_msg_recvmsg(sk, psock, msg, len, flags);
 	if (!copied) {
-		long timeo;
 		int data;
 
-		timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
-		data = tcp_msg_wait_data(sk, psock, timeo);
+		if (sock_flag(sk, SOCK_DONE)) {
+			ret = 0;
+			goto unlock;
+		}
+
+		if (sk->sk_err) {
+			if (!sk_psock_queue_empty(psock))
+				goto msg_bytes_ready;
+			if (!skb_queue_empty(&sk->sk_receive_queue)) {
+				release_sock(sk);
+				sk_psock_put(sk, psock);
+				return tcp_recvmsg(sk, msg, len, flags);
+			}
+			ret = sock_error(sk);
+			goto unlock;
+		}
+
+		if (sk->sk_shutdown & RCV_SHUTDOWN) {
+			if (!sk_psock_queue_empty(psock))
+				goto msg_bytes_ready;
+			if (!skb_queue_empty(&sk->sk_receive_queue)) {
+				release_sock(sk);
+				sk_psock_put(sk, psock);
+				return tcp_recvmsg(sk, msg, len, flags);
+			}
+			ret = 0;
+			goto unlock;
+		}
+
+		if (sk->sk_state == TCP_CLOSE) {
+			ret = -ENOTCONN;
+			goto unlock;
+		}
+
+		if (!timeo) {
+			ret = -EAGAIN;
+			goto unlock;
+		}
+
+		if (signal_pending(current)) {
+			ret = sock_intr_errno(timeo);
+			goto unlock;
+		}
+
+		data = tcp_msg_wait_data(sk, psock, &timeo);
 		if (data < 0) {
 			ret = data;
 			goto unlock;
@@ -390,6 +439,8 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 			sk_psock_put(sk, psock);
 			return tcp_recvmsg(sk, msg, len, flags);
 		}
+		if (!data && timeo > 0)
+			goto msg_bytes_ready;
 		copied = -EAGAIN;
 	}
 	ret = copied;
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v5 2/2] selftest/bpf: added selftest sockmap_basic/sockmap recvfrom eagain
  2026-07-17 15:53 [PATCH v5 0/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup Nnamdi Onyeyiri
  2026-07-17 15:53 ` [PATCH v5 1/2] " Nnamdi Onyeyiri
@ 2026-07-17 15:53 ` Nnamdi Onyeyiri
  2026-07-17 16:36   ` bot+bpf-ci
  2026-07-17 17:29   ` sashiko-bot
  1 sibling, 2 replies; 6+ messages in thread
From: Nnamdi Onyeyiri @ 2026-07-17 15:53 UTC (permalink / raw)
  To: nnamdio
  Cc: bpf, davem, edumazet, horms, jakub, jiayuan.chen, john.fastabend,
	kuba, kuniyu, ncardwell, netdev, pabeni, sashiko-reviews,
	linux-kernel

These selftests exercise the tcp_bpf_recvmsg() and tcp_bpf_recvmsg_parser()
functions, to ensure that they are properly handling spurious wakeups in
tcp_msg_wait_data().

The expected behaviour is that recvfrom() does not return an EAGAIN
error.  If the spurious wakeups are incorrectly handled, this assertion
will fail.

Signed-off-by: Nnamdi Onyeyiri <nnamdio@gmail.com>
---
 .../selftests/bpf/prog_tests/sockmap_basic.c  | 115 ++++++++++++++++++
 1 file changed, 115 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
index cb3229711f93..852a12b17e6b 100644
--- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
+++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
@@ -1373,6 +1373,117 @@ static void test_sockmap_multi_channels(int sotype)
 	test_sockmap_pass_prog__destroy(skel);
 }
 
+static void *test_sockmap_recvfrom_eagain_thread(void *arg)
+{
+	int fd = *(int *)arg;
+	char buf[1024];
+	void *result = NULL;
+
+	while (true) {
+		ssize_t len = recvfrom(fd, buf, sizeof(buf), 0, NULL, NULL);
+
+		if (len == -1) {
+			if (errno == EINTR)
+				continue;
+			result = (void *)1;
+			break;
+		}
+
+		if (!len || buf[len - 1] == 'e')
+			break;
+	}
+
+	send(fd, "test", 4, MSG_NOSIGNAL);
+
+	return result;
+}
+
+static void test_sockmap_recvfrom_eagain(bool with_verdict)
+{
+	struct test_sockmap_pass_prog *skel = NULL;
+	struct bpf_program *prog = NULL;
+	size_t buflen = 1024 * 1024 * 25;
+	char *buf = NULL;
+	int map, err;
+
+	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_msg);
+
+	if (with_verdict) {
+		prog = skel->progs.prog_skb_verdict;
+		err = bpf_prog_attach(bpf_program__fd(prog), map, BPF_SK_SKB_STREAM_VERDICT, 0);
+		if (!ASSERT_OK(err, "bpf_prog_attach verdict"))
+			goto cleanup;
+	}
+
+	buf = malloc(buflen);
+	if (!ASSERT_OK_PTR(buf, "malloc buf"))
+		goto cleanup;
+	memset(buf, 0, buflen);
+	buf[buflen - 1] = 'e';
+
+	for (int i = 0; i < 200; ++i) {
+		ssize_t sent;
+		char ignored[128];
+		pthread_t thread;
+		bool thread_created = false;
+		size_t rem = buflen;
+		int c = -1, p = -1, zero = 0;
+		bool success = false;
+
+		err = create_pair(AF_INET, SOCK_STREAM, &c, &p);
+		if (!ASSERT_OK(err, "create_pair"))
+			goto end_attempt;
+
+		err = pthread_create(&thread, NULL, &test_sockmap_recvfrom_eagain_thread, &p);
+		if (!ASSERT_OK(err, "pthread_create"))
+			goto end_attempt;
+		thread_created = true;
+
+		err = bpf_map_update_elem(map, &zero, &c, BPF_ANY);
+		if (!ASSERT_OK(err, "bpf_map_update_elem"))
+			goto end_attempt;
+
+		while (rem) {
+			sent = xsend(c, buf + (buflen - rem), rem, 0);
+			if (sent == -1)
+				goto end_attempt;
+			rem -= sent;
+		}
+
+		err = recvfrom(c, ignored, sizeof(ignored), 0, NULL, NULL);
+		if (err < 0 && !ASSERT_NEQ(errno, EAGAIN, "recvfrom eagain"))
+			goto end_attempt;
+
+		success = true;
+
+end_attempt:
+		if (c >= 0)
+			close(c);
+
+		if (thread_created) {
+			void *retval = NULL;
+
+			pthread_join(thread, &retval);
+			ASSERT_NULL(retval, "retval");
+		}
+
+		if (p >= 0)
+			close(p);
+		if (!success)
+			break;
+	}
+
+cleanup:
+	if (buf)
+		free(buf);
+
+	test_sockmap_pass_prog__destroy(skel);
+}
+
 void test_sockmap_basic(void)
 {
 	if (test__start_subtest("sockmap create_update_free"))
@@ -1451,4 +1562,8 @@ void test_sockmap_basic(void)
 		test_sockmap_multi_channels(SOCK_STREAM);
 	if (test__start_subtest("sockmap udp multi channels"))
 		test_sockmap_multi_channels(SOCK_DGRAM);
+	if (test__start_subtest("sockmap recvfrom eagain"))
+		test_sockmap_recvfrom_eagain(false);
+	if (test__start_subtest("sockmap recvfrom eagain with verdict"))
+		test_sockmap_recvfrom_eagain(true);
 }
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v5 2/2] selftest/bpf: added selftest sockmap_basic/sockmap recvfrom eagain
  2026-07-17 15:53 ` [PATCH v5 2/2] selftest/bpf: added selftest sockmap_basic/sockmap recvfrom eagain Nnamdi Onyeyiri
@ 2026-07-17 16:36   ` bot+bpf-ci
  2026-07-17 17:29   ` sashiko-bot
  1 sibling, 0 replies; 6+ messages in thread
From: bot+bpf-ci @ 2026-07-17 16:36 UTC (permalink / raw)
  To: nnamdio, nnamdio
  Cc: bpf, davem, edumazet, horms, jakub, jiayuan.chen, john.fastabend,
	kuba, kuniyu, ncardwell, netdev, pabeni, sashiko-reviews,
	linux-kernel, ast, andrii, daniel, martin.lau, eddyz87,
	yonghong.song, clm, ihor.solodrai

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

> selftest/bpf: added selftest sockmap_basic/sockmap recvfrom eagain
>
> These selftests exercise the tcp_bpf_recvmsg() and tcp_bpf_recvmsg_parser()
> functions, to ensure that they are properly handling spurious wakeups in
> tcp_msg_wait_data().
>
> The expected behaviour is that recvfrom() does not return an EAGAIN
> error.  If the spurious wakeups are incorrectly handled, this assertion
> will fail.
>
> Signed-off-by: Nnamdi Onyeyiri <nnamdio@gmail.com>

this isn't a bug, but could the subject use the imperative mood, for
example "selftests/bpf: add sockmap recvfrom EAGAIN selftest", to match
the changelog convention and the sibling commit in the series?  The
current "added selftest sockmap_basic/sockmap recvfrom eagain" also
repeats "selftest".


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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v5 1/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup
  2026-07-17 15:53 ` [PATCH v5 1/2] " Nnamdi Onyeyiri
@ 2026-07-17 17:01   ` sashiko-bot
  0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-07-17 17:01 UTC (permalink / raw)
  To: Nnamdi Onyeyiri; +Cc: bpf

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

Pre-existing issues:
- [High] Calling recv() with a length of 0 on a BPF sockmap TCP socket with pending data and an error state causes an unkillable infinite loop in the kernel.
- [High] tcp_bpf_recvmsg propagates -EFAULT to userspace instead of returning EOF (0) when encountering a 0-length message (such as a redirected FIN).
--

--- Patch [1]: [PATCH v5 1/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup ---
commit 27d21b0db0456984003fefcc5f3d8528cdd5ea84
Author: Nnamdi Onyeyiri <nnamdio@gmail.com>

bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup

This commit fixes spurious wakeups in tcp_msg_wait_data() by looping back
to msg_bytes_ready instead of returning -EAGAIN. It also introduces timeo
pointer passing to avoid excessive waits and ports parser logic over to
tcp_bpf_recvmsg().

> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index 8e905b50deadb..e1c0f8b0930e1 100644
> --- a/net/ipv4/tcp_bpf.c
> +++ b/net/ipv4/tcp_bpf.c

[ ... ]

> @@ -386,14 +390,59 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
>  		return tcp_recvmsg(sk, msg, len, flags);
>  	}
>  	lock_sock(sk);
> +
> +	timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
> +
>  msg_bytes_ready:
>  	copied = sk_msg_recvmsg(sk, psock, msg, len, flags);
>  	if (!copied) {
>  		int data;
>  
> +		if (sock_flag(sk, SOCK_DONE)) {
> +			ret = 0;
> +			goto unlock;
> +		}
> +
> +		if (sk->sk_err) {
> +			if (!sk_psock_queue_empty(psock))
> +				goto msg_bytes_ready;
> +			if (!skb_queue_empty(&sk->sk_receive_queue)) {
> +				release_sock(sk);
> +				sk_psock_put(sk, psock);
> +				return tcp_recvmsg(sk, msg, len, flags);
> +			}
> +			ret = sock_error(sk);
> +			goto unlock;
> +		}
> +
> +		if (sk->sk_shutdown & RCV_SHUTDOWN) {
> +			if (!sk_psock_queue_empty(psock))
> +				goto msg_bytes_ready;

[Severity: High]
This is a pre-existing issue in tcp_bpf_recvmsg_parser(), but this patch
copies the same logic into tcp_bpf_recvmsg(). Does this create an unkillable
infinite loop if the user calls recv() with a length of 0?

If a user waits for data to be queued and ensures an error state is set (for
example, the peer sends a FIN setting RCV_SHUTDOWN), and then calls
recv(fd, buf, 0, 0), sk_msg_recvmsg() immediately returns 0 since len is 0.

The code enters the if (!copied) block, hits the RCV_SHUTDOWN check, and
because !sk_psock_queue_empty(psock) is true, it executes goto
msg_bytes_ready.

Since len is 0, the cycle repeats immediately without releasing the socket
lock or checking for signals, pegging the CPU and potentially causing a
soft lockup. Is there a way to safely break out of this loop when len is 0?

> +			if (!skb_queue_empty(&sk->sk_receive_queue)) {
> +				release_sock(sk);
> +				sk_psock_put(sk, psock);
> +				return tcp_recvmsg(sk, msg, len, flags);
> +			}
> +			ret = 0;
> +			goto unlock;
> +		}

[ ... ]

> @@ -405,6 +454,8 @@ static int tcp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
>  			sk_psock_put(sk, psock);
>  			return tcp_recvmsg(sk, msg, len, flags);
>  		}
> +		if (!data && timeo > 0)
> +			goto msg_bytes_ready;
>  		copied = -EAGAIN;
>  	}
>  	ret = copied;

[Severity: High]
This is a pre-existing issue, but does tcp_bpf_recvmsg() propagate an
incorrect -EFAULT to userspace when receiving a 0-length message?

If a BPF verdict program redirects a 0-length packet (such as an empty TCP
FIN) to a sockmap socket, it is queued as an sk_msg with sge->length == 0.

When the user calls recv(), __sk_msg_recvmsg() in net/core/skmsg.c
processes the element, sets copy = sge->length (which is 0), and bypasses
copy_page_to_iter(). It then evaluates if (!copy).

Since copy is 0, it triggers copied = copied ? copied : -EFAULT, returning
-EFAULT.

Because -EFAULT is not 0, tcp_bpf_recvmsg() bypasses the if (!copied) error
handling block above and directly assigns ret = copied, returning the
-EFAULT back to the syscall caller. Should this case return EOF (0) instead
of -EFAULT?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717155348.54975-1-nnamdio@gmail.com?part=1

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v5 2/2] selftest/bpf: added selftest sockmap_basic/sockmap recvfrom eagain
  2026-07-17 15:53 ` [PATCH v5 2/2] selftest/bpf: added selftest sockmap_basic/sockmap recvfrom eagain Nnamdi Onyeyiri
  2026-07-17 16:36   ` bot+bpf-ci
@ 2026-07-17 17:29   ` sashiko-bot
  1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-07-17 17:29 UTC (permalink / raw)
  To: Nnamdi Onyeyiri; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The test silently ignores non-EAGAIN errors from recvfrom(), incorrectly treating socket failures or premature EOFs as a test success.
- [Medium] Test suite hangs indefinitely if the receiver thread encounters an error during recvfrom() and exits early.
--

--- Patch [2]: [PATCH v5 2/2] selftest/bpf: added selftest sockmap_basic/sockmap recvfrom eagain ---
commit 5b1a8047d6f2573ba61a579d78502e3442c615cc
Author: Nnamdi Onyeyiri <nnamdio@gmail.com>

selftest/bpf: added selftest sockmap_basic/sockmap recvfrom eagain

This commit adds a selftest to ensure that tcp_bpf_recvmsg() and
tcp_bpf_recvmsg_parser() correctly handle spurious wakeups in
tcp_msg_wait_data(). It verifies that recvfrom() does not incorrectly return
EAGAIN.

> diff --git a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> index e5fc038d747b6..c38d1514e2533 100644
> --- a/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c
> +++ b/tools/testing/selftests/bpf/prog_tests/sockmap_basic.c

[ ... ]

> @@ -1410,6 +1410,117 @@ static void test_sockmap_no_verdict_fionread(void)
>  	test_sockmap_pass_prog__destroy(skel);
>  }
>  
> +static void *test_sockmap_recvfrom_eagain_thread(void *arg)
> +{
> +	int fd = *(int *)arg;
> +	char buf[1024];
> +	void *result = NULL;
> +
> +	while (true) {
> +		ssize_t len = recvfrom(fd, buf, sizeof(buf), 0, NULL, NULL);
> +
> +		if (len == -1) {
> +			if (errno == EINTR)
> +				continue;
> +			result = (void *)1;
> +			break;
> +		}
> +
> +		if (!len || buf[len - 1] == 'e')
> +			break;
> +	}
> +
> +	send(fd, "test", 4, MSG_NOSIGNAL);
> +
> +	return result;

[Severity: Medium]
If the receiver thread encounters an unexpected error (e.g., -1 from recvfrom),
it breaks out of the loop and exits without closing the file descriptor fd.

Meanwhile, the main thread attempts to send 25MB of data via a blocking xsend().
Since fd remains open but is no longer being read, the TCP receive window fills
up, and the main thread will block indefinitely in xsend().

Should the socket fd be closed before returning to prevent the test suite from
hanging?

> +}

[ ... ]

> +		while (rem) {
> +			sent = xsend(c, buf + (buflen - rem), rem, 0);
> +			if (sent == -1)
> +				goto end_attempt;
> +			rem -= sent;
> +		}
> +
> +		err = recvfrom(c, ignored, sizeof(ignored), 0, NULL, NULL);
> +		if (err < 0 && !ASSERT_NEQ(errno, EAGAIN, "recvfrom eagain"))
> +			goto end_attempt;
> +
> +		success = true;

[Severity: Medium]
Does this logic silently ignore errors from recvfrom() when errno is not EAGAIN?

If recvfrom() fails with an error other than EAGAIN, ASSERT_NEQ(errno, EAGAIN)
evaluates to true. The logical NOT operator negates this to false, causing the
if condition to fail.

The code then falls through to success = true, which treats the failure as a
successful test pass.

> +
> +end_attempt:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717155348.54975-1-nnamdio@gmail.com?part=2

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-07-17 17:29 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-17 15:53 [PATCH v5 0/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup Nnamdi Onyeyiri
2026-07-17 15:53 ` [PATCH v5 1/2] " Nnamdi Onyeyiri
2026-07-17 17:01   ` sashiko-bot
2026-07-17 15:53 ` [PATCH v5 2/2] selftest/bpf: added selftest sockmap_basic/sockmap recvfrom eagain Nnamdi Onyeyiri
2026-07-17 16:36   ` bot+bpf-ci
2026-07-17 17:29   ` sashiko-bot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox