All of lore.kernel.org
 help / color / mirror / Atom feed
* net: memory leak in ip_cmsg_send
@ 2016-02-04  9:47 Dmitry Vyukov
  2016-02-04 14:04 ` Eric Dumazet
  0 siblings, 1 reply; 5+ messages in thread
From: Dmitry Vyukov @ 2016-02-04  9:47 UTC (permalink / raw)
  To: David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, Eric Dumazet
  Cc: Kostya Serebryany, syzkaller, Alexander Potapenko, Sasha Levin

Hello,

I've hit the following memory leak while running syzkaller fuzzer:

unreferenced object 0xffff88002ea39708 (size 64):
  comm "syz-executor", pid 19887, jiffies 4295848369 (age 8.676s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
    00 00 00 00 00 00 00 00 04 00 00 00 04 00 00 00  ................
  backtrace:
    [<     inline     >] kzalloc include/linux/slab.h:607
    [<     inline     >] ip_options_get_alloc net/ipv4/ip_options.c:515
    [<ffffffff85836ea4>] ip_options_get+0x34/0x90 net/ipv4/ip_options.c:551
    [<ffffffff8584924d>] ip_cmsg_send+0x5bd/0x6f0 net/ipv4/ip_sockglue.c:252
    [<ffffffff858e8502>] raw_sendmsg+0xa52/0x25a0 net/ieee802154/socket.c:738
    [<ffffffff859246e7>] inet_sendmsg+0x2f7/0x4c0 net/ipv4/af_inet.c:736
    [<     inline     >] sock_sendmsg_nosec net/socket.c:611
    [<ffffffff854cdffa>] sock_sendmsg+0xca/0x110 net/socket.c:621
    [<ffffffff854cfe3a>] ___sys_sendmsg+0x72a/0x840 net/socket.c:1947
    [<ffffffff854d1e0e>] __sys_sendmsg+0xce/0x170 net/socket.c:1981
    [<     inline     >] SYSC_sendmsg net/socket.c:1992
    [<ffffffff854d1edd>] SyS_sendmsg+0x2d/0x50 net/socket.c:1988

ip_cmsg_send seems to forget to free ipc->opt when it returns an error
(all callers expect that there is nothing to cleanup if it returns an
error).

On commit 34229b277480f46c1e9a19f027f30b074512e68b.

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: net: memory leak in ip_cmsg_send
  2016-02-04  9:47 net: memory leak in ip_cmsg_send Dmitry Vyukov
@ 2016-02-04 14:04 ` Eric Dumazet
  2016-02-04 14:23   ` [PATCH net] ipv4: fix memory leaks in ip_cmsg_send() callers Eric Dumazet
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Dumazet @ 2016-02-04 14:04 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: David S. Miller, Alexey Kuznetsov, James Morris,
	Hideaki YOSHIFUJI, Patrick McHardy, netdev, LKML, Eric Dumazet,
	Kostya Serebryany, syzkaller, Alexander Potapenko, Sasha Levin

On Thu, 2016-02-04 at 10:47 +0100, Dmitry Vyukov wrote:
> Hello,
> 
> I've hit the following memory leak while running syzkaller fuzzer:
> 
> unreferenced object 0xffff88002ea39708 (size 64):
>   comm "syz-executor", pid 19887, jiffies 4295848369 (age 8.676s)
>   hex dump (first 32 bytes):
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
>     00 00 00 00 00 00 00 00 04 00 00 00 04 00 00 00  ................
>   backtrace:
>     [<     inline     >] kzalloc include/linux/slab.h:607
>     [<     inline     >] ip_options_get_alloc net/ipv4/ip_options.c:515
>     [<ffffffff85836ea4>] ip_options_get+0x34/0x90 net/ipv4/ip_options.c:551
>     [<ffffffff8584924d>] ip_cmsg_send+0x5bd/0x6f0 net/ipv4/ip_sockglue.c:252
>     [<ffffffff858e8502>] raw_sendmsg+0xa52/0x25a0 net/ieee802154/socket.c:738
>     [<ffffffff859246e7>] inet_sendmsg+0x2f7/0x4c0 net/ipv4/af_inet.c:736
>     [<     inline     >] sock_sendmsg_nosec net/socket.c:611
>     [<ffffffff854cdffa>] sock_sendmsg+0xca/0x110 net/socket.c:621
>     [<ffffffff854cfe3a>] ___sys_sendmsg+0x72a/0x840 net/socket.c:1947
>     [<ffffffff854d1e0e>] __sys_sendmsg+0xce/0x170 net/socket.c:1981
>     [<     inline     >] SYSC_sendmsg net/socket.c:1992
>     [<ffffffff854d1edd>] SyS_sendmsg+0x2d/0x50 net/socket.c:1988
> 
> ip_cmsg_send seems to forget to free ipc->opt when it returns an error
> (all callers expect that there is nothing to cleanup if it returns an
> error).
> 
> On commit 34229b277480f46c1e9a19f027f30b074512e68b.

Hmmm... thanks for the report, I am testing a patch.

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH net] ipv4: fix memory leaks in ip_cmsg_send() callers
  2016-02-04 14:04 ` Eric Dumazet
@ 2016-02-04 14:23   ` Eric Dumazet
  2016-02-13 10:59     ` David Miller
  2016-02-15 20:11     ` Cong Wang
  0 siblings, 2 replies; 5+ messages in thread
From: Eric Dumazet @ 2016-02-04 14:23 UTC (permalink / raw)
  To: Dmitry Vyukov, David Miller; +Cc: netdev

From: Eric Dumazet <edumazet@google.com>

Dmitry reported memory leaks of IP options allocated in
ip_cmsg_send() when/if this function returns an error.

Callers are responsible for the freeing.

Many thanks to Dmitry for the report and diagnostic.

Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/ip_sockglue.c |    2 ++
 net/ipv4/ping.c        |    4 +++-
 net/ipv4/raw.c         |    4 +++-
 net/ipv4/udp.c         |    4 +++-
 4 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c
index 5f73a7c03e27..a50124260f5a 100644
--- a/net/ipv4/ip_sockglue.c
+++ b/net/ipv4/ip_sockglue.c
@@ -249,6 +249,8 @@ int ip_cmsg_send(struct net *net, struct msghdr *msg, struct ipcm_cookie *ipc,
 		switch (cmsg->cmsg_type) {
 		case IP_RETOPTS:
 			err = cmsg->cmsg_len - CMSG_ALIGN(sizeof(struct cmsghdr));
+
+			/* Our caller is responsible for freeing ipc->opt */
 			err = ip_options_get(net, &ipc->opt, CMSG_DATA(cmsg),
 					     err < 40 ? err : 40);
 			if (err)
diff --git a/net/ipv4/ping.c b/net/ipv4/ping.c
index c117b21b937d..ec74b1a34315 100644
--- a/net/ipv4/ping.c
+++ b/net/ipv4/ping.c
@@ -746,8 +746,10 @@ static int ping_v4_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 
 	if (msg->msg_controllen) {
 		err = ip_cmsg_send(sock_net(sk), msg, &ipc, false);
-		if (err)
+		if (unlikely(err)) {
+			kfree(ipc.opt);
 			return err;
+		}
 		if (ipc.opt)
 			free = 1;
 	}
diff --git a/net/ipv4/raw.c b/net/ipv4/raw.c
index bc35f1842512..bba4f6f379a9 100644
--- a/net/ipv4/raw.c
+++ b/net/ipv4/raw.c
@@ -547,8 +547,10 @@ static int raw_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 
 	if (msg->msg_controllen) {
 		err = ip_cmsg_send(net, msg, &ipc, false);
-		if (err)
+		if (unlikely(err)) {
+			kfree(ipc.opt);
 			goto out;
+		}
 		if (ipc.opt)
 			free = 1;
 	}
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index be0b21852b13..8182363bff5e 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1048,8 +1048,10 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
 	if (msg->msg_controllen) {
 		err = ip_cmsg_send(sock_net(sk), msg, &ipc,
 				   sk->sk_family == AF_INET6);
-		if (err)
+		if (unlikely(err)) {
+			kfree(ipc.opt);
 			return err;
+		}
 		if (ipc.opt)
 			free = 1;
 		connected = 0;

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH net] ipv4: fix memory leaks in ip_cmsg_send() callers
  2016-02-04 14:23   ` [PATCH net] ipv4: fix memory leaks in ip_cmsg_send() callers Eric Dumazet
@ 2016-02-13 10:59     ` David Miller
  2016-02-15 20:11     ` Cong Wang
  1 sibling, 0 replies; 5+ messages in thread
From: David Miller @ 2016-02-13 10:59 UTC (permalink / raw)
  To: eric.dumazet; +Cc: dvyukov, netdev

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 04 Feb 2016 06:23:28 -0800

> From: Eric Dumazet <edumazet@google.com>
> 
> Dmitry reported memory leaks of IP options allocated in
> ip_cmsg_send() when/if this function returns an error.
> 
> Callers are responsible for the freeing.
> 
> Many thanks to Dmitry for the report and diagnostic.
> 
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>

Applied and queued up for -stable.

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH net] ipv4: fix memory leaks in ip_cmsg_send() callers
  2016-02-04 14:23   ` [PATCH net] ipv4: fix memory leaks in ip_cmsg_send() callers Eric Dumazet
  2016-02-13 10:59     ` David Miller
@ 2016-02-15 20:11     ` Cong Wang
  1 sibling, 0 replies; 5+ messages in thread
From: Cong Wang @ 2016-02-15 20:11 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Dmitry Vyukov, David Miller, netdev

On Thu, Feb 4, 2016 at 6:23 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> Dmitry reported memory leaks of IP options allocated in
> ip_cmsg_send() when/if this function returns an error.
>
> Callers are responsible for the freeing.

Right, because there is a loop in ip_cmsg_send(), so the callers
are easier to free it than the callee.

The other thing is we perhaps have another leak in the following code:

        if (ipc.opt && ipc.opt->opt.srr) {
                if (!daddr)
                        return -EINVAL;
                faddr = ipc.opt->opt.faddr;
        }

since ipc.opt could be allocated on heap... We need something like:

@@ -770,8 +770,11 @@ static int ping_v4_sendmsg(struct sock *sk,
struct msghdr *msg, size_t len)
        ipc.addr = faddr = daddr;

        if (ipc.opt && ipc.opt->opt.srr) {
-               if (!daddr)
+               if (!daddr) {
+                       if (free)
+                               kfree(ipc.opt);
                        return -EINVAL;
+               }
                faddr = ipc.opt->opt.faddr;
        }
        tos = get_rttos(&ipc, inet);

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-02-15 20:11 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-04  9:47 net: memory leak in ip_cmsg_send Dmitry Vyukov
2016-02-04 14:04 ` Eric Dumazet
2016-02-04 14:23   ` [PATCH net] ipv4: fix memory leaks in ip_cmsg_send() callers Eric Dumazet
2016-02-13 10:59     ` David Miller
2016-02-15 20:11     ` Cong Wang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.