* memory leak in do_ipv6_setsockopt
@ 2015-12-01 12:27 Dmitry Vyukov
2015-12-01 13:36 ` Eric Dumazet
0 siblings, 1 reply; 10+ messages in thread
From: Dmitry Vyukov @ 2015-12-01 12:27 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, Vlad Yasevich,
Neil Horman, linux-sctp
Cc: syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin,
Eric Dumazet
Hello,
The following program causes a memory leak :
// autogenerated by syzkaller (http://github.com/google/syzkaller)
#include <syscall.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <string.h>
#include <stdint.h>
#include <linux/in.h>
#include <linux/socket.h>
int main()
{
long r1 = syscall(SYS_socket, PF_INET6,
SOCK_STREAM|SOCK_CLOEXEC|SOCK_NONBLOCK, IPPROTO_SCTP);
const char *opt = "\x15\x53\x5e\x2d\x97\xab\xe1";
long r3 = syscall(SYS_setsockopt, r1, 0x29ul, 0x6ul, opt, 0x7ul);
return 0;
}
unreferenced object 0xffff880039a55260 (size 64):
comm "executor", pid 11746, jiffies 4298984475 (age 16.078s)
hex dump (first 32 bytes):
2f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 /...............
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
backtrace:
[< inline >] kmalloc include/linux/slab.h:463
[<ffffffff848a2f5f>] sock_kmalloc+0x7f/0xc0 net/core/sock.c:1774
[<ffffffff84e5bea0>] do_ipv6_setsockopt.isra.7+0x15d0/0x2830
net/ipv6/ipv6_sockglue.c:483
[<ffffffff84e5d19b>] ipv6_setsockopt+0x9b/0x140 net/ipv6/ipv6_sockglue.c:885
[<ffffffff8544616c>] sctp_setsockopt+0x15c/0x36c0 net/sctp/socket.c:3702
[<ffffffff848a2035>] sock_common_setsockopt+0x95/0xd0 net/core/sock.c:2645
[< inline >] SYSC_setsockopt net/socket.c:1757
[<ffffffff8489f1d8>] SyS_setsockopt+0x158/0x240 net/socket.c:1736
I confirmed that running this program in a loop steadily increases
number of objects in kmalloc-64 slab. The leak does not happen with
IPPROTO_TCP, so probably it is sctp-related.
On commit 31ade3b83e1821da5fbb2f11b5b3d4ab2ec39db8 (Nov 29).
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: memory leak in do_ipv6_setsockopt 2015-12-01 12:27 memory leak in do_ipv6_setsockopt Dmitry Vyukov @ 2015-12-01 13:36 ` Eric Dumazet 2015-12-01 14:07 ` Daniel Borkmann 0 siblings, 1 reply; 10+ messages in thread From: Eric Dumazet @ 2015-12-01 13:36 UTC (permalink / raw) To: Dmitry Vyukov Cc: David S. Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, Vlad Yasevich, Neil Horman, linux-sctp, syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin, Eric Dumazet On Tue, 2015-12-01 at 13:27 +0100, Dmitry Vyukov wrote: > Hello, > > The following program causes a memory leak : > > // autogenerated by syzkaller (http://github.com/google/syzkaller) > #include <syscall.h> > #include <sys/types.h> > #include <sys/socket.h> > #include <string.h> > #include <stdint.h> > #include <linux/in.h> > #include <linux/socket.h> > > int main() > { > long r1 = syscall(SYS_socket, PF_INET6, > SOCK_STREAM|SOCK_CLOEXEC|SOCK_NONBLOCK, IPPROTO_SCTP); > const char *opt = "\x15\x53\x5e\x2d\x97\xab\xe1"; > long r3 = syscall(SYS_setsockopt, r1, 0x29ul, 0x6ul, opt, 0x7ul); > return 0; > } > > > unreferenced object 0xffff880039a55260 (size 64): > comm "executor", pid 11746, jiffies 4298984475 (age 16.078s) > hex dump (first 32 bytes): > 2f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 /............... > 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ > backtrace: > [< inline >] kmalloc include/linux/slab.h:463 > [<ffffffff848a2f5f>] sock_kmalloc+0x7f/0xc0 net/core/sock.c:1774 > [<ffffffff84e5bea0>] do_ipv6_setsockopt.isra.7+0x15d0/0x2830 > net/ipv6/ipv6_sockglue.c:483 > [<ffffffff84e5d19b>] ipv6_setsockopt+0x9b/0x140 net/ipv6/ipv6_sockglue.c:885 > [<ffffffff8544616c>] sctp_setsockopt+0x15c/0x36c0 net/sctp/socket.c:3702 > [<ffffffff848a2035>] sock_common_setsockopt+0x95/0xd0 net/core/sock.c:2645 > [< inline >] SYSC_setsockopt net/socket.c:1757 > [<ffffffff8489f1d8>] SyS_setsockopt+0x158/0x240 net/socket.c:1736 > > > I confirmed that running this program in a loop steadily increases > number of objects in kmalloc-64 slab. The leak does not happen with > IPPROTO_TCP, so probably it is sctp-related. Thanks for the report. Probably fixed by : diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 897c01c029ca..8079ecd8465d 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -7375,6 +7375,12 @@ struct proto sctp_prot = { #if IS_ENABLED(CONFIG_IPV6) +static void sctp_v6_destroy_sock(struct sock *sk) +{ + sctp_destroy_sock(sk); + inet6_destroy_sock(sk); +} + struct proto sctpv6_prot = { .name = "SCTPv6", .owner = THIS_MODULE, @@ -7384,7 +7390,7 @@ struct proto sctpv6_prot = { .accept = sctp_accept, .ioctl = sctp_ioctl, .init = sctp_init_sock, - .destroy = sctp_destroy_sock, + .destroy = sctp_v6_destroy_sock, .shutdown = sctp_shutdown, .setsockopt = sctp_setsockopt, .getsockopt = sctp_getsockopt, ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: memory leak in do_ipv6_setsockopt 2015-12-01 13:36 ` Eric Dumazet @ 2015-12-01 14:07 ` Daniel Borkmann 2015-12-01 14:16 ` Eric Dumazet 0 siblings, 1 reply; 10+ messages in thread From: Daniel Borkmann @ 2015-12-01 14:07 UTC (permalink / raw) To: Eric Dumazet, Dmitry Vyukov Cc: David S. Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, Vlad Yasevich, Neil Horman, linux-sctp, syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin, Eric Dumazet On 12/01/2015 02:36 PM, Eric Dumazet wrote: > On Tue, 2015-12-01 at 13:27 +0100, Dmitry Vyukov wrote: >> Hello, >> >> The following program causes a memory leak : >> >> // autogenerated by syzkaller (http://github.com/google/syzkaller) >> #include <syscall.h> >> #include <sys/types.h> >> #include <sys/socket.h> >> #include <string.h> >> #include <stdint.h> >> #include <linux/in.h> >> #include <linux/socket.h> >> >> int main() >> { >> long r1 = syscall(SYS_socket, PF_INET6, >> SOCK_STREAM|SOCK_CLOEXEC|SOCK_NONBLOCK, IPPROTO_SCTP); >> const char *opt = "\x15\x53\x5e\x2d\x97\xab\xe1"; >> long r3 = syscall(SYS_setsockopt, r1, 0x29ul, 0x6ul, opt, 0x7ul); >> return 0; >> } >> >> >> unreferenced object 0xffff880039a55260 (size 64): >> comm "executor", pid 11746, jiffies 4298984475 (age 16.078s) >> hex dump (first 32 bytes): >> 2f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 /............... >> 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ >> backtrace: >> [< inline >] kmalloc include/linux/slab.h:463 >> [<ffffffff848a2f5f>] sock_kmalloc+0x7f/0xc0 net/core/sock.c:1774 >> [<ffffffff84e5bea0>] do_ipv6_setsockopt.isra.7+0x15d0/0x2830 >> net/ipv6/ipv6_sockglue.c:483 >> [<ffffffff84e5d19b>] ipv6_setsockopt+0x9b/0x140 net/ipv6/ipv6_sockglue.c:885 >> [<ffffffff8544616c>] sctp_setsockopt+0x15c/0x36c0 net/sctp/socket.c:3702 >> [<ffffffff848a2035>] sock_common_setsockopt+0x95/0xd0 net/core/sock.c:2645 >> [< inline >] SYSC_setsockopt net/socket.c:1757 >> [<ffffffff8489f1d8>] SyS_setsockopt+0x158/0x240 net/socket.c:1736 >> >> >> I confirmed that running this program in a loop steadily increases >> number of objects in kmalloc-64 slab. The leak does not happen with >> IPPROTO_TCP, so probably it is sctp-related. > > Thanks for the report. > > Probably fixed by : > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > index 897c01c029ca..8079ecd8465d 100644 > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -7375,6 +7375,12 @@ struct proto sctp_prot = { > > #if IS_ENABLED(CONFIG_IPV6) > > +static void sctp_v6_destroy_sock(struct sock *sk) > +{ > + sctp_destroy_sock(sk); > + inet6_destroy_sock(sk); > +} Yeah, we miss inet6_destroy_sock() in SCTP. :-( Looks good to me. > struct proto sctpv6_prot = { > .name = "SCTPv6", > .owner = THIS_MODULE, > @@ -7384,7 +7390,7 @@ struct proto sctpv6_prot = { > .accept = sctp_accept, > .ioctl = sctp_ioctl, > .init = sctp_init_sock, > - .destroy = sctp_destroy_sock, > + .destroy = sctp_v6_destroy_sock, > .shutdown = sctp_shutdown, > .setsockopt = sctp_setsockopt, > .getsockopt = sctp_getsockopt, > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: memory leak in do_ipv6_setsockopt 2015-12-01 14:07 ` Daniel Borkmann @ 2015-12-01 14:16 ` Eric Dumazet 2015-12-01 14:24 ` Daniel Borkmann 0 siblings, 1 reply; 10+ messages in thread From: Eric Dumazet @ 2015-12-01 14:16 UTC (permalink / raw) To: Daniel Borkmann Cc: Dmitry Vyukov, David S. Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, Vlad Yasevich, Neil Horman, linux-sctp, syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin, Eric Dumazet On Tue, 2015-12-01 at 15:07 +0100, Daniel Borkmann wrote: > Yeah, we miss inet6_destroy_sock() in SCTP. :-( > > Looks good to me. OK, I will send a formal (and tested ;) ) patch. Thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: memory leak in do_ipv6_setsockopt 2015-12-01 14:16 ` Eric Dumazet @ 2015-12-01 14:24 ` Daniel Borkmann 2015-12-01 14:38 ` Eric Dumazet 0 siblings, 1 reply; 10+ messages in thread From: Daniel Borkmann @ 2015-12-01 14:24 UTC (permalink / raw) To: Eric Dumazet Cc: Dmitry Vyukov, David S. Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, Vlad Yasevich, Neil Horman, linux-sctp, syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin, Eric Dumazet On 12/01/2015 03:16 PM, Eric Dumazet wrote: > On Tue, 2015-12-01 at 15:07 +0100, Daniel Borkmann wrote: > >> Yeah, we miss inet6_destroy_sock() in SCTP. :-( >> >> Looks good to me. > > OK, I will send a formal (and tested ;) ) patch. I was shortly wondering whether there could be a use-after-free by doing this after sctp_destroy_sock() due to the sctp_endpoint_destroy() that would eventually drop a ref on the socket, but the endpoint holds a separate ref, so we should be good. Thanks, Daniel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: memory leak in do_ipv6_setsockopt 2015-12-01 14:24 ` Daniel Borkmann @ 2015-12-01 14:38 ` Eric Dumazet 2015-12-01 14:49 ` Daniel Borkmann 2015-12-01 15:20 ` [PATCH net] ipv6: sctp: implement sctp_v6_destroy_sock() Eric Dumazet 0 siblings, 2 replies; 10+ messages in thread From: Eric Dumazet @ 2015-12-01 14:38 UTC (permalink / raw) To: Daniel Borkmann Cc: Dmitry Vyukov, David S. Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, Vlad Yasevich, Neil Horman, linux-sctp, syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin, Eric Dumazet On Tue, 2015-12-01 at 15:24 +0100, Daniel Borkmann wrote: > On 12/01/2015 03:16 PM, Eric Dumazet wrote: > > On Tue, 2015-12-01 at 15:07 +0100, Daniel Borkmann wrote: > > > >> Yeah, we miss inet6_destroy_sock() in SCTP. :-( > >> > >> Looks good to me. > > > > OK, I will send a formal (and tested ;) ) patch. > > I was shortly wondering whether there could be a use-after-free by > doing this after sctp_destroy_sock() due to the sctp_endpoint_destroy() > that would eventually drop a ref on the socket, but the endpoint holds > a separate ref, so we should be good. > More generically ->destroy() caller must keep a reference on the socket. inet_csk_destroy_sock() for example uses sk after sk->sk_prot->destroy(sk); ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: memory leak in do_ipv6_setsockopt 2015-12-01 14:38 ` Eric Dumazet @ 2015-12-01 14:49 ` Daniel Borkmann 2015-12-01 15:20 ` [PATCH net] ipv6: sctp: implement sctp_v6_destroy_sock() Eric Dumazet 1 sibling, 0 replies; 10+ messages in thread From: Daniel Borkmann @ 2015-12-01 14:49 UTC (permalink / raw) To: Eric Dumazet Cc: Dmitry Vyukov, David S. Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, Vlad Yasevich, Neil Horman, linux-sctp, syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin, Eric Dumazet On 12/01/2015 03:38 PM, Eric Dumazet wrote: > On Tue, 2015-12-01 at 15:24 +0100, Daniel Borkmann wrote: >> On 12/01/2015 03:16 PM, Eric Dumazet wrote: >>> On Tue, 2015-12-01 at 15:07 +0100, Daniel Borkmann wrote: >>> >>>> Yeah, we miss inet6_destroy_sock() in SCTP. :-( >>>> >>>> Looks good to me. >>> >>> OK, I will send a formal (and tested ;) ) patch. >> >> I was shortly wondering whether there could be a use-after-free by >> doing this after sctp_destroy_sock() due to the sctp_endpoint_destroy() >> that would eventually drop a ref on the socket, but the endpoint holds >> a separate ref, so we should be good. > > More generically ->destroy() caller must keep a reference on the socket. > > inet_csk_destroy_sock() for example uses sk after > > sk->sk_prot->destroy(sk); Right, and later on, we might call into ->sk_destruct() when there are no more refs (in SCTP case: sctp_destruct_sock()). Thanks, Daniel ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH net] ipv6: sctp: implement sctp_v6_destroy_sock() 2015-12-01 14:38 ` Eric Dumazet 2015-12-01 14:49 ` Daniel Borkmann @ 2015-12-01 15:20 ` Eric Dumazet 2015-12-01 15:55 ` Daniel Borkmann 2015-12-03 17:06 ` David Miller 1 sibling, 2 replies; 10+ messages in thread From: Eric Dumazet @ 2015-12-01 15:20 UTC (permalink / raw) To: Daniel Borkmann Cc: Dmitry Vyukov, David S. Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, Vlad Yasevich, Neil Horman, linux-sctp, syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin, Eric Dumazet From: Eric Dumazet <edumazet@google.com> Dmitry Vyukov reported a memory leak using IPV6 SCTP sockets. We need to call inet6_destroy_sock() to properly release inet6 specific fields. Reported-by: Dmitry Vyukov <dvyukov@google.com> Signed-off-by: Eric Dumazet <edumazet@google.com> --- net/sctp/socket.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 897c01c029ca..9d0b9bae9052 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -7375,6 +7375,13 @@ struct proto sctp_prot = { #if IS_ENABLED(CONFIG_IPV6) +#include <net/transp_v6.h> +static void sctp_v6_destroy_sock(struct sock *sk) +{ + sctp_destroy_sock(sk); + inet6_destroy_sock(sk); +} + struct proto sctpv6_prot = { .name = "SCTPv6", .owner = THIS_MODULE, @@ -7384,7 +7391,7 @@ struct proto sctpv6_prot = { .accept = sctp_accept, .ioctl = sctp_ioctl, .init = sctp_init_sock, - .destroy = sctp_destroy_sock, + .destroy = sctp_v6_destroy_sock, .shutdown = sctp_shutdown, .setsockopt = sctp_setsockopt, .getsockopt = sctp_getsockopt, ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH net] ipv6: sctp: implement sctp_v6_destroy_sock() 2015-12-01 15:20 ` [PATCH net] ipv6: sctp: implement sctp_v6_destroy_sock() Eric Dumazet @ 2015-12-01 15:55 ` Daniel Borkmann 2015-12-03 17:06 ` David Miller 1 sibling, 0 replies; 10+ messages in thread From: Daniel Borkmann @ 2015-12-01 15:55 UTC (permalink / raw) To: Eric Dumazet Cc: Dmitry Vyukov, David S. Miller, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, Vlad Yasevich, Neil Horman, linux-sctp, syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin, Eric Dumazet On 12/01/2015 04:20 PM, Eric Dumazet wrote: > From: Eric Dumazet <edumazet@google.com> > > Dmitry Vyukov reported a memory leak using IPV6 SCTP sockets. > > We need to call inet6_destroy_sock() to properly release > inet6 specific fields. > > Reported-by: Dmitry Vyukov <dvyukov@google.com> > Signed-off-by: Eric Dumazet <edumazet@google.com> Acked-by: Daniel Borkmann <daniel@iogearbox.net> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net] ipv6: sctp: implement sctp_v6_destroy_sock() 2015-12-01 15:20 ` [PATCH net] ipv6: sctp: implement sctp_v6_destroy_sock() Eric Dumazet 2015-12-01 15:55 ` Daniel Borkmann @ 2015-12-03 17:06 ` David Miller 1 sibling, 0 replies; 10+ messages in thread From: David Miller @ 2015-12-03 17:06 UTC (permalink / raw) To: eric.dumazet Cc: daniel, dvyukov, kuznet, jmorris, yoshfuji, kaber, netdev, linux-kernel, vyasevich, nhorman, linux-sctp, syzkaller, kcc, glider, sasha.levin, edumazet From: Eric Dumazet <eric.dumazet@gmail.com> Date: Tue, 01 Dec 2015 07:20:07 -0800 > From: Eric Dumazet <edumazet@google.com> > > Dmitry Vyukov reported a memory leak using IPV6 SCTP sockets. > > We need to call inet6_destroy_sock() to properly release > inet6 specific fields. > > Reported-by: Dmitry Vyukov <dvyukov@google.com> > Signed-off-by: Eric Dumazet <edumazet@google.com> Applied and queued up for -stable, thanks. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-12-03 17:06 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-12-01 12:27 memory leak in do_ipv6_setsockopt Dmitry Vyukov 2015-12-01 13:36 ` Eric Dumazet 2015-12-01 14:07 ` Daniel Borkmann 2015-12-01 14:16 ` Eric Dumazet 2015-12-01 14:24 ` Daniel Borkmann 2015-12-01 14:38 ` Eric Dumazet 2015-12-01 14:49 ` Daniel Borkmann 2015-12-01 15:20 ` [PATCH net] ipv6: sctp: implement sctp_v6_destroy_sock() Eric Dumazet 2015-12-01 15:55 ` Daniel Borkmann 2015-12-03 17:06 ` David Miller
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).