* [MPTCP] [PATCH net] inet_connection_sock: clear inet_num out of destroy helper
@ 2020-06-04 16:55 ` Paolo Abeni
0 siblings, 0 replies; 6+ messages in thread
From: Paolo Abeni @ 2020-06-04 16:55 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 1827 bytes --]
Clearing the 'inet_num' field is necessary and safe if and
only if the socket is not bound. The MPTCP protocol calls
the destroy helper on bound sockets, as tcp_v{4,6}_syn_recv_sock
completed successfully.
Move the clearing of such field out of the common code, otherwise
the MPTCP MP_JOIN error path will find the wrong 'inet_num' value
on socket disposal, __inet_put_port() will acquire the wrong lock
and bind_node removal could race with other modifiers possibly
corrupting the bind hash table.
Reported-and-tested-by: Christoph Paasch <cpaasch(a)apple.com>
Fixes: 729cd6436f35 ("mptcp: cope better with MP_JOIN failure")
Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
---
include/net/inet_connection_sock.h | 1 -
net/ipv4/inet_connection_sock.c | 1 +
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
index 2f1f8c3efb26..e5b388f5fa20 100644
--- a/include/net/inet_connection_sock.h
+++ b/include/net/inet_connection_sock.h
@@ -292,7 +292,6 @@ static inline void inet_csk_prepare_for_destroy_sock(struct sock *sk)
/* The below has to be done to allow calling inet_csk_destroy_sock */
sock_set_flag(sk, SOCK_DEAD);
percpu_counter_inc(sk->sk_prot->orphan_count);
- inet_sk(sk)->inet_num = 0;
}
void inet_csk_destroy_sock(struct sock *sk);
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index f40b1b72f979..afaf582a5aa9 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -902,6 +902,7 @@ void inet_csk_prepare_forced_close(struct sock *sk)
bh_unlock_sock(sk);
sock_put(sk);
inet_csk_prepare_for_destroy_sock(sk);
+ inet_sk(sk)->inet_num = 0;
}
EXPORT_SYMBOL(inet_csk_prepare_forced_close);
--
2.21.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net] inet_connection_sock: clear inet_num out of destroy helper
@ 2020-06-04 16:55 ` Paolo Abeni
0 siblings, 0 replies; 6+ messages in thread
From: Paolo Abeni @ 2020-06-04 16:55 UTC (permalink / raw)
To: netdev
Cc: Mat Martineau, David S. Miller, Jakub Kicinski, mptcp,
Eric Dumazet, Christoph Paasch
Clearing the 'inet_num' field is necessary and safe if and
only if the socket is not bound. The MPTCP protocol calls
the destroy helper on bound sockets, as tcp_v{4,6}_syn_recv_sock
completed successfully.
Move the clearing of such field out of the common code, otherwise
the MPTCP MP_JOIN error path will find the wrong 'inet_num' value
on socket disposal, __inet_put_port() will acquire the wrong lock
and bind_node removal could race with other modifiers possibly
corrupting the bind hash table.
Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Fixes: 729cd6436f35 ("mptcp: cope better with MP_JOIN failure")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
include/net/inet_connection_sock.h | 1 -
net/ipv4/inet_connection_sock.c | 1 +
2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
index 2f1f8c3efb26..e5b388f5fa20 100644
--- a/include/net/inet_connection_sock.h
+++ b/include/net/inet_connection_sock.h
@@ -292,7 +292,6 @@ static inline void inet_csk_prepare_for_destroy_sock(struct sock *sk)
/* The below has to be done to allow calling inet_csk_destroy_sock */
sock_set_flag(sk, SOCK_DEAD);
percpu_counter_inc(sk->sk_prot->orphan_count);
- inet_sk(sk)->inet_num = 0;
}
void inet_csk_destroy_sock(struct sock *sk);
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index f40b1b72f979..afaf582a5aa9 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -902,6 +902,7 @@ void inet_csk_prepare_forced_close(struct sock *sk)
bh_unlock_sock(sk);
sock_put(sk);
inet_csk_prepare_for_destroy_sock(sk);
+ inet_sk(sk)->inet_num = 0;
}
EXPORT_SYMBOL(inet_csk_prepare_forced_close);
--
2.21.3
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [MPTCP] Re: [PATCH net] inet_connection_sock: clear inet_num out of destroy helper
2020-06-04 16:55 ` Paolo Abeni
@ 2020-06-04 17:43 ` Eric Dumazet
-1 siblings, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2020-06-04 17:43 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 833 bytes --]
On 6/4/20 9:55 AM, Paolo Abeni wrote:
> Clearing the 'inet_num' field is necessary and safe if and
> only if the socket is not bound. The MPTCP protocol calls
> the destroy helper on bound sockets, as tcp_v{4,6}_syn_recv_sock
> completed successfully.
>
> Move the clearing of such field out of the common code, otherwise
> the MPTCP MP_JOIN error path will find the wrong 'inet_num' value
> on socket disposal, __inet_put_port() will acquire the wrong lock
> and bind_node removal could race with other modifiers possibly
> corrupting the bind hash table.
>
> Reported-and-tested-by: Christoph Paasch <cpaasch(a)apple.com>
> Fixes: 729cd6436f35 ("mptcp: cope better with MP_JOIN failure")
> Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
> ---
Reviewed-by: Eric Dumazet <edumazet(a)google.com>
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] inet_connection_sock: clear inet_num out of destroy helper
@ 2020-06-04 17:43 ` Eric Dumazet
0 siblings, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2020-06-04 17:43 UTC (permalink / raw)
To: Paolo Abeni, netdev
Cc: Mat Martineau, David S. Miller, Jakub Kicinski, mptcp,
Eric Dumazet, Christoph Paasch
On 6/4/20 9:55 AM, Paolo Abeni wrote:
> Clearing the 'inet_num' field is necessary and safe if and
> only if the socket is not bound. The MPTCP protocol calls
> the destroy helper on bound sockets, as tcp_v{4,6}_syn_recv_sock
> completed successfully.
>
> Move the clearing of such field out of the common code, otherwise
> the MPTCP MP_JOIN error path will find the wrong 'inet_num' value
> on socket disposal, __inet_put_port() will acquire the wrong lock
> and bind_node removal could race with other modifiers possibly
> corrupting the bind hash table.
>
> Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
> Fixes: 729cd6436f35 ("mptcp: cope better with MP_JOIN failure")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
Reviewed-by: Eric Dumazet <edumazet@google.com>
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [MPTCP] Re: [PATCH net] inet_connection_sock: clear inet_num out of destroy helper
2020-06-04 16:55 ` Paolo Abeni
@ 2020-06-04 23:00 ` David Miller
-1 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2020-06-04 23:00 UTC (permalink / raw)
To: mptcp
[-- Attachment #1: Type: text/plain, Size: 825 bytes --]
From: Paolo Abeni <pabeni(a)redhat.com>
Date: Thu, 4 Jun 2020 18:55:45 +0200
> Clearing the 'inet_num' field is necessary and safe if and
> only if the socket is not bound. The MPTCP protocol calls
> the destroy helper on bound sockets, as tcp_v{4,6}_syn_recv_sock
> completed successfully.
>
> Move the clearing of such field out of the common code, otherwise
> the MPTCP MP_JOIN error path will find the wrong 'inet_num' value
> on socket disposal, __inet_put_port() will acquire the wrong lock
> and bind_node removal could race with other modifiers possibly
> corrupting the bind hash table.
>
> Reported-and-tested-by: Christoph Paasch <cpaasch(a)apple.com>
> Fixes: 729cd6436f35 ("mptcp: cope better with MP_JOIN failure")
> Signed-off-by: Paolo Abeni <pabeni(a)redhat.com>
Applied, thanks Paolo.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net] inet_connection_sock: clear inet_num out of destroy helper
@ 2020-06-04 23:00 ` David Miller
0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2020-06-04 23:00 UTC (permalink / raw)
To: pabeni; +Cc: netdev, mathew.j.martineau, kuba, mptcp, edumazet, cpaasch
From: Paolo Abeni <pabeni@redhat.com>
Date: Thu, 4 Jun 2020 18:55:45 +0200
> Clearing the 'inet_num' field is necessary and safe if and
> only if the socket is not bound. The MPTCP protocol calls
> the destroy helper on bound sockets, as tcp_v{4,6}_syn_recv_sock
> completed successfully.
>
> Move the clearing of such field out of the common code, otherwise
> the MPTCP MP_JOIN error path will find the wrong 'inet_num' value
> on socket disposal, __inet_put_port() will acquire the wrong lock
> and bind_node removal could race with other modifiers possibly
> corrupting the bind hash table.
>
> Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
> Fixes: 729cd6436f35 ("mptcp: cope better with MP_JOIN failure")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Applied, thanks Paolo.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-06-04 23:00 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-04 16:55 [MPTCP] [PATCH net] inet_connection_sock: clear inet_num out of destroy helper Paolo Abeni
2020-06-04 16:55 ` Paolo Abeni
2020-06-04 17:43 [MPTCP] " Eric Dumazet
2020-06-04 17:43 ` Eric Dumazet
2020-06-04 23:00 [MPTCP] " David Miller
2020-06-04 23:00 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.