[PATCH v1 net 1/5] tcp/udp: Fix memory leak in ipv6_renew_options().

2 views
Skip to first unread message

Kuniyuki Iwashima

unread,
Sep 28, 2022, 2:12:26 AMSep 28
to David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Ahern, Kuniyuki Iwashima, Kuniyuki Iwashima, net...@vger.kernel.org, syzkall...@googlegroups.com, linux-...@vger.kernel.org, syzbot
syzbot reported a memory leak [0] related to IPV6_ADDRFORM.

The scenario is that while one thread is converting an IPv6 socket into
IPv4 with IPV6_ADDRFORM, another thread calls do_ipv6_setsockopt() and
allocates memory to inet6_sk(sk)->XXX after conversion.

Then, the converted sk with (tcp|udp)_prot never frees the IPv6 resources,
which inet6_destroy_sock() should have cleaned up.

setsockopt(IPV6_ADDRFORM) setsockopt(IPV6_DSTOPTS)
+-----------------------+ +----------------------+
- do_ipv6_setsockopt(sk, ...)
- lock_sock(sk) - do_ipv6_setsockopt(sk, ...)
- WRITE_ONCE(sk->sk_prot, &tcp_prot) ^._ called via tcpv6_prot
- xchg(&np->opt, NULL) before WRITE_ONCE()
- txopt_put(opt)
- release_sock(sk)
- lock_sock(sk)
- ipv6_set_opt_hdr(sk, ...)
- ipv6_update_options(sk, opt)
- xchg(&inet6_sk(sk)->opt, opt)
^._ opt is never freed.

- release_sock(sk)

Since IPV6_DSTOPTS allocates options under lock_sock(), we can avoid this
memory leak by testing whether sk_family is changed by IPV6_ADDRFORM after
acquiring the lock.

This issue exists from the initial commit between IPV6_ADDRFORM and
IPV6_PKTOPTIONS.

[0]:
BUG: memory leak
unreferenced object 0xffff888009ab9f80 (size 96):
comm "syz-executor583", pid 328, jiffies 4294916198 (age 13.034s)
hex dump (first 32 bytes):
01 00 00 00 48 00 00 00 08 00 00 00 00 00 00 00 ....H...........
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[<000000002ee98ae1>] kmalloc include/linux/slab.h:605 [inline]
[<000000002ee98ae1>] sock_kmalloc+0xb3/0x100 net/core/sock.c:2566
[<0000000065d7b698>] ipv6_renew_options+0x21e/0x10b0 net/ipv6/exthdrs.c:1318
[<00000000a8c756d7>] ipv6_set_opt_hdr net/ipv6/ipv6_sockglue.c:354 [inline]
[<00000000a8c756d7>] do_ipv6_setsockopt.constprop.0+0x28b7/0x4350 net/ipv6/ipv6_sockglue.c:668
[<000000002854d204>] ipv6_setsockopt+0xdf/0x190 net/ipv6/ipv6_sockglue.c:1021
[<00000000e69fdcf8>] tcp_setsockopt+0x13b/0x2620 net/ipv4/tcp.c:3789
[<0000000090da4b9b>] __sys_setsockopt+0x239/0x620 net/socket.c:2252
[<00000000b10d192f>] __do_sys_setsockopt net/socket.c:2263 [inline]
[<00000000b10d192f>] __se_sys_setsockopt net/socket.c:2260 [inline]
[<00000000b10d192f>] __x64_sys_setsockopt+0xbe/0x160 net/socket.c:2260
[<000000000a80d7aa>] do_syscall_x64 arch/x86/entry/common.c:50 [inline]
[<000000000a80d7aa>] do_syscall_64+0x38/0x90 arch/x86/entry/common.c:80
[<000000004562b5c6>] entry_SYSCALL_64_after_hwframe+0x63/0xcd

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: syzbot <syzk...@googlegroups.com>
Signed-off-by: Kuniyuki Iwashima <kun...@amazon.com>
---
Note the syzbot is running on our EC2, so we don't have a URL or hash.
Also, there seems to be no similar report on the public syzkaller dashboard.
Thus the Reported-by address might be inappropriate.

Please let me know if we should keep it empty or use another email address.
---
net/ipv6/ipv6_sockglue.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c
index e0dcc7a193df..b61066ac8648 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -419,6 +419,12 @@ static int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
rtnl_lock();
lock_sock(sk);

+ /* Another thread has converted the socket into IPv4 with
+ * IPV6_ADDRFORM concurrently.
+ */
+ if (unlikely(sk->sk_family != AF_INET6))
+ goto unlock;
+
switch (optname) {

case IPV6_ADDRFORM:
@@ -994,6 +1000,7 @@ static int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
break;
}

+unlock:
release_sock(sk);
if (needs_rtnl)
rtnl_unlock();
--
2.30.2

Kuniyuki Iwashima

unread,
Sep 28, 2022, 2:12:26 AMSep 28
to David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Ahern, Kuniyuki Iwashima, Kuniyuki Iwashima, net...@vger.kernel.org, syzkall...@googlegroups.com, linux-...@vger.kernel.org, syzbot

Kuniyuki Iwashima

unread,
Sep 28, 2022, 9:26:39 PMSep 28
to David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Ahern, Kuniyuki Iwashima, Kuniyuki Iwashima, net...@vger.kernel.org, syzkall...@googlegroups.com, linux-...@vger.kernel.org, syzbot

Kuniyuki Iwashima

unread,
Oct 3, 2022, 11:47:24 AMOct 3
to David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Ahern, Hideaki YOSHIFUJI, Kuniyuki Iwashima, Kuniyuki Iwashima, net...@vger.kernel.org, syzkall...@googlegroups.com, linux-...@vger.kernel.org, syzbot

Kuniyuki Iwashima

unread,
Oct 4, 2022, 1:39:04 PMOct 4
to David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Ahern, Hideaki YOSHIFUJI, Kuniyuki Iwashima, Kuniyuki Iwashima, net...@vger.kernel.org, syzkall...@googlegroups.com, linux-...@vger.kernel.org, syzbot

Kuniyuki Iwashima

unread,
Oct 6, 2022, 3:09:23 PMOct 6
to David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, David Ahern, Hideaki YOSHIFUJI, Kuniyuki Iwashima, Kuniyuki Iwashima, net...@vger.kernel.org, syzkall...@googlegroups.com, linux-...@vger.kernel.org, syzbot
syzbot reported a memory leak [0] related to IPV6_ADDRFORM.

The scenario is that while one thread is converting an IPv6 socket into
IPv4 with IPV6_ADDRFORM, another thread calls do_ipv6_setsockopt() and
allocates memory to inet6_sk(sk)->XXX after conversion.

Then, the converted sk with (tcp|udp)_prot never frees the IPv6 resources,
which inet6_destroy_sock() should have cleaned up.

setsockopt(IPV6_ADDRFORM) setsockopt(IPV6_DSTOPTS)
+-----------------------+ +----------------------+
- do_ipv6_setsockopt(sk, ...)
- sockopt_lock_sock(sk) - do_ipv6_setsockopt(sk, ...)
- lock_sock(sk) ^._ called via tcpv6_prot
- WRITE_ONCE(sk->sk_prot, &tcp_prot) before WRITE_ONCE()
- xchg(&np->opt, NULL)
- txopt_put(opt)
- sockopt_release_sock(sk)
- release_sock(sk) - sockopt_lock_sock(sk)
- lock_sock(sk)
- ipv6_set_opt_hdr(sk, ...)
- ipv6_update_options(sk, opt)
- xchg(&inet6_sk(sk)->opt, opt)
^._ opt is never freed.

- sockopt_release_sock(sk)
index 2d2f4dd9e5df..408345fc4c5c 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -419,6 +419,12 @@ int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
rtnl_lock();
sockopt_lock_sock(sk);

+ /* Another thread has converted the socket into IPv4 with
+ * IPV6_ADDRFORM concurrently.
+ */
+ if (unlikely(sk->sk_family != AF_INET6))
+ goto unlock;
+
switch (optname) {

case IPV6_ADDRFORM:
@@ -994,6 +1000,7 @@ int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
break;
}

+unlock:
sockopt_release_sock(sk);
Reply all
Reply to author
Forward
0 new messages