Re: [PATCH bpf-next 1/2] bpf, sockmap: Fix udp_bpf_recvmsg() spinning on backlog-only ingress

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf, sockmap: udp_bpf_recvmsg deadlocks on data in ingress_skb
--

commit 4b047bd002c9028eba9dda4d7ee00341f2525fa3
Author: Yuan Chen <chenyuan@kylinos.cn>

bpf, sockmap: Fix udp_bpf_recvmsg() spinning on backlog-only ingress
    
udp_bpf_recvmsg() re-arms its msg_bytes_ready loop whenever psock_has_data()
is true, but that predicate also covers an skb parked in psock->ingress_skb.
This commit changes it to re-arm only on !sk_psock_queue_empty(psock).

> diff --git a/net/ipv4/udp_bpf.c b/net/ipv4/udp_bpf.c
> index ad57c4c9eaab6..8aca9fb893349 100644
> --- a/net/ipv4/udp_bpf.c
> +++ b/net/ipv4/udp_bpf.c
> @@ -91,7 +91,7 @@ static int udp_bpf_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
>  		timeo = sock_rcvtimeo(sk, flags & MSG_DONTWAIT);
>  		data = udp_msg_wait_data(sk, psock, timeo);
>  		if (data) {
> -			if (psock_has_data(psock))
> +			if (!sk_psock_queue_empty(psock))
>  				goto msg_bytes_ready;
>  
>  			release_sock(sk);

[Severity: High]
Does this introduce a deadlock when data is parked in ingress_skb?

If a BPF program redirects UDP data into a socket's ingress_skb,
udp_msg_wait_data() will return 1 immediately because it relies on
udp_msg_has_data(), which evaluates to true when ingress_skb has data.

With this patch, the execution then strictly checks sk_psock_queue_empty().
Because this function only evaluates ingress_msg, it will return true when
ingress_msg is empty. This causes the if condition to evaluate to false,
routing the code to sk_udp_recvmsg():

    release_sock(sk);
    ret = sk_udp_recvmsg(sk, msg, len, flags);
    goto out;

When sk_udp_recvmsg() is called, it eventually sleeps in __skb_recv_udp()
waiting for data in sk_receive_queue. It completely ignores data parked in
the BPF ingress_msg queue. 

When the backlog workqueue eventually moves the data to ingress_msg and wakes
the socket, __skb_recv_udp() will ignore ingress_msg, find sk_receive_queue
still empty, and go back to sleep indefinitely. 

Could this cause the receiving thread to hang permanently in recv(), leading
to a denial of service?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929083447.558818-1-chenyuan_fl@163.com?part=1