[PATCH 5.15 0/1] Request to backport 3c52c6bb831f to 5.15.y

[PATCH 5.15 0/1] Request to backport 3c52c6bb831f to 5.15.y

[Meena Shanmugam]

The commit 3c52c6bb831f (tcp/udp: Fix memory leak in
ipv6_renew_options()) fixes a memory leak reported by syzbot. This seems
to be a good candidate for the stable trees. This patch didn't apply cleanly
in 5.15 kernel, since release_sock() calls are changed to
sockopt_release_sock() in the latest kernel versions.

Kuniyuki Iwashima (1):
  tcp/udp: Fix memory leak in ipv6_renew_options().

 net/ipv6/ipv6_sockglue.c | 7 +++++++
 1 file changed, 7 insertions(+)

-- 
2.38.1.273.g43a17bfeac-goog

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

[Meena Shanmugam]

From: Kuniyuki Iwashima <kuniyu@amazon.com>

commit 3c52c6bb831f6335c176a0fc7214e26f43adbd11 upstream.

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)
                                              - 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 <syzkaller@googlegroups.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Meena Shanmugam <meenashanmugam@google.com>
---
 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 8a1c78f38508..b24e0e5d55f9 100644
--- a/net/ipv6/ipv6_sockglue.c
+++ b/net/ipv6/ipv6_sockglue.c
@@ -417,6 +417,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:
@@ -976,6 +982,7 @@ static int do_ipv6_setsockopt(struct sock *sk, int level, int optname,
 		break;
 	}
 
+unlock:
 	release_sock(sk);
 	if (needs_rtnl)
 		rtnl_unlock();
-- 
2.38.1.273.g43a17bfeac-goog

Re: [PATCH 5.15 0/1] Request to backport 3c52c6bb831f to 5.15.y

[Greg KH]

On Tue, Nov 01, 2022 at 12:52:01AM +0000, Meena Shanmugam wrote:
> The commit 3c52c6bb831f (tcp/udp: Fix memory leak in
> ipv6_renew_options()) fixes a memory leak reported by syzbot. This seems
> to be a good candidate for the stable trees. This patch didn't apply cleanly
> in 5.15 kernel, since release_sock() calls are changed to
> sockopt_release_sock() in the latest kernel versions.
> 
> Kuniyuki Iwashima (1):
>   tcp/udp: Fix memory leak in ipv6_renew_options().
> 
>  net/ipv6/ipv6_sockglue.c | 7 +++++++
>  1 file changed, 7 insertions(+)

Can you provide a working version for 6.0.y first?  We can not have
people upgrading to newer kernels and having a regression.

thanks,

greg k-h

Re: [PATCH 5.15 0/1] Request to backport 3c52c6bb831f to 5.15.y

[Meena Shanmugam]

On Tue, Nov 1, 2022 at 12:32 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Tue, Nov 01, 2022 at 12:52:01AM +0000, Meena Shanmugam wrote:
> > The commit 3c52c6bb831f (tcp/udp: Fix memory leak in
> > ipv6_renew_options()) fixes a memory leak reported by syzbot. This seems
> > to be a good candidate for the stable trees. This patch didn't apply cleanly
> > in 5.15 kernel, since release_sock() calls are changed to
> > sockopt_release_sock() in the latest kernel versions.
> >
> > Kuniyuki Iwashima (1):
> >   tcp/udp: Fix memory leak in ipv6_renew_options().
> >
> >  net/ipv6/ipv6_sockglue.c | 7 +++++++
> >  1 file changed, 7 insertions(+)
>
> Can you provide a working version for 6.0.y first?  We can not have
> people upgrading to newer kernels and having a regression.
>
> thanks,
>
> greg k-h

I have submitted the patch for 6.0.y as well.
https://lore.kernel.org/all/20221101200505.291406-2-meenashanmugam@google.com/

Thanks,
Meena

Re: [PATCH 5.15 0/1] Request to backport 3c52c6bb831f to 5.15.y

[Greg KH]

On Tue, Nov 01, 2022 at 01:55:21PM -0700, Meena Shanmugam wrote:
> On Tue, Nov 1, 2022 at 12:32 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Tue, Nov 01, 2022 at 12:52:01AM +0000, Meena Shanmugam wrote:
> > > The commit 3c52c6bb831f (tcp/udp: Fix memory leak in
> > > ipv6_renew_options()) fixes a memory leak reported by syzbot. This seems
> > > to be a good candidate for the stable trees. This patch didn't apply cleanly
> > > in 5.15 kernel, since release_sock() calls are changed to
> > > sockopt_release_sock() in the latest kernel versions.
> > >
> > > Kuniyuki Iwashima (1):
> > >   tcp/udp: Fix memory leak in ipv6_renew_options().
> > >
> > >  net/ipv6/ipv6_sockglue.c | 7 +++++++
> > >  1 file changed, 7 insertions(+)
> >
> > Can you provide a working version for 6.0.y first?  We can not have
> > people upgrading to newer kernels and having a regression.
> >
> > thanks,
> >
> > greg k-h
> 
> I have submitted the patch for 6.0.y as well.
> https://lore.kernel.org/all/20221101200505.291406-2-meenashanmugam@google.com/

Wonderful, all now queued up, thanks.

greg k-h