| Message ID | 20230509173131.3263780-1-edumazet@google.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 5bca1d081f44c9443e61841842ce4e9179d327b6 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] net: datagram: fix data-races in datagram_poll() | expand |
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
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!
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; }
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(-)