2018-09-12 10:24:52

by syzbot

[permalink] [raw]
Subject: KMSAN: uninit-value in pppoe_rcv

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: [email protected]

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 [email protected].

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


2018-09-12 10:39:14

by Alexander Potapenko

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in pppoe_rcv

On Wed, Sep 12, 2018 at 12:24 PM syzbot
<[email protected]> 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: [email protected]
>
> 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 [email protected].
>
> 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 [email protected].
> 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

2018-09-13 13:59:42

by Eric Dumazet

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in pppoe_rcv



On 09/12/2018 03:38 AM, Alexander Potapenko wrote:
> On Wed, Sep 12, 2018 at 12:24 PM syzbot
> <[email protected]> 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: [email protected]
>>
>> 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;




2018-09-13 14:13:24

by Alexander Potapenko

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in pppoe_rcv

On Thu, Sep 13, 2018 at 3:57 PM Eric Dumazet <[email protected]> wrote:
>
>
>
> On 09/12/2018 03:38 AM, Alexander Potapenko wrote:
> > On Wed, Sep 12, 2018 at 12:24 PM syzbot
> > <[email protected]> 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: [email protected]
> >>
> >> 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 [email protected].
> 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

2018-09-13 16:21:43

by Guillaume Nault

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in pppoe_rcv

On Thu, Sep 13, 2018 at 04:12:38PM +0200, Alexander Potapenko wrote:
> On Thu, Sep 13, 2018 at 3:57 PM Eric Dumazet <[email protected]> wrote:
> >
> >
> >
> > On 09/12/2018 03:38 AM, Alexander Potapenko wrote:
> > > On Wed, Sep 12, 2018 at 12:24 PM syzbot
> > > <[email protected]> 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: [email protected]
> > >>
> > >> 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.

2018-09-13 16:36:20

by Eric Dumazet

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in pppoe_rcv



On 09/13/2018 07:12 AM, Alexander Potapenko wrote:
> On Thu, Sep 13, 2018 at 3:57 PM Eric Dumazet <[email protected]> wrote:
>>
>>
>>
>> On 09/12/2018 03:38 AM, Alexander Potapenko wrote:
>>> On Wed, Sep 12, 2018 at 12:24 PM syzbot
>>> <[email protected]> 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: [email protected]
>>>>
>>>> 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.


2018-09-13 17:26:02

by Guillaume Nault

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in pppoe_rcv

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 <[email protected]> wrote:
> > >
> > >
> > >
> > > On 09/12/2018 03:38 AM, Alexander Potapenko wrote:
> > > > On Wed, Sep 12, 2018 at 12:24 PM syzbot
> > > > <[email protected]> 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: [email protected]
> > > >>
> > > >> 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;


2018-09-13 17:46:53

by Eric Dumazet

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in pppoe_rcv



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

2018-09-18 16:54:00

by Guillaume Nault

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in pppoe_rcv

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

2018-09-18 17:04:34

by Eric Dumazet

[permalink] [raw]
Subject: Re: KMSAN: uninit-value in pppoe_rcv



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.