From 7c56badc9a66aecf61a2aad3fa23ce0599e04196 Mon Sep 17 00:00:00 2001 From: Roger Hu Date: Sat, 4 Sep 2021 09:17:12 -0700 Subject: [PATCH] kernel: Revert "tcp: do not lock listener to process SYN packets" This commit belongs to the patch set (https://lwn.net/Articles/659199/) that attempts to remove the use of locks on the socket table by relocating the SYN table to a separate hash table and adding a spin lock to protect the SYN request queue. Adding only this commit introduces a race condition for LineageOS kernels for TCP listens, since the TCP SYN data structures can be corrupted. A TCP curl bomb on a TCP listen port will corrupt the SYN accept backlog: for i in $(seq 1 400); do curl -x localhost:443 https://myhost.com -L --connect-timeout 30 -o /dev/null -sS & done Run `ss -nltp` and usually the RecVQ column does not drain to 0. This reverts commit 7d9f104f9cabe1d72a50c4816a48f64fc1da7a64. This really needs to be reverted across all LineageOS forks: https://gitlab.com/LineageOS/issues/android/-/issues/3916#note_669493796 Change-Id: Ia7969aeedae411677b307a8e094f9a4cc02b801d --- net/ipv4/tcp_ipv4.c | 8 +------- net/ipv6/tcp_ipv6.c | 8 +------- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c index 600c447fef23..019b35231420 100644 --- a/net/ipv4/tcp_ipv4.c +++ b/net/ipv4/tcp_ipv4.c @@ -1589,7 +1589,7 @@ static __sum16 tcp_v4_checksum_init(struct sk_buff *skb) /* The socket must have it's spinlock held when we get - * here, unless it is a TCP_LISTEN socket. + * here. * * We have a potential double-lock case here, so even when * doing backlog processing we use the BH locking scheme. @@ -1728,11 +1728,6 @@ process: skb->dev = NULL; - if (sk->sk_state == TCP_LISTEN) { - ret = tcp_v4_do_rcv(sk, skb); - goto put_and_return; - } - bh_lock_sock_nested(sk); tcp_sk(sk)->segs_in += max_t(u16, 1, skb_shinfo(skb)->gso_segs); ret = 0; @@ -1756,7 +1751,6 @@ process: } bh_unlock_sock(sk); -put_and_return: sock_put(sk); return ret; diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index 5c140ad29aca..67b8629060c4 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c @@ -1464,7 +1464,7 @@ static __sum16 tcp_v6_checksum_init(struct sk_buff *skb) } /* The socket must have it's spinlock held when we get - * here, unless it is a TCP_LISTEN socket. + * here. * * We have a potential double-lock case here, so even when * doing backlog processing we use the BH locking scheme. @@ -1658,11 +1658,6 @@ process: skb->dev = NULL; - if (sk->sk_state == TCP_LISTEN) { - ret = tcp_v6_do_rcv(sk, skb); - goto put_and_return; - } - bh_lock_sock_nested(sk); tcp_sk(sk)->segs_in += max_t(u16, 1, skb_shinfo(skb)->gso_segs); ret = 0; @@ -1686,7 +1681,6 @@ process: } bh_unlock_sock(sk); -put_and_return: sock_put(sk); return ret ? -1 : 0;