Message ID | 20170523112028.132630-1-glider@google.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
On Tue, 2017-05-23 at 13:20 +0200, Alexander Potapenko wrote: > rtnl_fdb_dump() failed to check the result of nlmsg_parse(), which led > to contents of |ifm| being uninitialized because nlh->nlmsglen was too > small to accommodate |ifm|. The uninitialized data may affect some > branches and result in unwanted effects, although kernel data doesn't > seem to leak to the userspace directly. > > The bug has been detected with KMSAN and syzkaller. > > Signed-off-by: Alexander Potapenko <glider@google.com> > --- > For the record, here is the KMSAN report: > > ================================================================== > BUG: KMSAN: use of unitialized memory in rtnl_fdb_dump+0x5dc/0x1000 > CPU: 0 PID: 1039 Comm: probe Not tainted 4.11.0-rc5+ #2727 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 > Call Trace: > __dump_stack lib/dump_stack.c:16 > dump_stack+0x143/0x1b0 lib/dump_stack.c:52 > kmsan_report+0x12a/0x180 mm/kmsan/kmsan.c:1007 > __kmsan_warning_32+0x66/0xb0 mm/kmsan/kmsan_instr.c:491 > rtnl_fdb_dump+0x5dc/0x1000 net/core/rtnetlink.c:3230 > netlink_dump+0x84f/0x1190 net/netlink/af_netlink.c:2168 > __netlink_dump_start+0xc97/0xe50 net/netlink/af_netlink.c:2258 > netlink_dump_start ./include/linux/netlink.h:165 > rtnetlink_rcv_msg+0xae9/0xb40 net/core/rtnetlink.c:4094 > netlink_rcv_skb+0x339/0x5a0 net/netlink/af_netlink.c:2339 > rtnetlink_rcv+0x83/0xa0 net/core/rtnetlink.c:4110 > netlink_unicast_kernel net/netlink/af_netlink.c:1272 > netlink_unicast+0x13b7/0x1480 net/netlink/af_netlink.c:1298 > netlink_sendmsg+0x10b8/0x10f0 net/netlink/af_netlink.c:1844 > sock_sendmsg_nosec net/socket.c:633 > sock_sendmsg net/socket.c:643 > ___sys_sendmsg+0xd4b/0x10f0 net/socket.c:1997 > __sys_sendmsg net/socket.c:2031 > SYSC_sendmsg+0x2c6/0x3f0 net/socket.c:2042 > SyS_sendmsg+0x87/0xb0 net/socket.c:2038 > do_syscall_64+0x102/0x150 arch/x86/entry/common.c:285 > entry_SYSCALL64_slow_path+0x25/0x25 arch/x86/entry/entry_64.S:246 > RIP: 0033:0x401300 > RSP: 002b:00007ffc3b0e6d58 EFLAGS: 00000246 ORIG_RAX: 000000000000002e > RAX: ffffffffffffffda RBX: 00000000004002b0 RCX: 0000000000401300 > RDX: 0000000000000000 RSI: 00007ffc3b0e6d80 RDI: 0000000000000003 > RBP: 00007ffc3b0e6e00 R08: 000000000000000b R09: 0000000000000004 > R10: 000000000000000d R11: 0000000000000246 R12: 0000000000000000 > R13: 00000000004065a0 R14: 0000000000406630 R15: 0000000000000000 > origin: 000000008fe00056 > save_stack_trace+0x59/0x60 arch/x86/kernel/stacktrace.c:59 > kmsan_save_stack_with_flags mm/kmsan/kmsan.c:352 > kmsan_internal_poison_shadow+0xb1/0x1a0 mm/kmsan/kmsan.c:247 > kmsan_poison_shadow+0x6d/0xc0 mm/kmsan/kmsan.c:260 > slab_alloc_node mm/slub.c:2743 > __kmalloc_node_track_caller+0x1f4/0x390 mm/slub.c:4349 > __kmalloc_reserve net/core/skbuff.c:138 > __alloc_skb+0x2cd/0x740 net/core/skbuff.c:231 > alloc_skb ./include/linux/skbuff.h:933 > netlink_alloc_large_skb net/netlink/af_netlink.c:1144 > netlink_sendmsg+0x934/0x10f0 net/netlink/af_netlink.c:1819 > sock_sendmsg_nosec net/socket.c:633 > sock_sendmsg net/socket.c:643 > ___sys_sendmsg+0xd4b/0x10f0 net/socket.c:1997 > __sys_sendmsg net/socket.c:2031 > SYSC_sendmsg+0x2c6/0x3f0 net/socket.c:2042 > SyS_sendmsg+0x87/0xb0 net/socket.c:2038 > do_syscall_64+0x102/0x150 arch/x86/entry/common.c:285 > return_from_SYSCALL_64+0x0/0x6a arch/x86/entry/entry_64.S:246 > ================================================================== > > and the reproducer: > > ================================================================== > #include <sys/socket.h> > #include <net/if_arp.h> > #include <linux/netlink.h> > #include <stdint.h> > > int main() > { > int sock = socket(PF_NETLINK, SOCK_DGRAM | SOCK_NONBLOCK, 0); > struct msghdr msg; > memset(&msg, 0, sizeof(msg)); > char nlmsg_buf[32]; > memset(nlmsg_buf, 0, sizeof(nlmsg_buf)); > struct nlmsghdr *nlmsg = nlmsg_buf; > nlmsg->nlmsg_len = 0x11; > nlmsg->nlmsg_type = 0x1e; // RTM_NEWROUTE = RTM_BASE + 0x0e > // type = 0x0e = 1110b > // kind = 2 > nlmsg->nlmsg_flags = 0x101; // NLM_F_ROOT | NLM_F_REQUEST > nlmsg->nlmsg_seq = 0; > nlmsg->nlmsg_pid = 0; > nlmsg_buf[16] = (char)7; > struct iovec iov; > iov.iov_base = nlmsg_buf; > iov.iov_len = 17; > msg.msg_iov = &iov; > msg.msg_iovlen = 1; > sendmsg(sock, &msg, 0); > return 0; > } > ================================================================== > --- > net/core/rtnetlink.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c > index 49a279a7cc15..9e2c0a7cb325 100644 > --- a/net/core/rtnetlink.c > +++ b/net/core/rtnetlink.c > @@ -3231,8 +3231,11 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb) > int err = 0; > int fidx = 0; > > - if (nlmsg_parse(cb->nlh, sizeof(struct ifinfomsg), tb, > - IFLA_MAX, ifla_policy, NULL) == 0) { > + err = nlmsg_parse(cb->nlh, sizeof(struct ifinfomsg), tb, > + IFLA_MAX, ifla_policy, NULL); > + if (err < 0) { > + return -EINVAL; > + } else if (err == 0) { > if (tb[IFLA_MASTER]) > br_idx = nla_get_u32(tb[IFLA_MASTER]); > } Fix looks right to me. Reviewed-by: Greg Rose <gvrose8192@gmail.com>
From: Alexander Potapenko <glider@google.com> Date: Tue, 23 May 2017 13:20:28 +0200 > rtnl_fdb_dump() failed to check the result of nlmsg_parse(), which led > to contents of |ifm| being uninitialized because nlh->nlmsglen was too > small to accommodate |ifm|. The uninitialized data may affect some > branches and result in unwanted effects, although kernel data doesn't > seem to leak to the userspace directly. > > The bug has been detected with KMSAN and syzkaller. > > Signed-off-by: Alexander Potapenko <glider@google.com> Applied, thanks.
Hi David, I've noticed that the upstream patch: https://github.com/torvalds/linux/commit/0ff50e83b5122e836ca492fefb11656b225ac29c contains the KMSAN report and the repro, despite I've put them under the triple dash (IIRC Eric told me I shouldn't bloat the patch descriptions with that information). Did I mess it up somehow? Alex On Wed, May 24, 2017 at 9:32 PM, David Miller <davem@davemloft.net> wrote: > From: Alexander Potapenko <glider@google.com> > Date: Tue, 23 May 2017 13:20:28 +0200 > >> rtnl_fdb_dump() failed to check the result of nlmsg_parse(), which led >> to contents of |ifm| being uninitialized because nlh->nlmsglen was too >> small to accommodate |ifm|. The uninitialized data may affect some >> branches and result in unwanted effects, although kernel data doesn't >> seem to leak to the userspace directly. >> >> The bug has been detected with KMSAN and syzkaller. >> >> Signed-off-by: Alexander Potapenko <glider@google.com> > > Applied, thanks.
From: Alexander Potapenko <glider@google.com> Date: Wed, 31 May 2017 10:56:47 +0200 > Hi David, > > I've noticed that the upstream patch: > https://github.com/torvalds/linux/commit/0ff50e83b5122e836ca492fefb11656b225ac29c > contains the KMSAN report and the repro, despite I've put them under > the triple dash (IIRC Eric told me I shouldn't bloat the patch > descriptions with that information). Did I mess it up somehow? I put them into the main commit log message by hand when I applied the patch, I think the more information in commit log messages the better.
On Wed, May 31, 2017 at 5:10 PM, David Miller <davem@davemloft.net> wrote: > From: Alexander Potapenko <glider@google.com> > Date: Wed, 31 May 2017 10:56:47 +0200 > >> Hi David, >> >> I've noticed that the upstream patch: >> https://github.com/torvalds/linux/commit/0ff50e83b5122e836ca492fefb11656b225ac29c >> contains the KMSAN report and the repro, despite I've put them under >> the triple dash (IIRC Eric told me I shouldn't bloat the patch >> descriptions with that information). Did I mess it up somehow? > > I put them into the main commit log message by hand when I applied the > patch, I think the more information in commit log messages the better. Ok, thanks for clarifying!
================================================================== BUG: KMSAN: use of unitialized memory in rtnl_fdb_dump+0x5dc/0x1000 CPU: 0 PID: 1039 Comm: probe Not tainted 4.11.0-rc5+ #2727 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:16 dump_stack+0x143/0x1b0 lib/dump_stack.c:52 kmsan_report+0x12a/0x180 mm/kmsan/kmsan.c:1007 __kmsan_warning_32+0x66/0xb0 mm/kmsan/kmsan_instr.c:491 rtnl_fdb_dump+0x5dc/0x1000 net/core/rtnetlink.c:3230 netlink_dump+0x84f/0x1190 net/netlink/af_netlink.c:2168 __netlink_dump_start+0xc97/0xe50 net/netlink/af_netlink.c:2258 netlink_dump_start ./include/linux/netlink.h:165 rtnetlink_rcv_msg+0xae9/0xb40 net/core/rtnetlink.c:4094 netlink_rcv_skb+0x339/0x5a0 net/netlink/af_netlink.c:2339 rtnetlink_rcv+0x83/0xa0 net/core/rtnetlink.c:4110 netlink_unicast_kernel net/netlink/af_netlink.c:1272 netlink_unicast+0x13b7/0x1480 net/netlink/af_netlink.c:1298 netlink_sendmsg+0x10b8/0x10f0 net/netlink/af_netlink.c:1844 sock_sendmsg_nosec net/socket.c:633 sock_sendmsg net/socket.c:643 ___sys_sendmsg+0xd4b/0x10f0 net/socket.c:1997 __sys_sendmsg net/socket.c:2031 SYSC_sendmsg+0x2c6/0x3f0 net/socket.c:2042 SyS_sendmsg+0x87/0xb0 net/socket.c:2038 do_syscall_64+0x102/0x150 arch/x86/entry/common.c:285 entry_SYSCALL64_slow_path+0x25/0x25 arch/x86/entry/entry_64.S:246 RIP: 0033:0x401300 RSP: 002b:00007ffc3b0e6d58 EFLAGS: 00000246 ORIG_RAX: 000000000000002e RAX: ffffffffffffffda RBX: 00000000004002b0 RCX: 0000000000401300 RDX: 0000000000000000 RSI: 00007ffc3b0e6d80 RDI: 0000000000000003 RBP: 00007ffc3b0e6e00 R08: 000000000000000b R09: 0000000000000004 R10: 000000000000000d R11: 0000000000000246 R12: 0000000000000000 R13: 00000000004065a0 R14: 0000000000406630 R15: 0000000000000000 origin: 000000008fe00056 save_stack_trace+0x59/0x60 arch/x86/kernel/stacktrace.c:59 kmsan_save_stack_with_flags mm/kmsan/kmsan.c:352 kmsan_internal_poison_shadow+0xb1/0x1a0 mm/kmsan/kmsan.c:247 kmsan_poison_shadow+0x6d/0xc0 mm/kmsan/kmsan.c:260 slab_alloc_node mm/slub.c:2743 __kmalloc_node_track_caller+0x1f4/0x390 mm/slub.c:4349 __kmalloc_reserve net/core/skbuff.c:138 __alloc_skb+0x2cd/0x740 net/core/skbuff.c:231 alloc_skb ./include/linux/skbuff.h:933 netlink_alloc_large_skb net/netlink/af_netlink.c:1144 netlink_sendmsg+0x934/0x10f0 net/netlink/af_netlink.c:1819 sock_sendmsg_nosec net/socket.c:633 sock_sendmsg net/socket.c:643 ___sys_sendmsg+0xd4b/0x10f0 net/socket.c:1997 __sys_sendmsg net/socket.c:2031 SYSC_sendmsg+0x2c6/0x3f0 net/socket.c:2042 SyS_sendmsg+0x87/0xb0 net/socket.c:2038 do_syscall_64+0x102/0x150 arch/x86/entry/common.c:285 return_from_SYSCALL_64+0x0/0x6a arch/x86/entry/entry_64.S:246 ================================================================== and the reproducer: ================================================================== #include <sys/socket.h> #include <net/if_arp.h> #include <linux/netlink.h> #include <stdint.h> int main() { int sock = socket(PF_NETLINK, SOCK_DGRAM | SOCK_NONBLOCK, 0); struct msghdr msg; memset(&msg, 0, sizeof(msg)); char nlmsg_buf[32]; memset(nlmsg_buf, 0, sizeof(nlmsg_buf)); struct nlmsghdr *nlmsg = nlmsg_buf; nlmsg->nlmsg_len = 0x11; nlmsg->nlmsg_type = 0x1e; // RTM_NEWROUTE = RTM_BASE + 0x0e // type = 0x0e = 1110b // kind = 2 nlmsg->nlmsg_flags = 0x101; // NLM_F_ROOT | NLM_F_REQUEST nlmsg->nlmsg_seq = 0; nlmsg->nlmsg_pid = 0; nlmsg_buf[16] = (char)7; struct iovec iov; iov.iov_base = nlmsg_buf; iov.iov_len = 17; msg.msg_iov = &iov; msg.msg_iovlen = 1; sendmsg(sock, &msg, 0); return 0; } ================================================================== --- net/core/rtnetlink.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c index 49a279a7cc15..9e2c0a7cb325 100644 --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -3231,8 +3231,11 @@ static int rtnl_fdb_dump(struct sk_buff *skb, struct netlink_callback *cb) int err = 0; int fidx = 0; - if (nlmsg_parse(cb->nlh, sizeof(struct ifinfomsg), tb, - IFLA_MAX, ifla_policy, NULL) == 0) { + err = nlmsg_parse(cb->nlh, sizeof(struct ifinfomsg), tb, + IFLA_MAX, ifla_policy, NULL); + if (err < 0) { + return -EINVAL; + } else if (err == 0) { if (tb[IFLA_MASTER]) br_idx = nla_get_u32(tb[IFLA_MASTER]); }
rtnl_fdb_dump() failed to check the result of nlmsg_parse(), which led to contents of |ifm| being uninitialized because nlh->nlmsglen was too small to accommodate |ifm|. The uninitialized data may affect some branches and result in unwanted effects, although kernel data doesn't seem to leak to the userspace directly. The bug has been detected with KMSAN and syzkaller. Signed-off-by: Alexander Potapenko <glider@google.com> --- For the record, here is the KMSAN report: