Re: KMSAN: kernel-infoleak in put_cmsg

On Tue, Jul 17, 2018 at 12:32 PM Willem de Bruijn
[off-list ref] wrote:

On Tue, Jul 17, 2018 at 6:25 AM syzbot
[off-list ref] wrote:

quoted

Hello,

syzbot found the following crash on:

HEAD commit:    123906095e30 kmsan: introduce kmsan_interrupt_enter()/kmsa..
git tree:       https://github.com/google/kmsan.git/master
console output: https://syzkaller.appspot.com/x/log.txt?x=166dafa0400000
kernel config:  https://syzkaller.appspot.com/x/.config?x=848e40757852af3e
dashboard link: https://syzkaller.appspot.com/bug?extid=9adb4b567003cac781f0
compiler:       clang version 7.0.0 (trunk 334104)
syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=164e4ab0400000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=15a41e40400000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+9adb4b567003cac781f0@syzkaller.appspotmail.com

random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
==================================================================
BUG: KMSAN: kernel-infoleak in copy_to_user include/linux/uaccess.h:184
[inline]
BUG: KMSAN: kernel-infoleak in put_cmsg+0x5ef/0x860 net/core/scm.c:242
CPU: 0 PID: 4501 Comm: syz-executor128 Not tainted 4.17.0+ #9
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:77 [inline]
  dump_stack+0x185/0x1d0 lib/dump_stack.c:113
  kmsan_report+0x188/0x2a0 mm/kmsan/kmsan.c:1125
  kmsan_internal_check_memory+0x138/0x1f0 mm/kmsan/kmsan.c:1219
  kmsan_copy_to_user+0x7a/0x160 mm/kmsan/kmsan.c:1261
  copy_to_user include/linux/uaccess.h:184 [inline]
  put_cmsg+0x5ef/0x860 net/core/scm.c:242
  ip6_datagram_recv_specific_ctl+0x1cf3/0x1eb0 net/ipv6/datagram.c:719

quoted

Bytes 2-3 of 24 are uninitialized
Memory access starts at ffff8801bde1f8a8

This socket requests IPV6_ORIGDSTADDR.

According to

  > Uninit was stored to memory at:
  >   ip6_datagram_recv_specific_ctl+0x1c3e/0x1eb0 net/ipv6/datagram.c:713
  >   ip6_datagram_recv_ctl+0x41c/0x450 net/ipv6/datagram.c:733

It is reading two uninitialized bytes at line

                        sin6.sin6_port = ports[1];

But this access is after the check

                __be16 *ports = (__be16 *) skb_transport_header(skb);

                if (skb_transport_offset(skb) + 4 <= (int)skb->len) {

and the sent packet is 725B.

The socket was opened with SOCK_RAW and protocol NEXTHDR_DEST.

  r0 = socket$inet6(0xa, 0x3, 0x3c)

so this is not a normal packet. Need to take a look at the contents.

The packet is generated in two stages with MSG_MORE. The first call
creates a zero-length packet, the second call appends the actual data.
Appends always happens in a frag (unless !SG). The existing test does
not catch this.

               if (skb_transport_offset(skb) + 4 <= (int)skb->len) {

Something like the following would be needed to ensure that the bytes
lie in the head.

-               __be16 *ports = (__be16 *) skb_transport_header(skb);
-
-               if (skb_transport_offset(skb) + 4 <= (int)skb->len) {

+               int off = skb_transport_offset(skb) + 4;
+
+               if (off <= 0 || pskb_may_pull(skb, off)) {
+                       __be16 *ports = (__be16 *) skb_transport_header(skb);

Here off can be negative, if the transport headers have already been
pulled, as in the case of UDP.

Casting the first four bytes to ports is really also not correct for
arbitrary protocols. This repro, for instance, has proto NEXTHDR_DEST.
This interface was perhaps not implemented with SOCK_RAW in mind;
either way, it's too late to exclude it now. But we can avoid the
__pskb_pull_tail and simply fail on odd packets like these:

-               if (skb_transport_offset(skb) + 4 <= (int)skb->len) {
+               if (skb_transport_offset(skb) + 4 <= (int) skb_headlen(skb)) {

From a quick read, IPv4 appears susceptible to this, too. Will take a look.