| Message ID | 20230922210530.2045146-1-i.maximets@ovn.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 9593c7cb6cf670ef724d17f7f9affd7a8d2ad0c5 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] ipv6: tcp: add a missing nf_reset_ct() in 3WHS handling | expand |
Ilya Maximets <i.maximets@ovn.org> wrote: > Commit b0e214d21203 ("netfilter: keep conntrack reference until > IPsecv6 policy checks are done") is a direct copy of the old > commit b59c270104f0 ("[NETFILTER]: Keep conntrack reference until > IPsec policy checks are done") but for IPv6. However, it also > copies a bug that this old commit had. That is: when the third > packet of 3WHS connection establishment contains payload, it is > added into socket receive queue without the XFRM check and the > drop of connection tracking context. > > That leads to nf_conntrack module being impossible to unload as > it waits for all the conntrack references to be dropped while > the packet release is deferred in per-cpu cache indefinitely, if > not consumed by the application. > > The issue for IPv4 was fixed in commit 6f0012e35160 ("tcp: add a > missing nf_reset_ct() in 3WHS handling") by adding a missing XFRM > check and correctly dropping the conntrack context. However, the > issue was introduced to IPv6 code afterwards. Fixing it the > same way for IPv6 now. > > Fixes: b0e214d21203 ("netfilter: keep conntrack reference until IPsecv6 policy checks are done") > Link: https://lore.kernel.org/netdev/d589a999-d4dd-2768-b2d5-89dec64a4a42@ovn.org/ > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- > net/ipv6/tcp_ipv6.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > LGTM, thanks for tracking this down. Acked-by: Florian Westphal <fw@strlen.de>
On Fri, Sep 22, 2023 at 11:04 PM Ilya Maximets <i.maximets@ovn.org> wrote: > > Commit b0e214d21203 ("netfilter: keep conntrack reference until > IPsecv6 policy checks are done") is a direct copy of the old > commit b59c270104f0 ("[NETFILTER]: Keep conntrack reference until > IPsec policy checks are done") but for IPv6. However, it also > copies a bug that this old commit had. That is: when the third > packet of 3WHS connection establishment contains payload, it is > added into socket receive queue without the XFRM check and the > drop of connection tracking context. > > That leads to nf_conntrack module being impossible to unload as > it waits for all the conntrack references to be dropped while > the packet release is deferred in per-cpu cache indefinitely, if > not consumed by the application. > > The issue for IPv4 was fixed in commit 6f0012e35160 ("tcp: add a > missing nf_reset_ct() in 3WHS handling") by adding a missing XFRM > check and correctly dropping the conntrack context. However, the > issue was introduced to IPv6 code afterwards. Fixing it the > same way for IPv6 now. > > Fixes: b0e214d21203 ("netfilter: keep conntrack reference until IPsecv6 policy checks are done") > Link: https://lore.kernel.org/netdev/d589a999-d4dd-2768-b2d5-89dec64a4a42@ovn.org/ > Signed-off-by: Ilya Maximets <i.maximets@ovn.org> > --- Nica catch, thanks a lot. Reviewed-by: Eric Dumazet <edumazet@google.com>
Hello: This patch was applied to netdev/net.git (main) by Paolo Abeni <pabeni@redhat.com>: On Fri, 22 Sep 2023 23:04:58 +0200 you wrote: > Commit b0e214d21203 ("netfilter: keep conntrack reference until > IPsecv6 policy checks are done") is a direct copy of the old > commit b59c270104f0 ("[NETFILTER]: Keep conntrack reference until > IPsec policy checks are done") but for IPv6. However, it also > copies a bug that this old commit had. That is: when the third > packet of 3WHS connection establishment contains payload, it is > added into socket receive queue without the XFRM check and the > drop of connection tracking context. > > [...] Here is the summary with links: - [net] ipv6: tcp: add a missing nf_reset_ct() in 3WHS handling https://git.kernel.org/netdev/net/c/9593c7cb6cf6 You are awesome, thank you!
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index 3a88545a265d..44b6949d72b2 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c @@ -1640,9 +1640,12 @@ INDIRECT_CALLABLE_SCOPE int tcp_v6_rcv(struct sk_buff *skb) struct sock *nsk; sk = req->rsk_listener; - drop_reason = tcp_inbound_md5_hash(sk, skb, - &hdr->saddr, &hdr->daddr, - AF_INET6, dif, sdif); + if (!xfrm6_policy_check(sk, XFRM_POLICY_IN, skb)) + drop_reason = SKB_DROP_REASON_XFRM_POLICY; + else + drop_reason = tcp_inbound_md5_hash(sk, skb, + &hdr->saddr, &hdr->daddr, + AF_INET6, dif, sdif); if (drop_reason) { sk_drops_add(sk, skb); reqsk_put(req); @@ -1689,6 +1692,7 @@ INDIRECT_CALLABLE_SCOPE int tcp_v6_rcv(struct sk_buff *skb) } goto discard_and_relse; } + nf_reset_ct(skb); if (nsk == sk) { reqsk_put(req); tcp_v6_restore_cb(skb);
Commit b0e214d21203 ("netfilter: keep conntrack reference until IPsecv6 policy checks are done") is a direct copy of the old commit b59c270104f0 ("[NETFILTER]: Keep conntrack reference until IPsec policy checks are done") but for IPv6. However, it also copies a bug that this old commit had. That is: when the third packet of 3WHS connection establishment contains payload, it is added into socket receive queue without the XFRM check and the drop of connection tracking context. That leads to nf_conntrack module being impossible to unload as it waits for all the conntrack references to be dropped while the packet release is deferred in per-cpu cache indefinitely, if not consumed by the application. The issue for IPv4 was fixed in commit 6f0012e35160 ("tcp: add a missing nf_reset_ct() in 3WHS handling") by adding a missing XFRM check and correctly dropping the conntrack context. However, the issue was introduced to IPv6 code afterwards. Fixing it the same way for IPv6 now. Fixes: b0e214d21203 ("netfilter: keep conntrack reference until IPsecv6 policy checks are done") Link: https://lore.kernel.org/netdev/d589a999-d4dd-2768-b2d5-89dec64a4a42@ovn.org/ Signed-off-by: Ilya Maximets <i.maximets@ovn.org> --- net/ipv6/tcp_ipv6.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)