Re: KMSAN reports use of uninitialized memory in pfkey_sendmsg()

[Eric Biggers <ebiggers3@gmail.com>]

On Fri, Dec 29, 2017 at 05:49:34PM +0100, Dmitry Vyukov wrote:
> On Fri, Dec 29, 2017 at 5:48 PM, Alexander Potapenko <glider@google.com> wrote:
> > Hi all,
> >
> > KMSAN reports a use of uninitialized value on the following program:
> >
> > ==========================
> > // autogenerated by syzkaller (http://github.com/google/syzkaller)
> >
> > #include <string.h>
> > #include <sys/types.h>
> > #include <sys/socket.h>
> >
> > int main()
> > {
> >   int sock = socket(PF_KEY, SOCK_RAW, 2);
> >   char buf[24];
> >   memset(buf, 0, 24);
> >   buf[0] = 2;
> >   buf[1] = 4;
> >   buf[4] = 3;
> >   buf[16] = 1;
> >   buf[18] = 0x17;  // SADB_X_EXT_NAT_T_OA
> >   struct msghdr hdr;
> >   memset(&hdr, 0, sizeof(struct msghdr));
> >   struct iovec iov;
> >   hdr.msg_iov = &iov;
> >   hdr.msg_iovlen = 1;
> >   iov.iov_base = buf;
> >   iov.iov_len = 0x18;
> >   sendmsg(sock, &hdr, 0);
> > }
> > ==========================
> >
> > the report is below:
> >
> > ==================================================================
> > BUG: KMSAN: use of uninitialized memory in pfkey_sendmsg+0x11ad/0x1900
> > CPU: 0 PID: 2946 Comm: probe Not tainted 4.13.0+ #3626
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> > Call Trace:
> >  show_stack+0x12f/0x180 arch/x86/kernel/dumpstack.c:177
> >  __dump_stack lib/dump_stack.c:16
> >  dump_stack+0x185/0x1d0 lib/dump_stack.c:52
> >  kmsan_report+0x137/0x1c0 mm/kmsan/kmsan.c:1066
> >  __msan_warning_32+0x69/0xb0 mm/kmsan/kmsan_instr.c:676
> >  verify_address_len net/key/af_key.c:404
> >  parse_exthdrs net/key/af_key.c:532
> >  pfkey_process net/key/af_key.c:2811
> >  pfkey_sendmsg+0x11ad/0x1900 net/key/af_key.c:3654
> >  sock_sendmsg_nosec net/socket.c:633
> >  sock_sendmsg net/socket.c:643
> >  ___sys_sendmsg+0xed5/0x1330 net/socket.c:2035
> >  __sys_sendmsg net/socket.c:2069
> >  SYSC_sendmsg+0x2a6/0x3d0 net/socket.c:2080
> >  SyS_sendmsg+0x54/0x80 net/socket.c:2076
> >  do_syscall_64+0x2f4/0x420 arch/x86/entry/common.c:284
> >  entry_SYSCALL64_slow_path+0x25/0x25 arch/x86/entry/entry_64.S:245
> > RIP: 0033:0x401140
> > RSP: 002b:00007ffe52abc9e8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> > RAX: ffffffffffffffda RBX: 00000000004002b0 RCX: 0000000000401140
> > RDX: 0000000000000000 RSI: 00007ffe52abca10 RDI: 0000000000000003
> > RBP: 00007ffe52abca80 R08: 0000000000000002 R09: 0000000000000004
> > R10: 0000000000000004 R11: 0000000000000246 R12: 0000000000000000
> > R13: 00000000004063e0 R14: 0000000000406470 R15: 0000000000000000
> > origin:
> >  save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:63
> >  kmsan_save_stack_with_flags mm/kmsan/kmsan.c:303
> >  kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:213
> >  kmsan_kmalloc+0x94/0x100 mm/kmsan/kmsan.c:339
> >  kmsan_slab_alloc+0x11/0x20 mm/kmsan/kmsan.c:346
> >  slab_post_alloc_hook mm/slab.h:442
> >  slab_alloc_node mm/slub.c:2724
> >  __kmalloc_node_track_caller+0xa46/0x1230 mm/slub.c:4335
> >  __kmalloc_reserve net/core/skbuff.c:139
> >  __alloc_skb+0x2c6/0x9f0 net/core/skbuff.c:232
> >  alloc_skb ./include/linux/skbuff.h:904
> >  pfkey_sendmsg+0x201/0x1900 net/key/af_key.c:3641
> >  sock_sendmsg_nosec net/socket.c:633
> >  sock_sendmsg net/socket.c:643
> >  ___sys_sendmsg+0xed5/0x1330 net/socket.c:2035
> >  __sys_sendmsg net/socket.c:2069
> >  SYSC_sendmsg+0x2a6/0x3d0 net/socket.c:2080
> >  SyS_sendmsg+0x54/0x80 net/socket.c:2076
> >  do_syscall_64+0x2f4/0x420 arch/x86/entry/common.c:284
> >  return_from_SYSCALL_64+0x0/0x6a arch/x86/entry/entry_64.S:245
> > ==================================================================
> >
> > Apparently pfkey_sendmsg reads skb past the end of the buffer copied
> > from the userspace.
> > Could someone please take a look?
> 

Thanks for reporting this!  The problem is that verify_address_len() doesn't
verify that the buffer has space for the ->sa_family field before reading it.
I've sent out a patch.

I also noticed a similar bug in parse_exthdrs(); it doesn't verify that the
buffer has space for the 'struct sadb_ext' before reading it.  I've sent out a
patch for that as well.

Eric