[PATCH net] net: datagram: fix data-races in datagram_poll()

[PATCH net] net: datagram: fix data-races in datagram_poll()

[Eric Dumazet]

datagram_poll() runs locklessly, we should add READ_ONCE()
annotations while reading sk->sk_err, sk->sk_shutdown and sk->sk_state.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/core/datagram.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/net/core/datagram.c b/net/core/datagram.c
index 5662dff3d381a92b271d9cba38a28a6a8478c114..176eb58347461b160890ce2d6b2d3cbc7412e321 100644
--- a/net/core/datagram.c
+++ b/net/core/datagram.c
@@ -807,18 +807,21 @@ __poll_t datagram_poll(struct file *file, struct socket *sock,
 {
 	struct sock *sk = sock->sk;
 	__poll_t mask;
+	u8 shutdown;
 
 	sock_poll_wait(file, sock, wait);
 	mask = 0;
 
 	/* exceptional events? */
-	if (sk->sk_err || !skb_queue_empty_lockless(&sk->sk_error_queue))
+	if (READ_ONCE(sk->sk_err) ||
+	    !skb_queue_empty_lockless(&sk->sk_error_queue))
 		mask |= EPOLLERR |
 			(sock_flag(sk, SOCK_SELECT_ERR_QUEUE) ? EPOLLPRI : 0);
 
-	if (sk->sk_shutdown & RCV_SHUTDOWN)
+	shutdown = READ_ONCE(sk->sk_shutdown);
+	if (shutdown & RCV_SHUTDOWN)
 		mask |= EPOLLRDHUP | EPOLLIN | EPOLLRDNORM;
-	if (sk->sk_shutdown == SHUTDOWN_MASK)
+	if (shutdown == SHUTDOWN_MASK)
 		mask |= EPOLLHUP;
 
 	/* readable? */
@@ -827,10 +830,12 @@ __poll_t datagram_poll(struct file *file, struct socket *sock,
 
 	/* Connection-based need to check for termination and startup */
 	if (connection_based(sk)) {
-		if (sk->sk_state == TCP_CLOSE)
+		int state = READ_ONCE(sk->sk_state);
+
+		if (state == TCP_CLOSE)
 			mask |= EPOLLHUP;
 		/* connection hasn't started yet? */
-		if (sk->sk_state == TCP_SYN_SENT)
+		if (state == TCP_SYN_SENT)
 			return mask;
 	}
 
-- 
2.40.1.521.gf1e218fcd8-goog

Re: [PATCH net] net: datagram: fix data-races in datagram_poll()

[Kuniyuki Iwashima]

From: Eric Dumazet <edumazet@google.com>
Date: Tue,  9 May 2023 17:31:31 +0000
> datagram_poll() runs locklessly, we should add READ_ONCE()
> annotations while reading sk->sk_err, sk->sk_shutdown and sk->sk_state.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@amazon.com>


> ---
>  net/core/datagram.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/net/core/datagram.c b/net/core/datagram.c
> index 5662dff3d381a92b271d9cba38a28a6a8478c114..176eb58347461b160890ce2d6b2d3cbc7412e321 100644
> --- a/net/core/datagram.c
> +++ b/net/core/datagram.c
> @@ -807,18 +807,21 @@ __poll_t datagram_poll(struct file *file, struct socket *sock,
>  {
>  	struct sock *sk = sock->sk;
>  	__poll_t mask;
> +	u8 shutdown;
>  
>  	sock_poll_wait(file, sock, wait);
>  	mask = 0;
>  
>  	/* exceptional events? */
> -	if (sk->sk_err || !skb_queue_empty_lockless(&sk->sk_error_queue))
> +	if (READ_ONCE(sk->sk_err) ||
> +	    !skb_queue_empty_lockless(&sk->sk_error_queue))
>  		mask |= EPOLLERR |
>  			(sock_flag(sk, SOCK_SELECT_ERR_QUEUE) ? EPOLLPRI : 0);
>  
> -	if (sk->sk_shutdown & RCV_SHUTDOWN)
> +	shutdown = READ_ONCE(sk->sk_shutdown);
> +	if (shutdown & RCV_SHUTDOWN)
>  		mask |= EPOLLRDHUP | EPOLLIN | EPOLLRDNORM;
> -	if (sk->sk_shutdown == SHUTDOWN_MASK)
> +	if (shutdown == SHUTDOWN_MASK)
>  		mask |= EPOLLHUP;
>  
>  	/* readable? */
> @@ -827,10 +830,12 @@ __poll_t datagram_poll(struct file *file, struct socket *sock,
>  
>  	/* Connection-based need to check for termination and startup */
>  	if (connection_based(sk)) {
> -		if (sk->sk_state == TCP_CLOSE)
> +		int state = READ_ONCE(sk->sk_state);
> +
> +		if (state == TCP_CLOSE)
>  			mask |= EPOLLHUP;
>  		/* connection hasn't started yet? */
> -		if (sk->sk_state == TCP_SYN_SENT)
> +		if (state == TCP_SYN_SENT)
>  			return mask;
>  	}
>  
> -- 
> 2.40.1.521.gf1e218fcd8-goog

Re: [PATCH net] net: datagram: fix data-races in datagram_poll()

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue,  9 May 2023 17:31:31 +0000 you wrote:
> datagram_poll() runs locklessly, we should add READ_ONCE()
> annotations while reading sk->sk_err, sk->sk_shutdown and sk->sk_state.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>  net/core/datagram.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)

Here is the summary with links:
  - [net] net: datagram: fix data-races in datagram_poll()
    https://git.kernel.org/netdev/net/c/5bca1d081f44

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html