Re: [PATCH] net: datagram: Drain queue before reporting EOF or ENOTCONN

[Kuniyuki Iwashima <kuniyu@google.com>]

From: Petr Malat <oss@malat.biz>
Date: Mon, 27 Apr 2026 22:23:33 -0700
> If a packet is queued and RCV_SHUTDOWN flag is set after the function
> __skb_wait_for_more_packets() checked the queue, the function returns
> EOF, which is then propagated by __unix_dgram_recvmsg() and the user
> reads EOF although there is a message or messages still pending.
> 
> The function should check if the queue is empty before returning EOF.
> As the same is true for disconnect and it's also reasonable for a pending
> signal, check in a common place before returning from the function.
> 
> Signed-off-by: Petr Malat <oss@malat.biz>
> ---
>  net/core/datagram.c | 38 +++++++++++++++++++++-----------------
>  1 file changed, 21 insertions(+), 17 deletions(-)
> 
> diff --git a/net/core/datagram.c b/net/core/datagram.c
> index c285c6465923..5952950f7233 100644
> --- a/net/core/datagram.c
> +++ b/net/core/datagram.c
> @@ -98,40 +98,44 @@ int __skb_wait_for_more_packets(struct sock *sk,
> struct sk_buff_head *queue,
>  	/* Socket errors? */
>  	error = sock_error(sk);
>  	if (error)
> -		goto out_err;
> +		goto out;
> 
>  	if (READ_ONCE(queue->prev) != skb)
>  		goto out;
> 
>  	/* Socket shut down? */
> -	if (sk->sk_shutdown & RCV_SHUTDOWN)
> -		goto out_noerr;
> +	if (sk->sk_shutdown & RCV_SHUTDOWN) {
> +		error = 1;
> +		goto check_queue;

We already have checked the same condition just above, and this
is a matter of timing.

Even after the duplicated check is evaluated to false, there is
a small chance that the concurrent sendmsg() enqueues a new skb.

Considering __skb_wait_for_more_packets() is called only when
the queue is empty, it's not worth another round when shutdown()ed.


> +	}
> 
>  	/* Sequenced packets can come disconnected.
>  	 * If so we report the problem
>  	 */
> -	error = -ENOTCONN;
>  	if (connection_based(sk) &&
> -	    !(sk->sk_state == TCP_ESTABLISHED || sk->sk_state == TCP_LISTEN))
> -		goto out_err;
> +	    !(sk->sk_state == TCP_ESTABLISHED || sk->sk_state == TCP_LISTEN)) {
> +		error = -ENOTCONN;

Also, the queue is always empty if SOCK_SEQPACKET sk is at TCP_CLOSE.


> +		goto check_queue;
> +	}
> 
>  	/* handle signals */
> -	if (signal_pending(current))
> -		goto interrupted;
> +	if (signal_pending(current)) {
> +		error = sock_intr_errno(*timeo_p);
> +		goto check_queue;

and we don't want to delay signal if it arrived first.


> +	}
> 
> -	error = 0;
>  	*timeo_p = schedule_timeout(*timeo_p);
>  out:
> +	*err = error < 0 ? error : 0;
>  	finish_wait(sk_sleep(sk), &wait);
>  	return error;
> -interrupted:
> -	error = sock_intr_errno(*timeo_p);
> -out_err:
> -	*err = error;
> -	goto out;
> -out_noerr:
> -	*err = 0;
> -	error = 1;
> +check_queue:
> +	/* A packet may have arrived between the initial queue check and any
> +	 * of the early-exit conditions above.  Return 0 to let the caller
> +	 * drain the queue before acting on the shutdown / disconnect / signal.
> +	 */
> +	if (READ_ONCE(queue->prev) != skb)
> +		error = 0;
>  	goto out;
>  }
>  EXPORT_SYMBOL(__skb_wait_for_more_packets);
> -- 
> 2.47.3
>