* 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 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).