net: ipv4: fix memory leak in ip_mc_add1_src

Message ID 20210616095925.1571600-1-cy.fan@huawei.com
State New
Headers show
Series
  • net: ipv4: fix memory leak in ip_mc_add1_src
Related show

Commit Message

Chengyang Fan June 16, 2021, 9:59 a.m.
BUG: memory leak
unreferenced object 0xffff888101bc4c00 (size 32):
  comm "syz-executor527", pid 360, jiffies 4294807421 (age 19.329s)
  hex dump (first 32 bytes):
    00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
    01 00 00 00 00 00 00 00 ac 14 14 bb 00 00 02 00 ................
  backtrace:
    [<00000000f17c5244>] kmalloc include/linux/slab.h:558 [inline]
    [<00000000f17c5244>] kzalloc include/linux/slab.h:688 [inline]
    [<00000000f17c5244>] ip_mc_add1_src net/ipv4/igmp.c:1971 [inline]
    [<00000000f17c5244>] ip_mc_add_src+0x95f/0xdb0 net/ipv4/igmp.c:2095
    [<000000001cb99709>] ip_mc_source+0x84c/0xea0 net/ipv4/igmp.c:2416
    [<0000000052cf19ed>] do_ip_setsockopt net/ipv4/ip_sockglue.c:1294 [inline]
    [<0000000052cf19ed>] ip_setsockopt+0x114b/0x30c0 net/ipv4/ip_sockglue.c:1423
    [<00000000477edfbc>] raw_setsockopt+0x13d/0x170 net/ipv4/raw.c:857
    [<00000000e75ca9bb>] __sys_setsockopt+0x158/0x270 net/socket.c:2117
    [<00000000bdb993a8>] __do_sys_setsockopt net/socket.c:2128 [inline]
    [<00000000bdb993a8>] __se_sys_setsockopt net/socket.c:2125 [inline]
    [<00000000bdb993a8>] __x64_sys_setsockopt+0xba/0x150 net/socket.c:2125
    [<000000006a1ffdbd>] do_syscall_64+0x40/0x80 arch/x86/entry/common.c:47
    [<00000000b11467c4>] entry_SYSCALL_64_after_hwframe+0x44/0xae

In commit 24803f38a5c0 ("igmp: do not remove igmp souce list info when set
link down"), the ip_mc_clear_src() in ip_mc_destroy_dev() was removed,
because it was also called in igmpv3_clear_delrec().

Rough callgraph:

inetdev_destroy
-> ip_mc_destroy_dev
     -> igmpv3_clear_delrec
        -> ip_mc_clear_src
-> RCU_INIT_POINTER(dev->ip_ptr, NULL)

However, ip_mc_clear_src() called in igmpv3_clear_delrec() doesn't
release in_dev->mc_list->sources. And RCU_INIT_POINTER() assigns the
NULL to dev->ip_ptr. As a result, in_dev cannot be obtained through
inetdev_by_index() and then in_dev->mc_list->sources cannot be released
by ip_mc_del1_src() in the sock_close. Rough call sequence goes like:

sock_close
-> __sock_release
   -> inet_release
      -> ip_mc_drop_socket
         -> inetdev_by_index
         -> ip_mc_leave_src
            -> ip_mc_del_src
               -> ip_mc_del1_src

So we still need to call ip_mc_clear_src() in ip_mc_destroy_dev() to free
in_dev->mc_list->sources.

Fixes: 24803f38a5c0 ("igmp: do not remove igmp souce list info ...")
Reported-by: Hulk Robot <hulkci@huawei.com>
Signed-off-by: Chengyang Fan <cy.fan@huawei.com>
---
 net/ipv4/igmp.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Hangbin Liu June 16, 2021, 11:52 a.m. | #1
On Wed, Jun 16, 2021 at 05:59:25PM +0800, Chengyang Fan wrote:
> BUG: memory leak
> unreferenced object 0xffff888101bc4c00 (size 32):
>   comm "syz-executor527", pid 360, jiffies 4294807421 (age 19.329s)
>   hex dump (first 32 bytes):
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
>     01 00 00 00 00 00 00 00 ac 14 14 bb 00 00 02 00 ................
>   backtrace:
>     [<00000000f17c5244>] kmalloc include/linux/slab.h:558 [inline]
>     [<00000000f17c5244>] kzalloc include/linux/slab.h:688 [inline]
>     [<00000000f17c5244>] ip_mc_add1_src net/ipv4/igmp.c:1971 [inline]
>     [<00000000f17c5244>] ip_mc_add_src+0x95f/0xdb0 net/ipv4/igmp.c:2095
>     [<000000001cb99709>] ip_mc_source+0x84c/0xea0 net/ipv4/igmp.c:2416
>     [<0000000052cf19ed>] do_ip_setsockopt net/ipv4/ip_sockglue.c:1294 [inline]
>     [<0000000052cf19ed>] ip_setsockopt+0x114b/0x30c0 net/ipv4/ip_sockglue.c:1423
>     [<00000000477edfbc>] raw_setsockopt+0x13d/0x170 net/ipv4/raw.c:857
>     [<00000000e75ca9bb>] __sys_setsockopt+0x158/0x270 net/socket.c:2117
>     [<00000000bdb993a8>] __do_sys_setsockopt net/socket.c:2128 [inline]
>     [<00000000bdb993a8>] __se_sys_setsockopt net/socket.c:2125 [inline]
>     [<00000000bdb993a8>] __x64_sys_setsockopt+0xba/0x150 net/socket.c:2125
>     [<000000006a1ffdbd>] do_syscall_64+0x40/0x80 arch/x86/entry/common.c:47
>     [<00000000b11467c4>] entry_SYSCALL_64_after_hwframe+0x44/0xae
> 
> In commit 24803f38a5c0 ("igmp: do not remove igmp souce list info when set
> link down"), the ip_mc_clear_src() in ip_mc_destroy_dev() was removed,
> because it was also called in igmpv3_clear_delrec().
> 
> Rough callgraph:
> 
> inetdev_destroy
> -> ip_mc_destroy_dev
>      -> igmpv3_clear_delrec
>         -> ip_mc_clear_src
> -> RCU_INIT_POINTER(dev->ip_ptr, NULL)
> 
> However, ip_mc_clear_src() called in igmpv3_clear_delrec() doesn't
> release in_dev->mc_list->sources. And RCU_INIT_POINTER() assigns the
> NULL to dev->ip_ptr. As a result, in_dev cannot be obtained through
> inetdev_by_index() and then in_dev->mc_list->sources cannot be released
> by ip_mc_del1_src() in the sock_close. Rough call sequence goes like:
> 
> sock_close
> -> __sock_release
>    -> inet_release
>       -> ip_mc_drop_socket
>          -> inetdev_by_index
>          -> ip_mc_leave_src
>             -> ip_mc_del_src
>                -> ip_mc_del1_src
> 
> So we still need to call ip_mc_clear_src() in ip_mc_destroy_dev() to free
> in_dev->mc_list->sources.
> 
> Fixes: 24803f38a5c0 ("igmp: do not remove igmp souce list info ...")
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: Chengyang Fan <cy.fan@huawei.com>
> ---
>  net/ipv4/igmp.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
> index 7b272bbed2b4..6b3c558a4f23 100644
> --- a/net/ipv4/igmp.c
> +++ b/net/ipv4/igmp.c
> @@ -1801,6 +1801,7 @@ void ip_mc_destroy_dev(struct in_device *in_dev)
>  	while ((i = rtnl_dereference(in_dev->mc_list)) != NULL) {
>  		in_dev->mc_list = i->next_rcu;
>  		in_dev->mc_count--;
> +		ip_mc_clear_src(i);
>  		ip_ma_put(i);
>  	}
>  }
> -- 
> 2.18.0.huawei.25
> 

Acked-by: Hangbin Liu <liuhangbin@gmail.com>
patchwork-bot+netdevbpf@kernel.org June 16, 2021, 7:50 p.m. | #2
Hello:

This patch was applied to netdev/net.git (refs/heads/master):

On Wed, 16 Jun 2021 17:59:25 +0800 you wrote:
> BUG: memory leak
> unreferenced object 0xffff888101bc4c00 (size 32):
>   comm "syz-executor527", pid 360, jiffies 4294807421 (age 19.329s)
>   hex dump (first 32 bytes):
>     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
>     01 00 00 00 00 00 00 00 ac 14 14 bb 00 00 02 00 ................
>   backtrace:
>     [<00000000f17c5244>] kmalloc include/linux/slab.h:558 [inline]
>     [<00000000f17c5244>] kzalloc include/linux/slab.h:688 [inline]
>     [<00000000f17c5244>] ip_mc_add1_src net/ipv4/igmp.c:1971 [inline]
>     [<00000000f17c5244>] ip_mc_add_src+0x95f/0xdb0 net/ipv4/igmp.c:2095
>     [<000000001cb99709>] ip_mc_source+0x84c/0xea0 net/ipv4/igmp.c:2416
>     [<0000000052cf19ed>] do_ip_setsockopt net/ipv4/ip_sockglue.c:1294 [inline]
>     [<0000000052cf19ed>] ip_setsockopt+0x114b/0x30c0 net/ipv4/ip_sockglue.c:1423
>     [<00000000477edfbc>] raw_setsockopt+0x13d/0x170 net/ipv4/raw.c:857
>     [<00000000e75ca9bb>] __sys_setsockopt+0x158/0x270 net/socket.c:2117
>     [<00000000bdb993a8>] __do_sys_setsockopt net/socket.c:2128 [inline]
>     [<00000000bdb993a8>] __se_sys_setsockopt net/socket.c:2125 [inline]
>     [<00000000bdb993a8>] __x64_sys_setsockopt+0xba/0x150 net/socket.c:2125
>     [<000000006a1ffdbd>] do_syscall_64+0x40/0x80 arch/x86/entry/common.c:47
>     [<00000000b11467c4>] entry_SYSCALL_64_after_hwframe+0x44/0xae
> 
> [...]

Here is the summary with links:
  - net: ipv4: fix memory leak in ip_mc_add1_src
    https://git.kernel.org/netdev/net/c/d8e2973029b8

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Patch

diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 7b272bbed2b4..6b3c558a4f23 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -1801,6 +1801,7 @@  void ip_mc_destroy_dev(struct in_device *in_dev)
 	while ((i = rtnl_dereference(in_dev->mc_list)) != NULL) {
 		in_dev->mc_list = i->next_rcu;
 		in_dev->mc_count--;
+		ip_mc_clear_src(i);
 		ip_ma_put(i);
 	}
 }