linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* KMSAN: uninit-value in pppoe_rcv
@ 2018-09-12 10:24 syzbot
  2018-09-12 10:38 ` Alexander Potapenko
  0 siblings, 1 reply; 10+ messages in thread
From: syzbot @ 2018-09-12 10:24 UTC (permalink / raw)
  To: linux-kernel, mostrows, netdev, syzkaller-bugs

Hello,

syzbot found the following crash on:

HEAD commit:    d2d741e5d189 kmsan: add initialization for shmem pages
git tree:       https://github.com/google/kmsan.git/master
console output: https://syzkaller.appspot.com/x/log.txt?x=1465fc37800000
kernel config:  https://syzkaller.appspot.com/x/.config?x=48f9de3384bcd0f
dashboard link: https://syzkaller.appspot.com/bug?extid=f5f6080811c849739212
compiler:       clang version 7.0.0 (trunk 329391)
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14d6e607800000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=10a15b5b800000

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

IPVS: ftp: loaded support on port[0] = 21
==================================================================
BUG: KMSAN: uninit-value in __get_item drivers/net/ppp/pppoe.c:172 [inline]
BUG: KMSAN: uninit-value in get_item drivers/net/ppp/pppoe.c:236 [inline]
BUG: KMSAN: uninit-value in pppoe_rcv+0xcef/0x10e0  
drivers/net/ppp/pppoe.c:450
CPU: 0 PID: 4543 Comm: syz-executor355 Not tainted 4.16.0+ #87
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
Call Trace:
  __dump_stack lib/dump_stack.c:17 [inline]
  dump_stack+0x185/0x1d0 lib/dump_stack.c:53
  kmsan_report+0x142/0x240 mm/kmsan/kmsan.c:1067
  __msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:683
  __get_item drivers/net/ppp/pppoe.c:172 [inline]
  get_item drivers/net/ppp/pppoe.c:236 [inline]
  pppoe_rcv+0xcef/0x10e0 drivers/net/ppp/pppoe.c:450
  __netif_receive_skb_core+0x47df/0x4a90 net/core/dev.c:4562
  __netif_receive_skb net/core/dev.c:4627 [inline]
  netif_receive_skb_internal+0x49d/0x630 net/core/dev.c:4701
  netif_receive_skb+0x230/0x240 net/core/dev.c:4725
  tun_rx_batched drivers/net/tun.c:1555 [inline]
  tun_get_user+0x740f/0x7c60 drivers/net/tun.c:1962
  tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990
  call_write_iter include/linux/fs.h:1782 [inline]
  new_sync_write fs/read_write.c:469 [inline]
  __vfs_write+0x7fb/0x9f0 fs/read_write.c:482
  vfs_write+0x463/0x8d0 fs/read_write.c:544
  SYSC_write+0x172/0x360 fs/read_write.c:589
  SyS_write+0x55/0x80 fs/read_write.c:581
  do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
  entry_SYSCALL_64_after_hwframe+0x3d/0xa2
RIP: 0033:0x4447c9
RSP: 002b:00007fff64c8fc28 EFLAGS: 00000297 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00000000004447c9
RDX: 000000000000fd87 RSI: 0000000020000600 RDI: 0000000000000004
RBP: 00000000006cf018 R08: 00007fff64c8fda8 R09: 00007fff00006bda
R10: 0000000000005fe7 R11: 0000000000000297 R12: 00000000004020d0
R13: 0000000000402160 R14: 0000000000000000 R15: 0000000000000000

Uninit was created at:
  kmsan_save_stack_with_flags mm/kmsan/kmsan.c:278 [inline]
  kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:188
  kmsan_kmalloc+0x94/0x100 mm/kmsan/kmsan.c:314
  kmsan_slab_alloc+0x11/0x20 mm/kmsan/kmsan.c:321
  slab_post_alloc_hook mm/slab.h:445 [inline]
  slab_alloc_node mm/slub.c:2737 [inline]
  __kmalloc_node_track_caller+0xaed/0x11c0 mm/slub.c:4369
  __kmalloc_reserve net/core/skbuff.c:138 [inline]
  __alloc_skb+0x2cf/0x9f0 net/core/skbuff.c:206
  alloc_skb include/linux/skbuff.h:984 [inline]
  alloc_skb_with_frags+0x1d4/0xb20 net/core/skbuff.c:5234
  sock_alloc_send_pskb+0xb56/0x1190 net/core/sock.c:2085
  tun_alloc_skb drivers/net/tun.c:1532 [inline]
  tun_get_user+0x2242/0x7c60 drivers/net/tun.c:1829
  tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990
  call_write_iter include/linux/fs.h:1782 [inline]
  new_sync_write fs/read_write.c:469 [inline]
  __vfs_write+0x7fb/0x9f0 fs/read_write.c:482
  vfs_write+0x463/0x8d0 fs/read_write.c:544
  SYSC_write+0x172/0x360 fs/read_write.c:589
  SyS_write+0x55/0x80 fs/read_write.c:581
  do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
  entry_SYSCALL_64_after_hwframe+0x3d/0xa2
==================================================================


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with  
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: KMSAN: uninit-value in pppoe_rcv
  2018-09-12 10:24 KMSAN: uninit-value in pppoe_rcv syzbot
@ 2018-09-12 10:38 ` Alexander Potapenko
  2018-09-13 13:57   ` Eric Dumazet
  0 siblings, 1 reply; 10+ messages in thread
From: Alexander Potapenko @ 2018-09-12 10:38 UTC (permalink / raw)
  To: syzbot+f5f6080811c849739212; +Cc: LKML, mostrows, Networking, syzkaller-bugs

On Wed, Sep 12, 2018 at 12:24 PM syzbot
<syzbot+f5f6080811c849739212@syzkaller.appspotmail.com> wrote:
>
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:    d2d741e5d189 kmsan: add initialization for shmem pages
> git tree:       https://github.com/google/kmsan.git/master
> console output: https://syzkaller.appspot.com/x/log.txt?x=1465fc37800000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=48f9de3384bcd0f
> dashboard link: https://syzkaller.appspot.com/bug?extid=f5f6080811c849739212
> compiler:       clang version 7.0.0 (trunk 329391)
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14d6e607800000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=10a15b5b800000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+f5f6080811c849739212@syzkaller.appspotmail.com
>
> IPVS: ftp: loaded support on port[0] = 21
> ==================================================================
> BUG: KMSAN: uninit-value in __get_item drivers/net/ppp/pppoe.c:172 [inline]
> BUG: KMSAN: uninit-value in get_item drivers/net/ppp/pppoe.c:236 [inline]
> BUG: KMSAN: uninit-value in pppoe_rcv+0xcef/0x10e0
> drivers/net/ppp/pppoe.c:450
> CPU: 0 PID: 4543 Comm: syz-executor355 Not tainted 4.16.0+ #87
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> Google 01/01/2011
> Call Trace:
>   __dump_stack lib/dump_stack.c:17 [inline]
>   dump_stack+0x185/0x1d0 lib/dump_stack.c:53
>   kmsan_report+0x142/0x240 mm/kmsan/kmsan.c:1067
>   __msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:683
>   __get_item drivers/net/ppp/pppoe.c:172 [inline]
>   get_item drivers/net/ppp/pppoe.c:236 [inline]
>   pppoe_rcv+0xcef/0x10e0 drivers/net/ppp/pppoe.c:450
>   __netif_receive_skb_core+0x47df/0x4a90 net/core/dev.c:4562
>   __netif_receive_skb net/core/dev.c:4627 [inline]
>   netif_receive_skb_internal+0x49d/0x630 net/core/dev.c:4701
>   netif_receive_skb+0x230/0x240 net/core/dev.c:4725
>   tun_rx_batched drivers/net/tun.c:1555 [inline]
>   tun_get_user+0x740f/0x7c60 drivers/net/tun.c:1962
>   tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990
>   call_write_iter include/linux/fs.h:1782 [inline]
>   new_sync_write fs/read_write.c:469 [inline]
>   __vfs_write+0x7fb/0x9f0 fs/read_write.c:482
>   vfs_write+0x463/0x8d0 fs/read_write.c:544
>   SYSC_write+0x172/0x360 fs/read_write.c:589
>   SyS_write+0x55/0x80 fs/read_write.c:581
>   do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
>   entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> RIP: 0033:0x4447c9
> RSP: 002b:00007fff64c8fc28 EFLAGS: 00000297 ORIG_RAX: 0000000000000001
> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00000000004447c9
> RDX: 000000000000fd87 RSI: 0000000020000600 RDI: 0000000000000004
> RBP: 00000000006cf018 R08: 00007fff64c8fda8 R09: 00007fff00006bda
> R10: 0000000000005fe7 R11: 0000000000000297 R12: 00000000004020d0
> R13: 0000000000402160 R14: 0000000000000000 R15: 0000000000000000
>
> Uninit was created at:
>   kmsan_save_stack_with_flags mm/kmsan/kmsan.c:278 [inline]
>   kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:188
>   kmsan_kmalloc+0x94/0x100 mm/kmsan/kmsan.c:314
>   kmsan_slab_alloc+0x11/0x20 mm/kmsan/kmsan.c:321
>   slab_post_alloc_hook mm/slab.h:445 [inline]
>   slab_alloc_node mm/slub.c:2737 [inline]
>   __kmalloc_node_track_caller+0xaed/0x11c0 mm/slub.c:4369
>   __kmalloc_reserve net/core/skbuff.c:138 [inline]
>   __alloc_skb+0x2cf/0x9f0 net/core/skbuff.c:206
>   alloc_skb include/linux/skbuff.h:984 [inline]
>   alloc_skb_with_frags+0x1d4/0xb20 net/core/skbuff.c:5234
>   sock_alloc_send_pskb+0xb56/0x1190 net/core/sock.c:2085
>   tun_alloc_skb drivers/net/tun.c:1532 [inline]
>   tun_get_user+0x2242/0x7c60 drivers/net/tun.c:1829
>   tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990
>   call_write_iter include/linux/fs.h:1782 [inline]
>   new_sync_write fs/read_write.c:469 [inline]
>   __vfs_write+0x7fb/0x9f0 fs/read_write.c:482
>   vfs_write+0x463/0x8d0 fs/read_write.c:544
>   SYSC_write+0x172/0x360 fs/read_write.c:589
>   SyS_write+0x55/0x80 fs/read_write.c:581
>   do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
>   entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> ==================================================================
I did a little digging before sending the bug upstream.
If I add memset(obj, 0xfe, size) to __kmalloc_reserve(), these 0xfe
bytes are visible in __get_item() at the place where KMSAN reports an
error.

The problem is somehow related to tun_get_user() creating a fragmented
sk_buff - when I change the call to tun_alloc_skb() so that it
allocates a single buffer the bug goes away.

>
> ---
> This bug is generated by a bot. It may contain errors.
> See https://goo.gl/tpsmEJ for more information about syzbot.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
>
> syzbot will keep track of this bug report. See:
> https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
> syzbot.
> syzbot can test patches for this bug, for details see:
> https://goo.gl/tpsmEJ#testing-patches
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/0000000000004624c30575a9fd40%40google.com.
> For more options, visit https://groups.google.com/d/optout.



-- 
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] 10+ messages in thread
* Re: KMSAN: uninit-value in pppoe_rcv
  2018-09-12 10:38 ` Alexander Potapenko
@ 2018-09-13 13:57   ` Eric Dumazet
  2018-09-13 14:12     ` Alexander Potapenko
  2018-09-18 16:52     ` Guillaume Nault
  0 siblings, 2 replies; 10+ messages in thread
From: Eric Dumazet @ 2018-09-13 13:57 UTC (permalink / raw)
  To: Alexander Potapenko, syzbot+f5f6080811c849739212
  Cc: LKML, mostrows, Networking, syzkaller-bugs



On 09/12/2018 03:38 AM, Alexander Potapenko wrote:
> On Wed, Sep 12, 2018 at 12:24 PM syzbot
> <syzbot+f5f6080811c849739212@syzkaller.appspotmail.com> wrote:
>>
>> Hello,
>>
>> syzbot found the following crash on:
>>
>> HEAD commit:    d2d741e5d189 kmsan: add initialization for shmem pages
>> git tree:       https://github.com/google/kmsan.git/master
>> console output: https://syzkaller.appspot.com/x/log.txt?x=1465fc37800000
>> kernel config:  https://syzkaller.appspot.com/x/.config?x=48f9de3384bcd0f
>> dashboard link: https://syzkaller.appspot.com/bug?extid=f5f6080811c849739212
>> compiler:       clang version 7.0.0 (trunk 329391)
>> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14d6e607800000
>> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=10a15b5b800000
>>
>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> Reported-by: syzbot+f5f6080811c849739212@syzkaller.appspotmail.com
>>
>> IPVS: ftp: loaded support on port[0] = 21
>> ==================================================================
>> BUG: KMSAN: uninit-value in __get_item drivers/net/ppp/pppoe.c:172 [inline]
>> BUG: KMSAN: uninit-value in get_item drivers/net/ppp/pppoe.c:236 [inline]
>> BUG: KMSAN: uninit-value in pppoe_rcv+0xcef/0x10e0
>> drivers/net/ppp/pppoe.c:450
>> CPU: 0 PID: 4543 Comm: syz-executor355 Not tainted 4.16.0+ #87
>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
>> Google 01/01/2011
>> Call Trace:
>>   __dump_stack lib/dump_stack.c:17 [inline]
>>   dump_stack+0x185/0x1d0 lib/dump_stack.c:53
>>   kmsan_report+0x142/0x240 mm/kmsan/kmsan.c:1067
>>   __msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:683
>>   __get_item drivers/net/ppp/pppoe.c:172 [inline]
>>   get_item drivers/net/ppp/pppoe.c:236 [inline]
>>   pppoe_rcv+0xcef/0x10e0 drivers/net/ppp/pppoe.c:450
>>   __netif_receive_skb_core+0x47df/0x4a90 net/core/dev.c:4562
>>   __netif_receive_skb net/core/dev.c:4627 [inline]
>>   netif_receive_skb_internal+0x49d/0x630 net/core/dev.c:4701
>>   netif_receive_skb+0x230/0x240 net/core/dev.c:4725
>>   tun_rx_batched drivers/net/tun.c:1555 [inline]
>>   tun_get_user+0x740f/0x7c60 drivers/net/tun.c:1962
>>   tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990
>>   call_write_iter include/linux/fs.h:1782 [inline]
>>   new_sync_write fs/read_write.c:469 [inline]
>>   __vfs_write+0x7fb/0x9f0 fs/read_write.c:482
>>   vfs_write+0x463/0x8d0 fs/read_write.c:544
>>   SYSC_write+0x172/0x360 fs/read_write.c:589
>>   SyS_write+0x55/0x80 fs/read_write.c:581
>>   do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
>>   entry_SYSCALL_64_after_hwframe+0x3d/0xa2
>> RIP: 0033:0x4447c9
>> RSP: 002b:00007fff64c8fc28 EFLAGS: 00000297 ORIG_RAX: 0000000000000001
>> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00000000004447c9
>> RDX: 000000000000fd87 RSI: 0000000020000600 RDI: 0000000000000004
>> RBP: 00000000006cf018 R08: 00007fff64c8fda8 R09: 00007fff00006bda
>> R10: 0000000000005fe7 R11: 0000000000000297 R12: 00000000004020d0
>> R13: 0000000000402160 R14: 0000000000000000 R15: 0000000000000000
>>
>> Uninit was created at:
>>   kmsan_save_stack_with_flags mm/kmsan/kmsan.c:278 [inline]
>>   kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:188
>>   kmsan_kmalloc+0x94/0x100 mm/kmsan/kmsan.c:314
>>   kmsan_slab_alloc+0x11/0x20 mm/kmsan/kmsan.c:321
>>   slab_post_alloc_hook mm/slab.h:445 [inline]
>>   slab_alloc_node mm/slub.c:2737 [inline]
>>   __kmalloc_node_track_caller+0xaed/0x11c0 mm/slub.c:4369
>>   __kmalloc_reserve net/core/skbuff.c:138 [inline]
>>   __alloc_skb+0x2cf/0x9f0 net/core/skbuff.c:206
>>   alloc_skb include/linux/skbuff.h:984 [inline]
>>   alloc_skb_with_frags+0x1d4/0xb20 net/core/skbuff.c:5234
>>   sock_alloc_send_pskb+0xb56/0x1190 net/core/sock.c:2085
>>   tun_alloc_skb drivers/net/tun.c:1532 [inline]
>>   tun_get_user+0x2242/0x7c60 drivers/net/tun.c:1829
>>   tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990
>>   call_write_iter include/linux/fs.h:1782 [inline]
>>   new_sync_write fs/read_write.c:469 [inline]
>>   __vfs_write+0x7fb/0x9f0 fs/read_write.c:482
>>   vfs_write+0x463/0x8d0 fs/read_write.c:544
>>   SYSC_write+0x172/0x360 fs/read_write.c:589
>>   SyS_write+0x55/0x80 fs/read_write.c:581
>>   do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
>>   entry_SYSCALL_64_after_hwframe+0x3d/0xa2
>> ==================================================================
> I did a little digging before sending the bug upstream.
> If I add memset(obj, 0xfe, size) to __kmalloc_reserve(), these 0xfe
> bytes are visible in __get_item() at the place where KMSAN reports an
> error.
> 
> The problem is somehow related to tun_get_user() creating a fragmented
> sk_buff - when I change the call to tun_alloc_skb() so that it
> allocates a single buffer the bug goes away.
>

I guess the following patch would fix the issue

(I will submit it more formally)

diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index ce61231e96ea5fe27f512fbd0d80d4609997e508..333e967ed968ea3ff2dda25289f7f657263db2b9 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -423,6 +423,7 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
        struct pppoe_hdr *ph;
        struct pppox_sock *po;
        struct pppoe_net *pn;
+       __be16 sid;
        int len;
 
        skb = skb_share_check(skb, GFP_ATOMIC);
@@ -434,6 +435,7 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
 
        ph = pppoe_hdr(skb);
        len = ntohs(ph->length);
+       sid = ph->sid;
 
        skb_pull_rcsum(skb, sizeof(*ph));
        if (skb->len < len)
@@ -447,7 +449,7 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
        /* Note that get_item does a sock_hold(), so sk_pppox(po)
         * is known to be safe.
         */
-       po = get_item(pn, ph->sid, eth_hdr(skb)->h_source, dev->ifindex);
+       po = get_item(pn, sid, eth_hdr(skb)->h_source, dev->ifindex);
        if (!po)
                goto drop;




^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: KMSAN: uninit-value in pppoe_rcv
  2018-09-13 13:57   ` Eric Dumazet
@ 2018-09-13 14:12     ` Alexander Potapenko
  2018-09-13 16:19       ` Guillaume Nault
  2018-09-13 16:35       ` Eric Dumazet
  2018-09-18 16:52     ` Guillaume Nault
  1 sibling, 2 replies; 10+ messages in thread
From: Alexander Potapenko @ 2018-09-13 14:12 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: syzbot+f5f6080811c849739212, LKML, mostrows, Networking, syzkaller-bugs

On Thu, Sep 13, 2018 at 3:57 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>
>
>
> On 09/12/2018 03:38 AM, Alexander Potapenko wrote:
> > On Wed, Sep 12, 2018 at 12:24 PM syzbot
> > <syzbot+f5f6080811c849739212@syzkaller.appspotmail.com> wrote:
> >>
> >> Hello,
> >>
> >> syzbot found the following crash on:
> >>
> >> HEAD commit:    d2d741e5d189 kmsan: add initialization for shmem pages
> >> git tree:       https://github.com/google/kmsan.git/master
> >> console output: https://syzkaller.appspot.com/x/log.txt?x=1465fc37800000
> >> kernel config:  https://syzkaller.appspot.com/x/.config?x=48f9de3384bcd0f
> >> dashboard link: https://syzkaller.appspot.com/bug?extid=f5f6080811c849739212
> >> compiler:       clang version 7.0.0 (trunk 329391)
> >> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14d6e607800000
> >> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=10a15b5b800000
> >>
> >> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> >> Reported-by: syzbot+f5f6080811c849739212@syzkaller.appspotmail.com
> >>
> >> IPVS: ftp: loaded support on port[0] = 21
> >> ==================================================================
> >> BUG: KMSAN: uninit-value in __get_item drivers/net/ppp/pppoe.c:172 [inline]
> >> BUG: KMSAN: uninit-value in get_item drivers/net/ppp/pppoe.c:236 [inline]
> >> BUG: KMSAN: uninit-value in pppoe_rcv+0xcef/0x10e0
> >> drivers/net/ppp/pppoe.c:450
> >> CPU: 0 PID: 4543 Comm: syz-executor355 Not tainted 4.16.0+ #87
> >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> >> Google 01/01/2011
> >> Call Trace:
> >>   __dump_stack lib/dump_stack.c:17 [inline]
> >>   dump_stack+0x185/0x1d0 lib/dump_stack.c:53
> >>   kmsan_report+0x142/0x240 mm/kmsan/kmsan.c:1067
> >>   __msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:683
> >>   __get_item drivers/net/ppp/pppoe.c:172 [inline]
> >>   get_item drivers/net/ppp/pppoe.c:236 [inline]
> >>   pppoe_rcv+0xcef/0x10e0 drivers/net/ppp/pppoe.c:450
> >>   __netif_receive_skb_core+0x47df/0x4a90 net/core/dev.c:4562
> >>   __netif_receive_skb net/core/dev.c:4627 [inline]
> >>   netif_receive_skb_internal+0x49d/0x630 net/core/dev.c:4701
> >>   netif_receive_skb+0x230/0x240 net/core/dev.c:4725
> >>   tun_rx_batched drivers/net/tun.c:1555 [inline]
> >>   tun_get_user+0x740f/0x7c60 drivers/net/tun.c:1962
> >>   tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990
> >>   call_write_iter include/linux/fs.h:1782 [inline]
> >>   new_sync_write fs/read_write.c:469 [inline]
> >>   __vfs_write+0x7fb/0x9f0 fs/read_write.c:482
> >>   vfs_write+0x463/0x8d0 fs/read_write.c:544
> >>   SYSC_write+0x172/0x360 fs/read_write.c:589
> >>   SyS_write+0x55/0x80 fs/read_write.c:581
> >>   do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
> >>   entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> >> RIP: 0033:0x4447c9
> >> RSP: 002b:00007fff64c8fc28 EFLAGS: 00000297 ORIG_RAX: 0000000000000001
> >> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00000000004447c9
> >> RDX: 000000000000fd87 RSI: 0000000020000600 RDI: 0000000000000004
> >> RBP: 00000000006cf018 R08: 00007fff64c8fda8 R09: 00007fff00006bda
> >> R10: 0000000000005fe7 R11: 0000000000000297 R12: 00000000004020d0
> >> R13: 0000000000402160 R14: 0000000000000000 R15: 0000000000000000
> >>
> >> Uninit was created at:
> >>   kmsan_save_stack_with_flags mm/kmsan/kmsan.c:278 [inline]
> >>   kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:188
> >>   kmsan_kmalloc+0x94/0x100 mm/kmsan/kmsan.c:314
> >>   kmsan_slab_alloc+0x11/0x20 mm/kmsan/kmsan.c:321
> >>   slab_post_alloc_hook mm/slab.h:445 [inline]
> >>   slab_alloc_node mm/slub.c:2737 [inline]
> >>   __kmalloc_node_track_caller+0xaed/0x11c0 mm/slub.c:4369
> >>   __kmalloc_reserve net/core/skbuff.c:138 [inline]
> >>   __alloc_skb+0x2cf/0x9f0 net/core/skbuff.c:206
> >>   alloc_skb include/linux/skbuff.h:984 [inline]
> >>   alloc_skb_with_frags+0x1d4/0xb20 net/core/skbuff.c:5234
> >>   sock_alloc_send_pskb+0xb56/0x1190 net/core/sock.c:2085
> >>   tun_alloc_skb drivers/net/tun.c:1532 [inline]
> >>   tun_get_user+0x2242/0x7c60 drivers/net/tun.c:1829
> >>   tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990
> >>   call_write_iter include/linux/fs.h:1782 [inline]
> >>   new_sync_write fs/read_write.c:469 [inline]
> >>   __vfs_write+0x7fb/0x9f0 fs/read_write.c:482
> >>   vfs_write+0x463/0x8d0 fs/read_write.c:544
> >>   SYSC_write+0x172/0x360 fs/read_write.c:589
> >>   SyS_write+0x55/0x80 fs/read_write.c:581
> >>   do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
> >>   entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> >> ==================================================================
> > I did a little digging before sending the bug upstream.
> > If I add memset(obj, 0xfe, size) to __kmalloc_reserve(), these 0xfe
> > bytes are visible in __get_item() at the place where KMSAN reports an
> > error.
> >
> > The problem is somehow related to tun_get_user() creating a fragmented
> > sk_buff - when I change the call to tun_alloc_skb() so that it
> > allocates a single buffer the bug goes away.
> >
>
> I guess the following patch would fix the issue
>
> (I will submit it more formally)
No, as far as I can see it doesn't.
Saving sid before __skb_pull() is still a good idea, but in this
particular case |ph| doesn't change.
> diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
> index ce61231e96ea5fe27f512fbd0d80d4609997e508..333e967ed968ea3ff2dda25289f7f657263db2b9 100644
> --- a/drivers/net/ppp/pppoe.c
> +++ b/drivers/net/ppp/pppoe.c
> @@ -423,6 +423,7 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
>         struct pppoe_hdr *ph;
>         struct pppox_sock *po;
>         struct pppoe_net *pn;
> +       __be16 sid;
>         int len;
>
>         skb = skb_share_check(skb, GFP_ATOMIC);
> @@ -434,6 +435,7 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
>
>         ph = pppoe_hdr(skb);
>         len = ntohs(ph->length);
> +       sid = ph->sid;
>
>         skb_pull_rcsum(skb, sizeof(*ph));
>         if (skb->len < len)
> @@ -447,7 +449,7 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
>         /* Note that get_item does a sock_hold(), so sk_pppox(po)
>          * is known to be safe.
>          */
> -       po = get_item(pn, ph->sid, eth_hdr(skb)->h_source, dev->ifindex);
> +       po = get_item(pn, sid, eth_hdr(skb)->h_source, dev->ifindex);
>         if (!po)
>                 goto drop;
>
>
>
> --
> You received this message because you are subscribed to the Google Groups "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller-bugs+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/syzkaller-bugs/7424e094-afda-084a-ad80-299f219ced92%40gmail.com.
> For more options, visit https://groups.google.com/d/optout.



-- 
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] 10+ messages in thread
* Re: KMSAN: uninit-value in pppoe_rcv
  2018-09-13 14:12     ` Alexander Potapenko
@ 2018-09-13 16:19       ` Guillaume Nault
  2018-09-13 17:23         ` Guillaume Nault
  2018-09-13 16:35       ` Eric Dumazet
  1 sibling, 1 reply; 10+ messages in thread
From: Guillaume Nault @ 2018-09-13 16:19 UTC (permalink / raw)
  To: Alexander Potapenko
  Cc: Eric Dumazet, syzbot+f5f6080811c849739212, LKML, mostrows,
	Networking, syzkaller-bugs

On Thu, Sep 13, 2018 at 04:12:38PM +0200, Alexander Potapenko wrote:
> On Thu, Sep 13, 2018 at 3:57 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
> >
> >
> >
> > On 09/12/2018 03:38 AM, Alexander Potapenko wrote:
> > > On Wed, Sep 12, 2018 at 12:24 PM syzbot
> > > <syzbot+f5f6080811c849739212@syzkaller.appspotmail.com> wrote:
> > >>
> > >> Hello,
> > >>
> > >> syzbot found the following crash on:
> > >>
> > >> HEAD commit:    d2d741e5d189 kmsan: add initialization for shmem pages
> > >> git tree:       https://github.com/google/kmsan.git/master
> > >> console output: https://syzkaller.appspot.com/x/log.txt?x=1465fc37800000
> > >> kernel config:  https://syzkaller.appspot.com/x/.config?x=48f9de3384bcd0f
> > >> dashboard link: https://syzkaller.appspot.com/bug?extid=f5f6080811c849739212
> > >> compiler:       clang version 7.0.0 (trunk 329391)
> > >> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14d6e607800000
> > >> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=10a15b5b800000
> > >>
> > >> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > >> Reported-by: syzbot+f5f6080811c849739212@syzkaller.appspotmail.com
> > >>
> > >> IPVS: ftp: loaded support on port[0] = 21
> > >> ==================================================================
> > >> BUG: KMSAN: uninit-value in __get_item drivers/net/ppp/pppoe.c:172 [inline]
> > >> BUG: KMSAN: uninit-value in get_item drivers/net/ppp/pppoe.c:236 [inline]
> > >> BUG: KMSAN: uninit-value in pppoe_rcv+0xcef/0x10e0
> > >> drivers/net/ppp/pppoe.c:450
> > >> CPU: 0 PID: 4543 Comm: syz-executor355 Not tainted 4.16.0+ #87
> > >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > >> Google 01/01/2011
> > >> Call Trace:
> > >>   __dump_stack lib/dump_stack.c:17 [inline]
> > >>   dump_stack+0x185/0x1d0 lib/dump_stack.c:53
> > >>   kmsan_report+0x142/0x240 mm/kmsan/kmsan.c:1067
> > >>   __msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:683
> > >>   __get_item drivers/net/ppp/pppoe.c:172 [inline]
> > >>   get_item drivers/net/ppp/pppoe.c:236 [inline]
> > >>   pppoe_rcv+0xcef/0x10e0 drivers/net/ppp/pppoe.c:450
> > >>   __netif_receive_skb_core+0x47df/0x4a90 net/core/dev.c:4562
> > >>   __netif_receive_skb net/core/dev.c:4627 [inline]
> > >>   netif_receive_skb_internal+0x49d/0x630 net/core/dev.c:4701
> > >>   netif_receive_skb+0x230/0x240 net/core/dev.c:4725
> > >>   tun_rx_batched drivers/net/tun.c:1555 [inline]
> > >>   tun_get_user+0x740f/0x7c60 drivers/net/tun.c:1962
> > >>   tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990
> > >>   call_write_iter include/linux/fs.h:1782 [inline]
> > >>   new_sync_write fs/read_write.c:469 [inline]
> > >>   __vfs_write+0x7fb/0x9f0 fs/read_write.c:482
> > >>   vfs_write+0x463/0x8d0 fs/read_write.c:544
> > >>   SYSC_write+0x172/0x360 fs/read_write.c:589
> > >>   SyS_write+0x55/0x80 fs/read_write.c:581
> > >>   do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
> > >>   entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> > >> RIP: 0033:0x4447c9
> > >> RSP: 002b:00007fff64c8fc28 EFLAGS: 00000297 ORIG_RAX: 0000000000000001
> > >> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00000000004447c9
> > >> RDX: 000000000000fd87 RSI: 0000000020000600 RDI: 0000000000000004
> > >> RBP: 00000000006cf018 R08: 00007fff64c8fda8 R09: 00007fff00006bda
> > >> R10: 0000000000005fe7 R11: 0000000000000297 R12: 00000000004020d0
> > >> R13: 0000000000402160 R14: 0000000000000000 R15: 0000000000000000
> > >>
> > >> Uninit was created at:
> > >>   kmsan_save_stack_with_flags mm/kmsan/kmsan.c:278 [inline]
> > >>   kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:188
> > >>   kmsan_kmalloc+0x94/0x100 mm/kmsan/kmsan.c:314
> > >>   kmsan_slab_alloc+0x11/0x20 mm/kmsan/kmsan.c:321
> > >>   slab_post_alloc_hook mm/slab.h:445 [inline]
> > >>   slab_alloc_node mm/slub.c:2737 [inline]
> > >>   __kmalloc_node_track_caller+0xaed/0x11c0 mm/slub.c:4369
> > >>   __kmalloc_reserve net/core/skbuff.c:138 [inline]
> > >>   __alloc_skb+0x2cf/0x9f0 net/core/skbuff.c:206
> > >>   alloc_skb include/linux/skbuff.h:984 [inline]
> > >>   alloc_skb_with_frags+0x1d4/0xb20 net/core/skbuff.c:5234
> > >>   sock_alloc_send_pskb+0xb56/0x1190 net/core/sock.c:2085
> > >>   tun_alloc_skb drivers/net/tun.c:1532 [inline]
> > >>   tun_get_user+0x2242/0x7c60 drivers/net/tun.c:1829
> > >>   tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990
> > >>   call_write_iter include/linux/fs.h:1782 [inline]
> > >>   new_sync_write fs/read_write.c:469 [inline]
> > >>   __vfs_write+0x7fb/0x9f0 fs/read_write.c:482
> > >>   vfs_write+0x463/0x8d0 fs/read_write.c:544
> > >>   SYSC_write+0x172/0x360 fs/read_write.c:589
> > >>   SyS_write+0x55/0x80 fs/read_write.c:581
> > >>   do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
> > >>   entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> > >> ==================================================================
> > > I did a little digging before sending the bug upstream.
> > > If I add memset(obj, 0xfe, size) to __kmalloc_reserve(), these 0xfe
> > > bytes are visible in __get_item() at the place where KMSAN reports an
> > > error.
> > >
> > > The problem is somehow related to tun_get_user() creating a fragmented
> > > sk_buff - when I change the call to tun_alloc_skb() so that it
> > > allocates a single buffer the bug goes away.
> > >
> >
> > I guess the following patch would fix the issue
> >
> > (I will submit it more formally)
> No, as far as I can see it doesn't.
> Saving sid before __skb_pull() is still a good idea, but in this
> particular case |ph| doesn't change.

Yes, we probably need to save sid.

But I think the problem found by syzbot is related to
eth_hdr(skb)->h_source. PPPoE expects that Ethernet header has already
been parsed and is accessible at skb_mac_header(skb).
But here skb_mac_header(skb) == skb->data, and we may pull only 6 bytes
(sizeof(truct pppoe_hdr)). Therefore eth_hdr(skb)->h_source points past
skb's head length.

Not sure if something needs to be changed in tun.c for properly setting
skb_mac_header. But PPPoE has no reason to consider packets from
non-Ethernet devices anyway.

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: KMSAN: uninit-value in pppoe_rcv
  2018-09-13 14:12     ` Alexander Potapenko
  2018-09-13 16:19       ` Guillaume Nault
@ 2018-09-13 16:35       ` Eric Dumazet
  1 sibling, 0 replies; 10+ messages in thread
From: Eric Dumazet @ 2018-09-13 16:35 UTC (permalink / raw)
  To: Alexander Potapenko, Eric Dumazet
  Cc: syzbot+f5f6080811c849739212, LKML, mostrows, Networking, syzkaller-bugs



On 09/13/2018 07:12 AM, Alexander Potapenko wrote:
> On Thu, Sep 13, 2018 at 3:57 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
>>
>>
>>
>> On 09/12/2018 03:38 AM, Alexander Potapenko wrote:
>>> On Wed, Sep 12, 2018 at 12:24 PM syzbot
>>> <syzbot+f5f6080811c849739212@syzkaller.appspotmail.com> wrote:
>>>>
>>>> Hello,
>>>>
>>>> syzbot found the following crash on:
>>>>
>>>> HEAD commit:    d2d741e5d189 kmsan: add initialization for shmem pages
>>>> git tree:       https://github.com/google/kmsan.git/master
>>>> console output: https://syzkaller.appspot.com/x/log.txt?x=1465fc37800000
>>>> kernel config:  https://syzkaller.appspot.com/x/.config?x=48f9de3384bcd0f
>>>> dashboard link: https://syzkaller.appspot.com/bug?extid=f5f6080811c849739212
>>>> compiler:       clang version 7.0.0 (trunk 329391)
>>>> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14d6e607800000
>>>> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=10a15b5b800000
>>>>
>>>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>>>> Reported-by: syzbot+f5f6080811c849739212@syzkaller.appspotmail.com
>>>>
>>>> IPVS: ftp: loaded support on port[0] = 21
>>>> ==================================================================
>>>> BUG: KMSAN: uninit-value in __get_item drivers/net/ppp/pppoe.c:172 [inline]
>>>> BUG: KMSAN: uninit-value in get_item drivers/net/ppp/pppoe.c:236 [inline]
>>>> BUG: KMSAN: uninit-value in pppoe_rcv+0xcef/0x10e0
>>>> drivers/net/ppp/pppoe.c:450
>>>> CPU: 0 PID: 4543 Comm: syz-executor355 Not tainted 4.16.0+ #87
>>>> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
>>>> Google 01/01/2011
>>>> Call Trace:
>>>>   __dump_stack lib/dump_stack.c:17 [inline]
>>>>   dump_stack+0x185/0x1d0 lib/dump_stack.c:53
>>>>   kmsan_report+0x142/0x240 mm/kmsan/kmsan.c:1067
>>>>   __msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:683
>>>>   __get_item drivers/net/ppp/pppoe.c:172 [inline]
>>>>   get_item drivers/net/ppp/pppoe.c:236 [inline]
>>>>   pppoe_rcv+0xcef/0x10e0 drivers/net/ppp/pppoe.c:450
>>>>   __netif_receive_skb_core+0x47df/0x4a90 net/core/dev.c:4562
>>>>   __netif_receive_skb net/core/dev.c:4627 [inline]
>>>>   netif_receive_skb_internal+0x49d/0x630 net/core/dev.c:4701
>>>>   netif_receive_skb+0x230/0x240 net/core/dev.c:4725
>>>>   tun_rx_batched drivers/net/tun.c:1555 [inline]
>>>>   tun_get_user+0x740f/0x7c60 drivers/net/tun.c:1962
>>>>   tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990
>>>>   call_write_iter include/linux/fs.h:1782 [inline]
>>>>   new_sync_write fs/read_write.c:469 [inline]
>>>>   __vfs_write+0x7fb/0x9f0 fs/read_write.c:482
>>>>   vfs_write+0x463/0x8d0 fs/read_write.c:544
>>>>   SYSC_write+0x172/0x360 fs/read_write.c:589
>>>>   SyS_write+0x55/0x80 fs/read_write.c:581
>>>>   do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
>>>>   entry_SYSCALL_64_after_hwframe+0x3d/0xa2
>>>> RIP: 0033:0x4447c9
>>>> RSP: 002b:00007fff64c8fc28 EFLAGS: 00000297 ORIG_RAX: 0000000000000001
>>>> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00000000004447c9
>>>> RDX: 000000000000fd87 RSI: 0000000020000600 RDI: 0000000000000004
>>>> RBP: 00000000006cf018 R08: 00007fff64c8fda8 R09: 00007fff00006bda
>>>> R10: 0000000000005fe7 R11: 0000000000000297 R12: 00000000004020d0
>>>> R13: 0000000000402160 R14: 0000000000000000 R15: 0000000000000000
>>>>
>>>> Uninit was created at:
>>>>   kmsan_save_stack_with_flags mm/kmsan/kmsan.c:278 [inline]
>>>>   kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:188
>>>>   kmsan_kmalloc+0x94/0x100 mm/kmsan/kmsan.c:314
>>>>   kmsan_slab_alloc+0x11/0x20 mm/kmsan/kmsan.c:321
>>>>   slab_post_alloc_hook mm/slab.h:445 [inline]
>>>>   slab_alloc_node mm/slub.c:2737 [inline]
>>>>   __kmalloc_node_track_caller+0xaed/0x11c0 mm/slub.c:4369
>>>>   __kmalloc_reserve net/core/skbuff.c:138 [inline]
>>>>   __alloc_skb+0x2cf/0x9f0 net/core/skbuff.c:206
>>>>   alloc_skb include/linux/skbuff.h:984 [inline]
>>>>   alloc_skb_with_frags+0x1d4/0xb20 net/core/skbuff.c:5234
>>>>   sock_alloc_send_pskb+0xb56/0x1190 net/core/sock.c:2085
>>>>   tun_alloc_skb drivers/net/tun.c:1532 [inline]
>>>>   tun_get_user+0x2242/0x7c60 drivers/net/tun.c:1829
>>>>   tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990
>>>>   call_write_iter include/linux/fs.h:1782 [inline]
>>>>   new_sync_write fs/read_write.c:469 [inline]
>>>>   __vfs_write+0x7fb/0x9f0 fs/read_write.c:482
>>>>   vfs_write+0x463/0x8d0 fs/read_write.c:544
>>>>   SYSC_write+0x172/0x360 fs/read_write.c:589
>>>>   SyS_write+0x55/0x80 fs/read_write.c:581
>>>>   do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
>>>>   entry_SYSCALL_64_after_hwframe+0x3d/0xa2
>>>> ==================================================================
>>> I did a little digging before sending the bug upstream.
>>> If I add memset(obj, 0xfe, size) to __kmalloc_reserve(), these 0xfe
>>> bytes are visible in __get_item() at the place where KMSAN reports an
>>> error.
>>>
>>> The problem is somehow related to tun_get_user() creating a fragmented
>>> sk_buff - when I change the call to tun_alloc_skb() so that it
>>> allocates a single buffer the bug goes away.
>>>
>>
>> I guess the following patch would fix the issue
>>
>> (I will submit it more formally)
> No, as far as I can see it doesn't.
> Saving sid before __skb_pull() is still a good idea, but in this
> particular case |ph| doesn't change.
>> diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
>> index ce61231e96ea5fe27f512fbd0d80d4609997e508..333e967ed968ea3ff2dda25289f7f657263db2b9 100644
>> --- a/drivers/net/ppp/pppoe.c
>> +++ b/drivers/net/ppp/pppoe.c
>> @@ -423,6 +423,7 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
>>         struct pppoe_hdr *ph;
>>         struct pppox_sock *po;
>>         struct pppoe_net *pn;
>> +       __be16 sid;
>>         int len;
>>
>>         skb = skb_share_check(skb, GFP_ATOMIC);
>> @@ -434,6 +435,7 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
>>
>>         ph = pppoe_hdr(skb);
>>         len = ntohs(ph->length);

Then ph->length needs to be better validated.

>> +       sid = ph->sid;

I'll take a look, thanks.


^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: KMSAN: uninit-value in pppoe_rcv
  2018-09-13 16:19       ` Guillaume Nault
@ 2018-09-13 17:23         ` Guillaume Nault
  2018-09-13 17:31           ` Eric Dumazet
  0 siblings, 1 reply; 10+ messages in thread
From: Guillaume Nault @ 2018-09-13 17:23 UTC (permalink / raw)
  To: Alexander Potapenko
  Cc: Eric Dumazet, syzbot+f5f6080811c849739212, LKML, mostrows,
	Networking, syzkaller-bugs

On Thu, Sep 13, 2018 at 06:19:29PM +0200, Guillaume Nault wrote:
> On Thu, Sep 13, 2018 at 04:12:38PM +0200, Alexander Potapenko wrote:
> > On Thu, Sep 13, 2018 at 3:57 PM Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > >
> > >
> > >
> > > On 09/12/2018 03:38 AM, Alexander Potapenko wrote:
> > > > On Wed, Sep 12, 2018 at 12:24 PM syzbot
> > > > <syzbot+f5f6080811c849739212@syzkaller.appspotmail.com> wrote:
> > > >>
> > > >> Hello,
> > > >>
> > > >> syzbot found the following crash on:
> > > >>
> > > >> HEAD commit:    d2d741e5d189 kmsan: add initialization for shmem pages
> > > >> git tree:       https://github.com/google/kmsan.git/master
> > > >> console output: https://syzkaller.appspot.com/x/log.txt?x=1465fc37800000
> > > >> kernel config:  https://syzkaller.appspot.com/x/.config?x=48f9de3384bcd0f
> > > >> dashboard link: https://syzkaller.appspot.com/bug?extid=f5f6080811c849739212
> > > >> compiler:       clang version 7.0.0 (trunk 329391)
> > > >> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14d6e607800000
> > > >> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=10a15b5b800000
> > > >>
> > > >> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > >> Reported-by: syzbot+f5f6080811c849739212@syzkaller.appspotmail.com
> > > >>
> > > >> IPVS: ftp: loaded support on port[0] = 21
> > > >> ==================================================================
> > > >> BUG: KMSAN: uninit-value in __get_item drivers/net/ppp/pppoe.c:172 [inline]
> > > >> BUG: KMSAN: uninit-value in get_item drivers/net/ppp/pppoe.c:236 [inline]
> > > >> BUG: KMSAN: uninit-value in pppoe_rcv+0xcef/0x10e0
> > > >> drivers/net/ppp/pppoe.c:450
> > > >> CPU: 0 PID: 4543 Comm: syz-executor355 Not tainted 4.16.0+ #87
> > > >> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > > >> Google 01/01/2011
> > > >> Call Trace:
> > > >>   __dump_stack lib/dump_stack.c:17 [inline]
> > > >>   dump_stack+0x185/0x1d0 lib/dump_stack.c:53
> > > >>   kmsan_report+0x142/0x240 mm/kmsan/kmsan.c:1067
> > > >>   __msan_warning_32+0x6c/0xb0 mm/kmsan/kmsan_instr.c:683
> > > >>   __get_item drivers/net/ppp/pppoe.c:172 [inline]
> > > >>   get_item drivers/net/ppp/pppoe.c:236 [inline]
> > > >>   pppoe_rcv+0xcef/0x10e0 drivers/net/ppp/pppoe.c:450
> > > >>   __netif_receive_skb_core+0x47df/0x4a90 net/core/dev.c:4562
> > > >>   __netif_receive_skb net/core/dev.c:4627 [inline]
> > > >>   netif_receive_skb_internal+0x49d/0x630 net/core/dev.c:4701
> > > >>   netif_receive_skb+0x230/0x240 net/core/dev.c:4725
> > > >>   tun_rx_batched drivers/net/tun.c:1555 [inline]
> > > >>   tun_get_user+0x740f/0x7c60 drivers/net/tun.c:1962
> > > >>   tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990
> > > >>   call_write_iter include/linux/fs.h:1782 [inline]
> > > >>   new_sync_write fs/read_write.c:469 [inline]
> > > >>   __vfs_write+0x7fb/0x9f0 fs/read_write.c:482
> > > >>   vfs_write+0x463/0x8d0 fs/read_write.c:544
> > > >>   SYSC_write+0x172/0x360 fs/read_write.c:589
> > > >>   SyS_write+0x55/0x80 fs/read_write.c:581
> > > >>   do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
> > > >>   entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> > > >> RIP: 0033:0x4447c9
> > > >> RSP: 002b:00007fff64c8fc28 EFLAGS: 00000297 ORIG_RAX: 0000000000000001
> > > >> RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00000000004447c9
> > > >> RDX: 000000000000fd87 RSI: 0000000020000600 RDI: 0000000000000004
> > > >> RBP: 00000000006cf018 R08: 00007fff64c8fda8 R09: 00007fff00006bda
> > > >> R10: 0000000000005fe7 R11: 0000000000000297 R12: 00000000004020d0
> > > >> R13: 0000000000402160 R14: 0000000000000000 R15: 0000000000000000
> > > >>
> > > >> Uninit was created at:
> > > >>   kmsan_save_stack_with_flags mm/kmsan/kmsan.c:278 [inline]
> > > >>   kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:188
> > > >>   kmsan_kmalloc+0x94/0x100 mm/kmsan/kmsan.c:314
> > > >>   kmsan_slab_alloc+0x11/0x20 mm/kmsan/kmsan.c:321
> > > >>   slab_post_alloc_hook mm/slab.h:445 [inline]
> > > >>   slab_alloc_node mm/slub.c:2737 [inline]
> > > >>   __kmalloc_node_track_caller+0xaed/0x11c0 mm/slub.c:4369
> > > >>   __kmalloc_reserve net/core/skbuff.c:138 [inline]
> > > >>   __alloc_skb+0x2cf/0x9f0 net/core/skbuff.c:206
> > > >>   alloc_skb include/linux/skbuff.h:984 [inline]
> > > >>   alloc_skb_with_frags+0x1d4/0xb20 net/core/skbuff.c:5234
> > > >>   sock_alloc_send_pskb+0xb56/0x1190 net/core/sock.c:2085
> > > >>   tun_alloc_skb drivers/net/tun.c:1532 [inline]
> > > >>   tun_get_user+0x2242/0x7c60 drivers/net/tun.c:1829
> > > >>   tun_chr_write_iter+0x1d4/0x330 drivers/net/tun.c:1990
> > > >>   call_write_iter include/linux/fs.h:1782 [inline]
> > > >>   new_sync_write fs/read_write.c:469 [inline]
> > > >>   __vfs_write+0x7fb/0x9f0 fs/read_write.c:482
> > > >>   vfs_write+0x463/0x8d0 fs/read_write.c:544
> > > >>   SYSC_write+0x172/0x360 fs/read_write.c:589
> > > >>   SyS_write+0x55/0x80 fs/read_write.c:581
> > > >>   do_syscall_64+0x309/0x430 arch/x86/entry/common.c:287
> > > >>   entry_SYSCALL_64_after_hwframe+0x3d/0xa2
> > > >> ==================================================================
> > > > I did a little digging before sending the bug upstream.
> > > > If I add memset(obj, 0xfe, size) to __kmalloc_reserve(), these 0xfe
> > > > bytes are visible in __get_item() at the place where KMSAN reports an
> > > > error.
> > > >
> > > > The problem is somehow related to tun_get_user() creating a fragmented
> > > > sk_buff - when I change the call to tun_alloc_skb() so that it
> > > > allocates a single buffer the bug goes away.
> > > >
> > >
> > > I guess the following patch would fix the issue
> > >
> > > (I will submit it more formally)
> > No, as far as I can see it doesn't.
> > Saving sid before __skb_pull() is still a good idea, but in this
> > particular case |ph| doesn't change.
> 
> Yes, we probably need to save sid.
> 
> But I think the problem found by syzbot is related to
> eth_hdr(skb)->h_source. PPPoE expects that Ethernet header has already
> been parsed and is accessible at skb_mac_header(skb).
> But here skb_mac_header(skb) == skb->data, and we may pull only 6 bytes
> (sizeof(truct pppoe_hdr)). Therefore eth_hdr(skb)->h_source points past
> skb's head length.
> 
> Not sure if something needs to be changed in tun.c for properly setting
> skb_mac_header. But PPPoE has no reason to consider packets from
> non-Ethernet devices anyway.

Nothing to change in tun.c. Just some more tests in pppoe.
Can you try this patch? It only addresses this particular report, not
the problems spotted by Eric.

-------- 8< --------
diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
index 5aa59f41bf8c..77241b584dff 100644
--- a/drivers/net/ppp/pppoe.c
+++ b/drivers/net/ppp/pppoe.c
@@ -429,6 +429,9 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
 	if (!skb)
 		goto out;
 
+	if (skb_mac_header_len(skb) < ETH_HLEN)
+		goto drop;
+
 	if (!pskb_may_pull(skb, sizeof(struct pppoe_hdr)))
 		goto drop;
 

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: KMSAN: uninit-value in pppoe_rcv
  2018-09-13 17:23         ` Guillaume Nault
@ 2018-09-13 17:31           ` Eric Dumazet
  0 siblings, 0 replies; 10+ messages in thread
From: Eric Dumazet @ 2018-09-13 17:31 UTC (permalink / raw)
  To: Guillaume Nault, Alexander Potapenko
  Cc: Eric Dumazet, syzbot+f5f6080811c849739212, LKML, mostrows,
	Networking, syzkaller-bugs



On 09/13/2018 10:23 AM, Guillaume Nault wrote:

> Nothing to change in tun.c. Just some more tests in pppoe.
> Can you try this patch? It only addresses this particular report, not
> the problems spotted by Eric.
> 
> -------- 8< --------
> diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
> index 5aa59f41bf8c..77241b584dff 100644
> --- a/drivers/net/ppp/pppoe.c
> +++ b/drivers/net/ppp/pppoe.c
> @@ -429,6 +429,9 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
>  	if (!skb)
>  		goto out;
>  
> +	if (skb_mac_header_len(skb) < ETH_HLEN)
> +		goto drop;
> +
>  	if (!pskb_may_pull(skb, sizeof(struct pppoe_hdr)))
>  		goto drop;
>  
> 


Yeah this probably will help ;)

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: KMSAN: uninit-value in pppoe_rcv
  2018-09-13 13:57   ` Eric Dumazet
  2018-09-13 14:12     ` Alexander Potapenko
@ 2018-09-18 16:52     ` Guillaume Nault
  2018-09-18 17:03       ` Eric Dumazet
  1 sibling, 1 reply; 10+ messages in thread
From: Guillaume Nault @ 2018-09-18 16:52 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Alexander Potapenko, syzbot+f5f6080811c849739212, LKML, mostrows,
	Networking, syzkaller-bugs

On Thu, Sep 13, 2018 at 06:57:54AM -0700, Eric Dumazet wrote:
> 
> 
> I guess the following patch would fix the issue
> 
> (I will submit it more formally)
>
Hi Eric,

Do you still plan to submit this patch? Otherwise I can take care of it.


> diff --git a/drivers/net/ppp/pppoe.c b/drivers/net/ppp/pppoe.c
> index ce61231e96ea5fe27f512fbd0d80d4609997e508..333e967ed968ea3ff2dda25289f7f657263db2b9 100644
> --- a/drivers/net/ppp/pppoe.c
> +++ b/drivers/net/ppp/pppoe.c
> @@ -423,6 +423,7 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
>         struct pppoe_hdr *ph;
>         struct pppox_sock *po;
>         struct pppoe_net *pn;
> +       __be16 sid;
>         int len;
>  
>         skb = skb_share_check(skb, GFP_ATOMIC);
> @@ -434,6 +435,7 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
>  
>         ph = pppoe_hdr(skb);
>         len = ntohs(ph->length);
> +       sid = ph->sid;
>  
>         skb_pull_rcsum(skb, sizeof(*ph));
>         if (skb->len < len)
> @@ -447,7 +449,7 @@ static int pppoe_rcv(struct sk_buff *skb, struct net_device *dev,
>         /* Note that get_item does a sock_hold(), so sk_pppox(po)
>          * is known to be safe.
>          */
> -       po = get_item(pn, ph->sid, eth_hdr(skb)->h_source, dev->ifindex);
> +       po = get_item(pn, sid, eth_hdr(skb)->h_source, dev->ifindex);
>         if (!po)
>                 goto drop;
> 
> 
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: KMSAN: uninit-value in pppoe_rcv
  2018-09-18 16:52     ` Guillaume Nault
@ 2018-09-18 17:03       ` Eric Dumazet
  0 siblings, 0 replies; 10+ messages in thread
From: Eric Dumazet @ 2018-09-18 17:03 UTC (permalink / raw)
  To: Guillaume Nault, Eric Dumazet
  Cc: Alexander Potapenko, syzbot+f5f6080811c849739212, LKML, mostrows,
	Networking, syzkaller-bugs



On 09/18/2018 09:52 AM, Guillaume Nault wrote:
> On Thu, Sep 13, 2018 at 06:57:54AM -0700, Eric Dumazet wrote:
>>
>>
>> I guess the following patch would fix the issue
>>
>> (I will submit it more formally)
>>
> Hi Eric,
> 
> Do you still plan to submit this patch? Otherwise I can take care of it.
> 

Yes I will submit it. Thanks.


^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-09-18 17:03 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-12 10:24 KMSAN: uninit-value in pppoe_rcv syzbot
2018-09-12 10:38 ` Alexander Potapenko
2018-09-13 13:57   ` Eric Dumazet
2018-09-13 14:12     ` Alexander Potapenko
2018-09-13 16:19       ` Guillaume Nault
2018-09-13 17:23         ` Guillaume Nault
2018-09-13 17:31           ` Eric Dumazet
2018-09-13 16:35       ` Eric Dumazet
2018-09-18 16:52     ` Guillaume Nault
2018-09-18 17:03       ` Eric Dumazet

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).