[BUG] bpf, sockmap: spurious wakeup by tcp_msg_wait_data() causing unexpected EAGAIN in recvfrom()

[BUG] bpf, sockmap: spurious wakeup by tcp_msg_wait_data() causing unexpected EAGAIN in recvfrom()

[Nnamdi Onyeyiri]

Hi,

We've encounted what appears to be a bug with bpf when invoking recvfrom() on
an ipv4 tcp socket that has been added to a sockmap.  It results in unexpected
EAGAIN errors, that we've diagnosed as the result of spurious wakeups from
tcp_msg_wait_data().

This has been confirmed to still be present on the mainline kernel, and I have
written a reproducer at: https://github.com/Nnamdi/recvfrom_sockmap_eagain/

Attched is a patch that we found resolved this using kpatch locally.  It just
causes spurious wakups to go round the loop again.  I'd like to get a sense of
whether this is expected behaviour, or really is a bug, in which case, is this
the correct fix?

Thanks,
Nnamdi.

----8<----
From 0e0c342363b2e435297ab1feda402cde6ad54525 Mon Sep 17 00:00:00 2001
From: Nnamdi Onyeyiri <nnamdio@gmail.com>
Date: Thu, 9 Jul 2026 13:06:33 +0100
Subject: [PATCH] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup

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.

this happens when tcp_msg_wait_data() wakes spuriously (returning 0) in which
case, if no receive timeout is configured, we loop again instead of returning
-EAGAIN.

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

diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index cc0bd73f3..38fd391ff 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -317,6 +317,8 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
 		}
 		if (data && !sk_psock_queue_empty(psock))
 			goto msg_bytes_ready;
+		if (!data && timeo == MAX_SCHEDULE_TIMEOUT)
+			goto msg_bytes_ready;
 		copied = -EAGAIN;
 	}
 out:
@@ -390,6 +392,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 == MAX_SCHEDULE_TIMEOUT)
+			goto msg_bytes_ready;
 		copied = -EAGAIN;
 	}
 	ret = copied;
-- 
2.52.0

Re: [BUG] bpf, sockmap: spurious wakeup by tcp_msg_wait_data() causing unexpected EAGAIN in recvfrom()

[sashiko-bot]

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

New issues:
- [Critical] The patch introduces an unkillable infinite kernel loop (DoS) in `tcp_bpf_recvmsg` when a blocking socket without a receive timeout receives a signal.

Pre-existing issues:
- [High] A race condition during stream parsing causes blocking sockets to prematurely return `-EAGAIN` when data is present.
- [High] The patch fails to fix spurious wakeups for sockets with finite receive timeouts (SO_RCVTIMEO), leaving them susceptible to premature -EAGAIN returns.
--

--- Patch [1]: [BUG] bpf, sockmap: spurious wakeup by tcp_msg_wait_data() causing unexpected EAGAIN in recvfrom() ---
commit 39fdd170f1532151400920cba0bc08398cb63cb8
Author: Nnamdi Onyeyiri <nnamdio@gmail.com>

bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup

This patch attempts to resolve unexpected EAGAIN errors on blocking
ipv4 tcp sockets added to a sockmap by looping on spurious wakeups.
It adds checks in tcp_bpf_recvmsg and tcp_bpf_recvmsg_parser to
jump back to msg_bytes_ready when no data is found and there is no
receive timeout configured.

> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index cc0bd73f36b6d..38fd391ffdb31 100644
> --- a/net/ipv4/tcp_bpf.c
> +++ b/net/ipv4/tcp_bpf.c
> @@ -317,6 +317,8 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
>  		}
>  		if (data && !sk_psock_queue_empty(psock))
>  			goto msg_bytes_ready;

[Severity: High]
This is a pre-existing issue, but does this logic incorrectly assume that if
data is present, the parsed data is already in the psock?

If data arrives in sk_receive_queue and triggers a wakeup, but the
thread re-acquires lock_sock before the stream parser can execute and
populate psock->ingress_msg, we bypass the retry and erroneously return
-EAGAIN.

> +		if (!data && timeo == MAX_SCHEDULE_TIMEOUT)
> +			goto msg_bytes_ready;

[Severity: High]
This is a pre-existing issue, but does this skip the fix for sockets
configured with finite timeouts (SO_RCVTIMEO)?

By explicitly checking for MAX_SCHEDULE_TIMEOUT, finite timeouts will
still randomly fail with -EAGAIN on spurious wakeups.

Additionally, because tcp_msg_wait_data() takes timeo by value, the
remaining time calculated by sk_wait_event() is discarded. If this loop
were enabled for finite timeouts, it would reset the timeout on every
iteration.

[ ... ]

> @@ -390,6 +392,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 == MAX_SCHEDULE_TIMEOUT)
> +			goto msg_bytes_ready;

[Severity: Critical]
Could this new unconditional branch cause an infinite kernel loop if the
process receives a signal?

Unlike tcp_bpf_recvmsg_parser(), tcp_bpf_recvmsg() lacks a
signal_pending(current) check inside its loop. If a blocking socket
without a timeout receives a signal, sk_wait_event() returns immediately
without sleeping. 

The new check then jumps back to msg_bytes_ready, causing the kernel 
to spin indefinitely without delivering the signal, resulting in a 
denial of service.

>  		copied = -EAGAIN;
>  	}
>  	ret = copied;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/ak_rR-Skd8Mvn4mH@localhost.localdomain?part=1

Re: [BUG] bpf, sockmap: spurious wakeup by tcp_msg_wait_data() causing unexpected EAGAIN in recvfrom()

[Nnamdi Onyeyiri]

Hi,

The updated patch below addresses the issues raised by sashiko-bot.  The closed
socket and signal handling code was added to tcp_bpf_recvmsg(), and the fix was
updated to work for sockets with SO_RCVTIMEO set.

Please let me know if any more changes are required, or if the patch would need
to be submitted some other way, I'm happy to adjust as necessary.

Thanks!

----8<----
From 20a1275aa5ecb927ff049971454b62682356ef0e Mon Sep 17 00:00:00 2001
From: Nnamdi Onyeyiri <nnamdio@gmail.com>
Date: Fri, 10 Jul 2026 20:47:36 +0100
Subject: [PATCH v2 1/1] bpf, sockmap: handle spurious tcp_msg_wait_data()
 wakeup

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.

this happens when tcp_msg_wait_data() wakes spuriously (returning 0) in which
case, if there is no timeout, or the timeout has not yet expired, we loop
again instead of returning.

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

diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index cc0bd73f36b6..e3109edb9621 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,45 @@ 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) {
+			ret = sock_error(sk);
+			goto unlock;
+		}
+
+		if (sk->sk_shutdown & RCV_SHUTDOWN) {
+			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 +425,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

Re: [BUG] bpf, sockmap: spurious wakeup by tcp_msg_wait_data() causing unexpected EAGAIN in recvfrom()

[Jiayuan Chen]

On 7/11/26 4:17 AM, Nnamdi Onyeyiri wrote:
> Hi,
>
> The updated patch below addresses the issues raised by sashiko-bot.  The closed
> socket and signal handling code was added to tcp_bpf_recvmsg(), and the fix was
> updated to work for sockets with SO_RCVTIMEO set.
>
> Please let me know if any more changes are required, or if the patch would need
> to be submitted some other way, I'm happy to adjust as necessary.
>
> Thanks!


Thanks for the report.


What's your use case here? With a verdict prog attached, we'd normally

expect the data to be redirected in kernel rather than read back via

  recvmsg(). Are you using SK_PASS? If so, please state that in the

commit message instead of the email body.


Also, this message could be included into commit message.

>
> ----8<----
>  From 20a1275aa5ecb927ff049971454b62682356ef0e Mon Sep 17 00:00:00 2001
> From: Nnamdi Onyeyiri <nnamdio@gmail.com>
> Date: Fri, 10 Jul 2026 20:47:36 +0100
> Subject: [PATCH v2 1/1] bpf, sockmap: handle spurious tcp_msg_wait_data()
>   wakeup
>
> 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.
>
> this happens when tcp_msg_wait_data() wakes spuriously (returning 0) in which
> case, if there is no timeout, or the timeout has not yet expired, we loop
> again instead of returning.

Also please add a selftest and send both as a patchset.


>
> Signed-off-by: Nnamdi Onyeyiri <nnamdio@gmail.com>
> ---
>   net/ipv4/tcp_bpf.c | 55 ++++++++++++++++++++++++++++++++++++++--------
>   1 file changed, 46 insertions(+), 9 deletions(-)
>
> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index cc0bd73f36b6..e3109edb9621 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;

'if (!*timeo)'  is enough


>   
>   	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,45 @@ 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) {
> +			ret = sock_error(sk);
> +			goto unlock;
> +		}
> +
> +		if (sk->sk_shutdown & RCV_SHUTDOWN) {
> +			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 +425,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;

Re: [BUG] bpf, sockmap: spurious wakeup by tcp_msg_wait_data() causing unexpected EAGAIN in recvfrom()

[John Fastabend]

On Mon, Jul 13, 2026 at 12:14:07PM +0800, Jiayuan Chen wrote:
>
>On 7/11/26 4:17 AM, Nnamdi Onyeyiri wrote:
>>Hi,
>>
>>The updated patch below addresses the issues raised by sashiko-bot.  The closed
>>socket and signal handling code was added to tcp_bpf_recvmsg(), and the fix was
>>updated to work for sockets with SO_RCVTIMEO set.
>>
>>Please let me know if any more changes are required, or if the patch would need
>>to be submitted some other way, I'm happy to adjust as necessary.
>>
>>Thanks!
>
>
>Thanks for the report.
>
>
>What's your use case here? With a verdict prog attached, we'd normally
>
>expect the data to be redirected in kernel rather than read back via
>
> recvmsg(). Are you using SK_PASS? If so, please state that in the
>
>commit message instead of the email body.

For our use case we never do redirect or packet operations (push, pop)
we merely use it as a mechanism to read data and possibly drop data
if it violates some policy.

[PATCH v3 0/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup

[Nnamdi Onyeyiri]

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.

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

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

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

 net/ipv4/tcp_bpf.c                            |  55 +++-
 tools/testing/selftests/net/.gitignore        |   2 +
 tools/testing/selftests/net/Makefile          |   6 +
 .../selftests/net/sockmap_recvfrom.bpf.c      |  31 ++
 .../testing/selftests/net/sockmap_recvfrom.c  | 288 ++++++++++++++++++
 5 files changed, 373 insertions(+), 9 deletions(-)
 create mode 100644 tools/testing/selftests/net/sockmap_recvfrom.bpf.c
 create mode 100644 tools/testing/selftests/net/sockmap_recvfrom.c

-- 
2.52.0

[PATCH v3 1/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup

[Nnamdi Onyeyiri]

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 | 55 ++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 46 insertions(+), 9 deletions(-)

diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index cc0bd73f36b6..e58ab2d1ee94 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,45 @@ 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) {
+			ret = sock_error(sk);
+			goto unlock;
+		}
+
+		if (sk->sk_shutdown & RCV_SHUTDOWN) {
+			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 +425,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

Re: [PATCH v3 1/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup

[sashiko-bot]

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

New issues:
- [High] Socket state checks in `tcp_bpf_recvmsg` bypass unread data in `sk_receive_queue`, causing data loss on concurrent socket teardown or errors.

Pre-existing issues:
- [High] Sporadic EAGAIN can still occur on blocking sockets with a verdict parser due to a race with the BPF parser workqueue.
- [High] `tcp_bpf_recvmsg()` incorrectly returns -EAGAIN instead of reading redirected data after waking up.
--

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

bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup

This commit modifies tcp_bpf_recvmsg() and tcp_bpf_recvmsg_parser() to handle
spurious wakeups from tcp_msg_wait_data(). It updates sk_wait_event() to
maintain the timeout by passing it as a pointer. The commit also introduces
socket state checks into tcp_bpf_recvmsg() to match tcp_bpf_recvmsg_parser().

> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index cc0bd73f36b6d..e58ab2d1ee943 100644
> --- a/net/ipv4/tcp_bpf.c
> +++ b/net/ipv4/tcp_bpf.c

[ ... ]

> @@ -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;
>  	}

[Severity: High]
This is a pre-existing issue, but does this logic still sporadically return
-EAGAIN on blocking sockets when data == 1?

If a blocking recv() is called on a sockmap socket with a verdict parser, and a
packet arrives in sk_receive_queue, tcp_msg_wait_data() will return 1.

However, if the BPF workqueue hasn't yet processed the data into
psock->ingress_msg, sk_psock_queue_empty(psock) could still be true.

It looks like the loop fails to handle data == 1 with an empty psock, bypassing
the new !data check and falling through to return -EAGAIN.

[ ... ]

> @@ -371,14 +375,45 @@ 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;
>  
> -		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) {
> +			ret = sock_error(sk);
> +			goto unlock;
> +		}
> +
> +		if (sk->sk_shutdown & RCV_SHUTDOWN) {
> +			ret = 0;
> +			goto unlock;
> +		}

[Severity: High]
Does evaluating these state flags before falling back to tcp_recvmsg() cause
data loss?

If normal data arrives and is queued in sk_receive_queue, and a FIN
(RCV_SHUTDOWN) or RST (sk_err) arrives immediately after, recv() would see
psock->ingress_msg as empty.

It seems like this newly added code evaluates the state flags and immediately
returns 0 or an error, bypassing the fallback to tcp_recvmsg() which would have
correctly drained the unread data.

[ ... ]

>  		data = tcp_msg_wait_data(sk, psock, &timeo);
>  		if (data < 0) {
>  			ret = data;
>  			goto unlock;
>  		}
>  		if (data && !skb_queue_empty_lockless(&sk->sk_receive_queue)) {
>  			sk_psock_put(sk, psock);
>  			return tcp_recvmsg(sk, msg, len, flags);
>  		}
> +		if (!data && timeo > 0)
> +			goto msg_bytes_ready;
>  		copied = -EAGAIN;
>  	}

[Severity: High]
This is a pre-existing issue, but could this return -EAGAIN instead of reading
redirected data?

If data == 1 because of redirected data in psock->ingress_msg, but
sk_receive_queue is empty, the condition:

    if (data && !skb_queue_empty_lockless(&sk->sk_receive_queue)) {

will be false.

The next check:

    if (!data && timeo > 0)

will also be false.

It seems the code then falls through to return -EAGAIN instead of looping back
to read the redirected data from the psock.

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

[PATCH v3 2/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup

[Nnamdi Onyeyiri]

This selftest exercises 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:

  * Without a timeout - recvfrom() does not return an EAGAIN error.

  * With a timeout - recvfrom() returns EAGAIN, but only after the
    SO_RCVTIMEO timeout has expired.

If the spurious wakeups are not correctly handled, the above assertions
fail.

Signed-off-by: Nnamdi Onyeyiri <nnamdio@gmail.com>
---
 tools/testing/selftests/net/.gitignore        |   2 +
 tools/testing/selftests/net/Makefile          |   6 +
 .../selftests/net/sockmap_recvfrom.bpf.c      |  31 ++
 .../testing/selftests/net/sockmap_recvfrom.c  | 288 ++++++++++++++++++
 4 files changed, 327 insertions(+)
 create mode 100644 tools/testing/selftests/net/sockmap_recvfrom.bpf.c
 create mode 100644 tools/testing/selftests/net/sockmap_recvfrom.c

diff --git a/tools/testing/selftests/net/.gitignore b/tools/testing/selftests/net/.gitignore
index c9f46031ac73..a1840a5d45f6 100644
--- a/tools/testing/selftests/net/.gitignore
+++ b/tools/testing/selftests/net/.gitignore
@@ -39,6 +39,8 @@ sk_connect_zero_addr
 sk_so_peek_off
 skf_net_off
 socket
+sockmap_recvfrom
+sockmap_recvfrom.bpf.o
 so_incoming_cpu
 so_netns_cookie
 so_rcv_listener
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 708d960ae07d..a05a4a924402 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -161,6 +161,7 @@ TEST_GEN_FILES := \
 	so_netns_cookie \
 	so_rcv_listener \
 	socket \
+	sockmap_recvfrom.bpf.o \
 	stress_reuseport_listen \
 	tcp_fastopen_backup_key \
 	tcp_inq \
@@ -191,6 +192,7 @@ TEST_GEN_PROGS := \
 	sk_connect_zero_addr \
 	sk_so_peek_off \
 	so_incoming_cpu \
+	sockmap_recvfrom \
 	tap \
 	tcp_port_share \
 	tls \
@@ -238,3 +240,7 @@ $(OUTPUT)/bind_bhash: LDLIBS += -lpthread
 $(OUTPUT)/io_uring_zerocopy_tx: CFLAGS += -I../../../include/
 
 include bpf.mk
+
+$(OUTPUT)/sockmap_recvfrom: $(BPFOBJ)
+$(OUTPUT)/sockmap_recvfrom: LDLIBS += $(BPFOBJ) -lelf -lz -lpthread
+$(OUTPUT)/sockmap_recvfrom: CFLAGS += -I$(OUTPUT)/tools/include
diff --git a/tools/testing/selftests/net/sockmap_recvfrom.bpf.c b/tools/testing/selftests/net/sockmap_recvfrom.bpf.c
new file mode 100644
index 000000000000..fec470c738f1
--- /dev/null
+++ b/tools/testing/selftests/net/sockmap_recvfrom.bpf.c
@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+#define AF_INET 2
+
+char LICENSE[] SEC("license") = "GPL";
+
+struct {
+	__uint(type, BPF_MAP_TYPE_SOCKHASH);
+	__uint(max_entries, 1024);
+	__type(key, __u64);
+	__type(value, __u64);
+
+} map_socks SEC(".maps");
+
+SEC("sockops") int on_sockops(struct bpf_sock_ops *ctx)
+{
+	if (ctx->family == AF_INET && ctx->op == BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB) {
+		__u64 cookie = bpf_get_socket_cookie(ctx);
+
+		bpf_sock_hash_update(ctx, &map_socks, &cookie, BPF_NOEXIST);
+	}
+
+	return 0;
+}
+
+SEC("sk_skb/stream_verdict") int on_recv(struct __sk_buff *ctx)
+{
+	return SK_PASS;
+}
diff --git a/tools/testing/selftests/net/sockmap_recvfrom.c b/tools/testing/selftests/net/sockmap_recvfrom.c
new file mode 100644
index 000000000000..f96b4673c977
--- /dev/null
+++ b/tools/testing/selftests/net/sockmap_recvfrom.c
@@ -0,0 +1,288 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <arpa/inet.h>
+#include <bpf/libbpf.h>
+#include <fcntl.h>
+#include <pthread.h>
+#include <stdatomic.h>
+#include <sys/socket.h>
+#include <sys/time.h>
+#include <unistd.h>
+
+#include "kselftest_harness.h"
+
+#define MAX_ITERATIONS 100
+#define PAYLOAD_END 'e'
+
+static int start_listening(struct __test_metadata *_metadata, uint16_t *port)
+{
+	struct sockaddr_in addr;
+	socklen_t addrlen;
+	int fd;
+
+	memset(&addr, 0, sizeof(addr));
+	addr.sin_family = AF_INET;
+	addr.sin_addr.s_addr = INADDR_ANY;
+
+	fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+
+	ASSERT_NE(fd, -1);
+	ASSERT_EQ(bind(fd, (struct sockaddr *)&addr, sizeof(addr)), 0);
+	ASSERT_EQ(listen(fd, 5), 0);
+
+	addrlen = sizeof(addr);
+
+	ASSERT_EQ(getsockname(fd, (struct sockaddr *)&addr, &addrlen), 0);
+
+	*port = addr.sin_port;
+
+	return fd;
+}
+
+static void process_client(struct __test_metadata *_metadata, int fd, atomic_int *running)
+{
+	char buf[1024];
+	struct timeval timeo;
+
+	timeo.tv_sec = 0;
+	timeo.tv_usec = 1000;
+
+	EXPECT_EQ(setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo)), 0);
+
+	while (atomic_load(running)) {
+		ssize_t len = recvfrom(fd, buf, sizeof(buf), 0, NULL, NULL);
+
+		if (len == -1) {
+			EXPECT_TRUE(errno == EAGAIN || errno == EINTR);
+			continue;
+		}
+
+		EXPECT_GE(len, 0);
+
+		if (len <= 0 || buf[len - 1] == PAYLOAD_END)
+			break;
+	}
+
+	write(fd, "test", 4);
+
+	close(fd);
+}
+
+struct bpf_t {
+	struct bpf_object *obj;
+	struct bpf_link *on_sockops;
+	struct bpf_link *on_recv;
+};
+
+static void setup_bpf(struct __test_metadata *_metadata, const char *path, bool recv,
+						struct bpf_t *bpf)
+{
+	struct bpf_program *prog;
+	int cgroup;
+
+	memset(bpf, 0, sizeof(*bpf));
+	bpf->obj = bpf_object__open_file(path, NULL);
+
+	ASSERT_NE(bpf->obj, NULL);
+	ASSERT_EQ(bpf_object__load(bpf->obj), 0);
+
+	prog = bpf_object__find_program_by_name(bpf->obj, "on_sockops");
+	ASSERT_NE(prog, NULL);
+
+	cgroup = open("/sys/fs/cgroup", O_RDONLY);
+	ASSERT_NE(cgroup, -1);
+
+	bpf->on_sockops = bpf_program__attach_cgroup(prog, cgroup);
+	close(cgroup);
+
+	ASSERT_NE(bpf->on_sockops, NULL);
+
+	if (recv) {
+		struct bpf_map *map = bpf_object__find_map_by_name(bpf->obj, "map_socks");
+
+		ASSERT_NE(map, NULL);
+
+		prog = bpf_object__find_program_by_name(bpf->obj, "on_recv");
+		ASSERT_NE(prog, NULL);
+
+		bpf->on_recv = bpf_program__attach_sockmap(prog, bpf_map__fd(map));
+		ASSERT_NE(bpf->on_recv, NULL);
+	}
+}
+
+struct server_t {
+	int fd;
+	atomic_int running;
+	pthread_t thread;
+	struct __test_metadata *metadata;
+};
+
+static void *run_server(void *arg)
+{
+	struct server_t *server = arg;
+	struct __test_metadata *_metadata = server->metadata;
+
+	while (atomic_load(&server->running)) {
+		int client_fd = accept(server->fd, NULL, NULL);
+
+		if (client_fd == -1) {
+			if (!atomic_load(&server->running))
+				break;
+
+			continue;
+		}
+
+		process_client(_metadata, client_fd, &server->running);
+	}
+
+	return NULL;
+}
+
+static int send_payload(struct __test_metadata *_metadata, int fd, const char *buf, size_t len)
+{
+	size_t remaining = len;
+
+	do {
+		ssize_t bytes = write(fd, buf + (len - remaining), remaining);
+
+		if (bytes < 0) {
+			if (errno == EINTR)
+				continue;
+
+			return -1;
+		}
+
+		remaining -= bytes;
+	} while (remaining);
+
+	return 0;
+}
+
+FIXTURE(sockmap_recvfrom)
+{
+	struct server_t server;
+	struct sockaddr_in addr;
+	struct bpf_t bpf;
+	char *payload;
+	size_t payload_len;
+};
+
+FIXTURE_VARIANT(sockmap_recvfrom)
+{
+	bool with_recv;
+};
+
+FIXTURE_VARIANT_ADD(sockmap_recvfrom, recvmsg)
+{
+	.with_recv = false
+};
+
+FIXTURE_VARIANT_ADD(sockmap_recvfrom, recvmsg_parser)
+{
+	.with_recv = true
+};
+
+FIXTURE_SETUP(sockmap_recvfrom)
+{
+	memset(&self->addr, 0, sizeof(self->addr));
+
+	self->payload_len = 1024 * 1024 * 25;
+	self->payload = malloc(self->payload_len);
+	ASSERT_NE(self->payload, NULL);
+
+	memset(self->payload, 0, self->payload_len);
+	self->payload[self->payload_len - 1] = PAYLOAD_END;
+
+	setup_bpf(_metadata, "sockmap_recvfrom.bpf.o", variant->with_recv, &self->bpf);
+	atomic_store(&self->server.running, 1);
+	self->server.fd = start_listening(_metadata, &self->addr.sin_port);
+	self->server.metadata = _metadata;
+
+	self->addr.sin_family = AF_INET;
+	self->addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+
+	pthread_create(&self->server.thread, NULL, &run_server, &self->server);
+}
+
+FIXTURE_TEARDOWN(sockmap_recvfrom)
+{
+	atomic_store(&self->server.running, 0);
+
+	if (self->server.fd) {
+		shutdown(self->server.fd, SHUT_RD);
+		close(self->server.fd);
+	}
+
+	if (self->server.thread)
+		pthread_join(self->server.thread, NULL);
+
+	free(self->payload);
+	bpf_link__destroy(self->bpf.on_sockops);
+
+	if (self->bpf.on_recv)
+		bpf_link__destroy(self->bpf.on_recv);
+
+	bpf_object__close(self->bpf.obj);
+}
+
+TEST_F(sockmap_recvfrom, no_timeout)
+{
+	char ignored[128];
+
+	for (int i = 0; i < MAX_ITERATIONS; ++i) {
+		int fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+
+		ASSERT_NE(fd, -1);
+		ASSERT_EQ(connect(fd, (struct sockaddr *)&self->addr, sizeof(self->addr)), 0);
+
+		ASSERT_EQ(send_payload(_metadata, fd, self->payload, self->payload_len), 0);
+
+		if (recvfrom(fd, ignored, sizeof(ignored), 0, NULL, NULL) < 0)
+			ASSERT_NE(errno, EAGAIN);
+
+		close(fd);
+	}
+}
+
+static int64_t to_nanos(struct timespec *time)
+{
+	return (time->tv_sec * 1000000000LL) + time->tv_nsec;
+}
+
+TEST_F(sockmap_recvfrom, with_timeout)
+{
+	char ignored[128];
+	struct timeval timeo;
+
+	timeo.tv_sec = 0;
+	timeo.tv_usec = 5000;
+
+	/* remove the payload end delimiter so the server never responds and recvfrom times out. */
+	self->payload[self->payload_len - 1] = 0;
+
+	for (int i = 0; i < MAX_ITERATIONS; ++i) {
+		struct timespec beg;
+		struct timespec end;
+		int err;
+
+		int fd = socket(AF_INET, SOCK_STREAM, IPPROTO_TCP);
+
+		ASSERT_NE(fd, -1);
+		ASSERT_EQ(connect(fd, (struct sockaddr *)&self->addr, sizeof(self->addr)), 0);
+
+		ASSERT_EQ(send_payload(_metadata, fd, self->payload, self->payload_len), 0);
+
+		ASSERT_EQ(setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo)), 0);
+
+		clock_gettime(CLOCK_MONOTONIC, &beg);
+		ASSERT_EQ(recvfrom(fd, ignored, sizeof(ignored), 0, NULL, NULL), -1);
+		err = errno;
+		clock_gettime(CLOCK_MONOTONIC, &end);
+
+		ASSERT_EQ(err, EAGAIN);
+		ASSERT_GE(to_nanos(&end) - to_nanos(&beg), timeo.tv_usec * 1000);
+
+		close(fd);
+	}
+}
+
+TEST_HARNESS_MAIN
-- 
2.52.0

Re: [PATCH v3 2/2] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] The test attaches its BPF cgroup program directly to the root cgroup (`/sys/fs/cgroup`), lacking isolation and risking system-wide side-effects.
- [Medium] Unchecked `pthread_create` failure causes the test suite to hang indefinitely.
- [Medium] The test incorrectly validates the return values of libbpf attach APIs using `ASSERT_NE(..., NULL)`, which fails to detect error pointers (`ERR_PTR`).
- [Medium] The assertion checking the elapsed time against `timeo.tv_usec * 1000` is flaky due to timer granularity and jiffy alignment.
--

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

bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup

This selftest exercises the tcp_bpf_recvmsg() and tcp_bpf_recvmsg_parser()
functions, to ensure that they are properly handling spurious wakeups in
tcp_msg_wait_data().

> diff --git a/tools/testing/selftests/net/sockmap_recvfrom.c b/tools/testing/selftests/net/sockmap_recvfrom.c
> new file mode 100644
> index 0000000000000..f96b4673c977c
> --- /dev/null
> +++ b/tools/testing/selftests/net/sockmap_recvfrom.c
[ ... ]
> +static void setup_bpf(struct __test_metadata *_metadata, const char *path, bool recv,
> +						struct bpf_t *bpf)
> +{
[ ... ]
> +	cgroup = open("/sys/fs/cgroup", O_RDONLY);
> +	ASSERT_NE(cgroup, -1);
> +
> +	bpf->on_sockops = bpf_program__attach_cgroup(prog, cgroup);

[Severity: Medium]
Is it safe to attach the BPF program directly to the global root cgroup here?

Since this is a test, attaching globally to /sys/fs/cgroup impacts the
entire system and subjects unrelated background connections to the test's
stream verdict logic. This could disrupt host networking and easily
overflow the 1024 max_entries of map_socks.

Should the test create and attach to an isolated test cgroup instead?

> +	close(cgroup);
> +
> +	ASSERT_NE(bpf->on_sockops, NULL);

[Severity: Medium]
Does ASSERT_NE() properly handle libbpf error pointers here?

Modern libbpf functions like bpf_program__attach_cgroup() return an ERR_PTR,
not NULL, on failure.

If the attachment fails, ASSERT_NE(link, NULL) evaluates to true, masking
the failure. The test will proceed without the BPF programs attached and
pass vacuously.

Would ASSERT_OK_PTR() be more appropriate for checking this return value?

> +
> +	if (recv) {
> +		struct bpf_map *map = bpf_object__find_map_by_name(bpf->obj, "map_socks");
> +
> +		ASSERT_NE(map, NULL);
> +
> +		prog = bpf_object__find_program_by_name(bpf->obj, "on_recv");
> +		ASSERT_NE(prog, NULL);
> +
> +		bpf->on_recv = bpf_program__attach_sockmap(prog, bpf_map__fd(map));
> +		ASSERT_NE(bpf->on_recv, NULL);

[Severity: Medium]
Will this assertion correctly catch a failure in bpf_program__attach_sockmap()?

Like bpf_program__attach_cgroup(), this function returns an ERR_PTR on
failure, so checking against NULL will allow error pointers to bypass
this check.

> +	}
> +}
[ ... ]
> +FIXTURE_SETUP(sockmap_recvfrom)
> +{
> +	memset(&self->addr, 0, sizeof(self->addr));
> +
> +	self->payload_len = 1024 * 1024 * 25;
> +	self->payload = malloc(self->payload_len);
> +	ASSERT_NE(self->payload, NULL);
> +
> +	memset(self->payload, 0, self->payload_len);
> +	self->payload[self->payload_len - 1] = PAYLOAD_END;
> +
> +	setup_bpf(_metadata, "sockmap_recvfrom.bpf.o", variant->with_recv, &self->bpf);
> +	atomic_store(&self->server.running, 1);
> +	self->server.fd = start_listening(_metadata, &self->addr.sin_port);
> +	self->server.metadata = _metadata;
> +
> +	self->addr.sin_family = AF_INET;
> +	self->addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
> +
> +	pthread_create(&self->server.thread, NULL, &run_server, &self->server);

[Severity: Medium]
What happens if pthread_create() fails here?

If the server thread fails to start, it looks like the main thread will
subsequently call send_payload() with a 25MB payload. Because no server
thread is calling accept() or reading the data, the TCP receive window
will fill up and the main thread's write() will block forever.

Should we check the return value of pthread_create() to prevent the
test suite from hanging indefinitely on resource limits?

> +}
[ ... ]
> +TEST_F(sockmap_recvfrom, with_timeout)
> +{
[ ... ]
> +		ASSERT_EQ(setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &timeo, sizeof(timeo)), 0);
> +
> +		clock_gettime(CLOCK_MONOTONIC, &beg);
> +		ASSERT_EQ(recvfrom(fd, ignored, sizeof(ignored), 0, NULL, NULL), -1);
> +		err = errno;
> +		clock_gettime(CLOCK_MONOTONIC, &end);
> +
> +		ASSERT_EQ(err, EAGAIN);
> +		ASSERT_GE(to_nanos(&end) - to_nanos(&beg), timeo.tv_usec * 1000);

[Severity: Medium]
Is this strict time assertion susceptible to jiffy alignment flakiness?

The kernel handles SO_RCVTIMEO timeouts in units of jiffies. When
requesting a 5000us timeout on a system with HZ=1000, it converts to 5
ticks. Because the sleep can start just prior to a tick boundary, the
actual elapsed time can be slightly less than the absolute 5000us requested.

Could this cause intermittent test failures where the elapsed time is just
under the requested 5000000ns?

> +
> +		close(fd);
> +	}
> +}

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

Re: [BUG] bpf, sockmap: spurious wakeup by tcp_msg_wait_data() causing unexpected EAGAIN in recvfrom()

[John Fastabend]

On Thu, Jul 09, 2026 at 07:43:56PM +0100, Nnamdi Onyeyiri wrote:
>Hi,
>
>We've encounted what appears to be a bug with bpf when invoking recvfrom() on
>an ipv4 tcp socket that has been added to a sockmap.  It results in unexpected
>EAGAIN errors, that we've diagnosed as the result of spurious wakeups from
>tcp_msg_wait_data().
>
>This has been confirmed to still be present on the mainline kernel, and I have
>written a reproducer at: https://github.com/Nnamdi/recvfrom_sockmap_eagain/
>
>Attched is a patch that we found resolved this using kpatch locally.  It just
>causes spurious wakups to go round the loop again.  I'd like to get a sense of
>whether this is expected behaviour, or really is a bug, in which case, is this
>the correct fix?

If its breaking/changing applications I think its a bug. Can you submit
the patch below? We had something similar with ioctl FIONREAD behavior
as well that was recently fixed.

Thanks,
John

>
>Thanks,
>Nnamdi.
>
>----8<----
>From 0e0c342363b2e435297ab1feda402cde6ad54525 Mon Sep 17 00:00:00 2001
>From: Nnamdi Onyeyiri <nnamdio@gmail.com>
>Date: Thu, 9 Jul 2026 13:06:33 +0100
>Subject: [PATCH] bpf, sockmap: handle spurious tcp_msg_wait_data() wakeup
>
>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.
>
>this happens when tcp_msg_wait_data() wakes spuriously (returning 0) in which
>case, if no receive timeout is configured, we loop again instead of returning
>-EAGAIN.
>
>Signed-off-by: Nnamdi Onyeyiri <nnamdio@gmail.com>
>---
> net/ipv4/tcp_bpf.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
>diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
>index cc0bd73f3..38fd391ff 100644
>--- a/net/ipv4/tcp_bpf.c
>+++ b/net/ipv4/tcp_bpf.c
>@@ -317,6 +317,8 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
> 		}
> 		if (data && !sk_psock_queue_empty(psock))
> 			goto msg_bytes_ready;
>+		if (!data && timeo == MAX_SCHEDULE_TIMEOUT)
>+			goto msg_bytes_ready;
> 		copied = -EAGAIN;
> 	}
> out:
>@@ -390,6 +392,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 == MAX_SCHEDULE_TIMEOUT)
>+			goto msg_bytes_ready;
> 		copied = -EAGAIN;
> 	}
> 	ret = copied;
>-- 
>2.52.0
>

Re: [BUG] bpf, sockmap: spurious wakeup by tcp_msg_wait_data() causing unexpected EAGAIN in recvfrom()

[Nnamdi Onyeyiri]

On Tue, Jul 14, 2026 at 11:16:27AM -0700, John Fastabend wrote:
> If its breaking/changing applications I think its a bug. Can you submit
> the patch below? We had something similar with ioctl FIONREAD behavior
> as well that was recently fixed.
> 
> Thanks,
> John

Yes, this was first discovered by a python application breaking (an
EAGAIN error causes a BlockingIOError exception).  I have submitted the change
as a patchset here: https://patch.msgid.link/20260714203927.32289-1-nnamdio@gmail.com

This is my first time submitting a patch, so I wasn't sure if the selftest belongs
in bpf or net.  I've put it in net.  Please let me know if this should be moved.

Thanks,
Nnamdi.