| 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: