netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* KMSAN reports use of uninitialized memory in pfkey_sendmsg()
@ 2017-12-29 16:48 Alexander Potapenko
  2017-12-29 16:49 ` Dmitry Vyukov
  0 siblings, 1 reply; 5+ messages in thread
From: Alexander Potapenko @ 2017-12-29 16:48 UTC (permalink / raw)
  To: David Miller, Steffen Klassert, Herbert Xu
  Cc: Networking, syzkaller, Dmitriy Vyukov, Kostya Serebryany

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,
-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: KMSAN reports use of uninitialized memory in pfkey_sendmsg()
  2017-12-29 16:48 KMSAN reports use of uninitialized memory in pfkey_sendmsg() Alexander Potapenko
@ 2017-12-29 16:49 ` Dmitry Vyukov
  2017-12-30  0:13   ` [PATCH] af_key: fix buffer overread in verify_address_len() Eric Biggers
  2017-12-30  0:19   ` KMSAN reports use of uninitialized memory in pfkey_sendmsg() Eric Biggers
  0 siblings, 2 replies; 5+ messages in thread
From: Dmitry Vyukov @ 2017-12-29 16:49 UTC (permalink / raw)
  To: Alexander Potapenko
  Cc: David Miller, Steffen Klassert, Herbert Xu, Networking, syzkaller,
	Kostya Serebryany, Eric Biggers

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?

+Eric

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH] af_key: fix buffer overread in verify_address_len()
  2017-12-29 16:49 ` Dmitry Vyukov
@ 2017-12-30  0:13   ` Eric Biggers
  2017-12-31  7:51     ` Steffen Klassert
  2017-12-30  0:19   ` KMSAN reports use of uninitialized memory in pfkey_sendmsg() Eric Biggers
  1 sibling, 1 reply; 5+ messages in thread
From: Eric Biggers @ 2017-12-30  0:13 UTC (permalink / raw)
  To: netdev, Steffen Klassert, Herbert Xu, David S . Miller
  Cc: Alexander Potapenko, Dmitry Vyukov, Kostya Serebryany, syzkaller,
	Eric Biggers, stable

From: Eric Biggers <ebiggers@google.com>

If a message sent to a PF_KEY socket ended with one of the extensions
that takes a 'struct sadb_address' but there were not enough bytes
remaining in the message for the ->sa_family member of the 'struct
sockaddr' which is supposed to follow, then verify_address_len() read
past the end of the message, into uninitialized memory.  Fix it by
returning -EINVAL in this case.

This bug was found using syzkaller with KMSAN.

Reproducer:

	#include <linux/pfkeyv2.h>
	#include <sys/socket.h>
	#include <unistd.h>

	int main()
	{
		int sock = socket(PF_KEY, SOCK_RAW, PF_KEY_V2);
		char buf[24] = { 0 };
		struct sadb_msg *msg = (void *)buf;
		struct sadb_address *addr = (void *)(msg + 1);

		msg->sadb_msg_version = PF_KEY_V2;
		msg->sadb_msg_type = SADB_DELETE;
		msg->sadb_msg_len = 3;
		addr->sadb_address_len = 1;
		addr->sadb_address_exttype = SADB_EXT_ADDRESS_SRC;

		write(sock, buf, 24);
	}

Reported-by: Alexander Potapenko <glider@google.com>
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 net/key/af_key.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/key/af_key.c b/net/key/af_key.c
index 3dffb892d52c..596499cc8b2f 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -401,6 +401,11 @@ static int verify_address_len(const void *p)
 #endif
 	int len;
 
+	if (sp->sadb_address_len <
+	    DIV_ROUND_UP(sizeof(*sp) + offsetofend(typeof(*addr), sa_family),
+			 sizeof(uint64_t)))
+		return -EINVAL;
+
 	switch (addr->sa_family) {
 	case AF_INET:
 		len = DIV_ROUND_UP(sizeof(*sp) + sizeof(*sin), sizeof(uint64_t));
-- 
2.15.1

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: KMSAN reports use of uninitialized memory in pfkey_sendmsg()
  2017-12-29 16:49 ` Dmitry Vyukov
  2017-12-30  0:13   ` [PATCH] af_key: fix buffer overread in verify_address_len() Eric Biggers
@ 2017-12-30  0:19   ` Eric Biggers
  1 sibling, 0 replies; 5+ messages in thread
From: Eric Biggers @ 2017-12-30  0:19 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Alexander Potapenko, David Miller, Steffen Klassert, Herbert Xu,
	Networking, syzkaller, Kostya Serebryany

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] af_key: fix buffer overread in verify_address_len()
  2017-12-30  0:13   ` [PATCH] af_key: fix buffer overread in verify_address_len() Eric Biggers
@ 2017-12-31  7:51     ` Steffen Klassert
  0 siblings, 0 replies; 5+ messages in thread
From: Steffen Klassert @ 2017-12-31  7:51 UTC (permalink / raw)
  To: Eric Biggers
  Cc: netdev, Herbert Xu, David S . Miller, Alexander Potapenko,
	Dmitry Vyukov, Kostya Serebryany, syzkaller, Eric Biggers, stable

On Fri, Dec 29, 2017 at 06:13:05PM -0600, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> If a message sent to a PF_KEY socket ended with one of the extensions
> that takes a 'struct sadb_address' but there were not enough bytes
> remaining in the message for the ->sa_family member of the 'struct
> sockaddr' which is supposed to follow, then verify_address_len() read
> past the end of the message, into uninitialized memory.  Fix it by
> returning -EINVAL in this case.
> 
> This bug was found using syzkaller with KMSAN.
> 
> Reproducer:
> 
> 	#include <linux/pfkeyv2.h>
> 	#include <sys/socket.h>
> 	#include <unistd.h>
> 
> 	int main()
> 	{
> 		int sock = socket(PF_KEY, SOCK_RAW, PF_KEY_V2);
> 		char buf[24] = { 0 };
> 		struct sadb_msg *msg = (void *)buf;
> 		struct sadb_address *addr = (void *)(msg + 1);
> 
> 		msg->sadb_msg_version = PF_KEY_V2;
> 		msg->sadb_msg_type = SADB_DELETE;
> 		msg->sadb_msg_len = 3;
> 		addr->sadb_address_len = 1;
> 		addr->sadb_address_exttype = SADB_EXT_ADDRESS_SRC;
> 
> 		write(sock, buf, 24);
> 	}
> 
> Reported-by: Alexander Potapenko <glider@google.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Applied to the ipsec tree, thanks Eric!

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-12-31  7:51 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-29 16:48 KMSAN reports use of uninitialized memory in pfkey_sendmsg() Alexander Potapenko
2017-12-29 16:49 ` Dmitry Vyukov
2017-12-30  0:13   ` [PATCH] af_key: fix buffer overread in verify_address_len() Eric Biggers
2017-12-31  7:51     ` Steffen Klassert
2017-12-30  0:19   ` KMSAN reports use of uninitialized memory in pfkey_sendmsg() Eric Biggers

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).