tcp/udp: Call inet6_destroy_sock() in IPv6 sk->sk_destruct().
authorKuniyuki Iwashima <kuniyu@amazon.com>
Thu, 6 Oct 2022 18:53:47 +0000 (11:53 -0700)
committerJakub Kicinski <kuba@kernel.org>
Thu, 13 Oct 2022 00:50:37 +0000 (17:50 -0700)
commitd38afeec26ed4739c640bf286c270559aab2ba5f
treebb670d5e384bc8cdf6c7531266306639d22371e7
parent21985f43376cee092702d6cb963ff97a9d2ede68
tcp/udp: Call inet6_destroy_sock() in IPv6 sk->sk_destruct().

Originally, inet6_sk(sk)->XXX were changed under lock_sock(), so we were
able to clean them up by calling inet6_destroy_sock() during the IPv6 ->
IPv4 conversion by IPV6_ADDRFORM.  However, commit 03485f2adcde ("udpv6:
Add lockless sendmsg() support") added a lockless memory allocation path,
which could cause a memory leak:

setsockopt(IPV6_ADDRFORM)                 sendmsg()
+-----------------------+                 +-------+
- do_ipv6_setsockopt(sk, ...)             - udpv6_sendmsg(sk, ...)
  - sockopt_lock_sock(sk)                   ^._ called via udpv6_prot
    - lock_sock(sk)                             before WRITE_ONCE()
  - WRITE_ONCE(sk->sk_prot, &tcp_prot)
  - inet6_destroy_sock()                    - if (!corkreq)
  - sockopt_release_sock(sk)                  - ip6_make_skb(sk, ...)
    - release_sock(sk)                          ^._ lockless fast path for
                                                    the non-corking case

                                                - __ip6_append_data(sk, ...)
                                                  - ipv6_local_rxpmtu(sk, ...)
                                                    - xchg(&np->rxpmtu, skb)
                                                      ^._ rxpmtu is never freed.

                                                - goto out_no_dst;

                                            - lock_sock(sk)

For now, rxpmtu is only the case, but not to miss the future change
and a similar bug fixed in commit e27326009a3d ("net: ping6: Fix
memleak in ipv6_renew_options()."), let's set a new function to IPv6
sk->sk_destruct() and call inet6_cleanup_sock() there.  Since the
conversion does not change sk->sk_destruct(), we can guarantee that
we can clean up IPv6 resources finally.

We can now remove all inet6_destroy_sock() calls from IPv6 protocol
specific ->destroy() functions, but such changes are invasive to
backport.  So they can be posted as a follow-up later for net-next.

Fixes: 03485f2adcde ("udpv6: Add lockless sendmsg() support")
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
include/net/ipv6.h
include/net/udp.h
include/net/udplite.h
net/ipv4/udp.c
net/ipv4/udplite.c
net/ipv6/af_inet6.c
net/ipv6/udp.c
net/ipv6/udp_impl.h
net/ipv6/udplite.c