netdev - Re: [PATCH net] net: datagram: fix data-races in datagram

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20230509225031.19553-1-kuniyu@amazon.com>
Date: Tue, 9 May 2023 15:50:31 -0700
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <edumazet@...gle.com>
CC: <davem@...emloft.net>, <eric.dumazet@...il.com>, <kuba@...nel.org>,
	<netdev@...r.kernel.org>, <pabeni@...hat.com>, <kuniyu@...zon.com>
Subject: Re: [PATCH net] net: datagram: fix data-races in datagram_poll()

From: Eric Dumazet <edumazet@...gle.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@...gle.com>

Reviewed-by: Kuniyuki Iwashima <kuniyu@...zon.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