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).
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,
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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
>
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.
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
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);
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
From: Eric Dumazet <[email protected]>
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 <[email protected]>
Signed-off-by: Eric Dumazet <[email protected]>
---
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,
On 12/01/2015 04:20 PM, Eric Dumazet wrote:
> From: Eric Dumazet <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Eric Dumazet <[email protected]>
Acked-by: Daniel Borkmann <[email protected]>
From: Eric Dumazet <[email protected]>
Date: Tue, 01 Dec 2015 07:20:07 -0800
> From: Eric Dumazet <[email protected]>
>
> 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 <[email protected]>
> Signed-off-by: Eric Dumazet <[email protected]>
Applied and queued up for -stable, thanks.