net: fix stack out-of-bounds access in dump_one_netdev()

Submitted by Andrey Ryabinin on Jan. 30, 2017, 3:18 p.m.

Details

Message ID 20170130151815.9638-2-aryabinin@virtuozzo.com
State Accepted
Series "net: fix stack out-of-bounds access in dump_one_netdev()"
Commit 607784cff46910e6ef9b88f3966e1b60c15a58ce
Headers show

Commit Message

Andrey Ryabinin Jan. 30, 2017, 3:18 p.m. 
'info' array is off-by-one, nla_parse_nested() requires destination
array (i.e. 'info') to have maxtype+1 (i.e. IFLA_INFO_MAX+1) elements:

	ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffef823e3f8
	WRITE of size 48 at 0x7ffef823e3f8 thread T0
	    #0 0x7f9ab7a3915b in __asan_memset (/usr/lib/gcc/x86_64-pc-linux-gnu/5.4.0/libasan.so.2+0x8d15b)
	    #1 0x7f9ab6d4e553 in nla_parse (/usr/lib64/libnl-3.so.200+0xa553)
	    #2 0x4acfb7 in dump_one_netdev criu/net.c:445
	    #3 0x4adb60 in dump_one_ethernet criu/net.c:594
	    #4 0x4adb60 in dump_one_link criu/net.c:665
	    #5 0x48af69 in nlmsg_receive criu/libnetlink.c:45
	    #6 0x48af69 in do_rtnl_req criu/libnetlink.c:119
	    #7 0x4b0e86 in dump_links criu/net.c:878
	    #8 0x4b0e86 in dump_net_ns criu/net.c:1651
	    #9 0x4a760d in do_dump_namespaces criu/namespaces.c:985
	    #10 0x4a760d in dump_namespaces criu/namespaces.c:1045
	    #11 0x451ef7 in cr_dump_tasks criu/cr-dump.c:1799
	    #12 0x424588 in main criu/crtools.c:736
	    #13 0x7f9ab67b171f in __libc_start_main (/lib64/libc.so.6+0x2071f)
	    #14 0x4253d8 in _start (/criu/criu/criu+0x4253d8)

	Address 0x7ffef823e3f8 is located in stack of thread T0 at offset 264 in frame
	    #0 0x4ac9ef in dump_one_netdev criu/net.c:364

	  This frame has 5 object(s):
	    [32, 168) 'netdev'
	    [224, 264) 'info' <== Memory access at offset 264 overflows this variable
	    [320, 1040) 'req'
	    [1088, 3368) 'path'
	    [3424, 3625) 'stable_secret'

Increase 'info' size to fix this.

Fixes: b705dcc34ddb ("net: pass the struct nlattrs to dump() functions")
Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
---
 criu/net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Patch hide | download patch | download mbox 

diff --git a/criu/net.c b/criu/net.c
index 69658ff..f3919a7 100644
--- a/criu/net.c
+++ b/criu/net.c
@@ -373,7 +373,7 @@  static int dump_one_netdev(int type, struct ifinfomsg *ifi,
 	SysctlEntry *confs6 = NULL;
 	int size6 = ARRAY_SIZE(devconfs6);
 	char stable_secret[MAX_STR_CONF_LEN + 1] = {};
-	struct nlattr *info[IFLA_INFO_MAX], **arg = NULL;
+	struct nlattr *info[IFLA_INFO_MAX + 1], **arg = NULL;
 
 	if (!tb[IFLA_IFNAME]) {
 		pr_err("No name for link %d\n", ifi->ifi_index);

Comments

Cyrill Gorcunov Jan. 30, 2017, 3:49 p.m. 
On Mon, Jan 30, 2017 at 06:18:14PM +0300, Andrey Ryabinin wrote:
> 'info' array is off-by-one, nla_parse_nested() requires destination
> array (i.e. 'info') to have maxtype+1 (i.e. IFLA_INFO_MAX+1) elements:
> 
Acked-by: Cyrill Gorcunov <gorcunov@openvz.org>
Pavel Emelianov Jan. 31, 2017, 12:18 p.m. 
Applied