linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* kernel BUG at mm/memory.c:LINE!
@ 2018-07-09  5:51 syzbot
  2018-07-09 10:15 ` Kirill A. Shutemov
  0 siblings, 1 reply; 9+ messages in thread
From: syzbot @ 2018-07-09  5:51 UTC (permalink / raw)
  To: akpm, dan.j.williams, jglisse, kirill.shutemov, ldufour,
	linux-kernel, linux-mm, mhocko, minchan, ross.zwisler,
	syzkaller-bugs, willy, ying.huang

Hello,

syzbot found the following crash on:

HEAD commit:    b2d44d145d2a Merge tag '4.18-rc3-smb3fixes' of git://git.s..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=11d07748400000
kernel config:  https://syzkaller.appspot.com/x/.config?x=2ca6c7a31d407f86
dashboard link: https://syzkaller.appspot.com/bug?extid=3f84280d52be9b7083cc
compiler:       gcc (GCC) 8.0.1 20180413 (experimental)

Unfortunately, I don't have any reproducer for this crash yet.

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

next ffff8801ce5e7040 prev ffff8801d20eca50 mm ffff88019c1e13c0
prot 27 anon_vma ffff88019680cdd8 vm_ops 0000000000000000
pgoff 0 file ffff8801b2ec2d00 private_data 0000000000000000
flags: 0xff(read|write|exec|shared|mayread|maywrite|mayexec|mayshare)
------------[ cut here ]------------
kernel BUG at mm/memory.c:1422!
invalid opcode: 0000 [#1] SMP KASAN
CPU: 0 PID: 18486 Comm: syz-executor3 Not tainted 4.18.0-rc3+ #136
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS  
Google 01/01/2011
RIP: 0010:zap_pmd_range mm/memory.c:1421 [inline]
RIP: 0010:zap_pud_range mm/memory.c:1466 [inline]
RIP: 0010:zap_p4d_range mm/memory.c:1487 [inline]
RIP: 0010:unmap_page_range+0x1c18/0x2220 mm/memory.c:1508
Code: ff 31 ff 4c 89 e6 42 c6 04 33 f8 e8 92 dd d0 ff 4d 85 e4 0f 85 4a eb  
ff ff e8 54 dc d0 ff 48 8b bd 10 fc ff ff e8 82 95 fe ff <0f> 0b e8 41 dc  
d0 ff 0f 0b 4c 89 ad 18 fc ff ff c7 85 7c fb ff ff
RSP: 0018:ffff8801b0587330 EFLAGS: 00010286
RAX: 000000000000013c RBX: 1ffff100360b0e9c RCX: ffffc90002620000
RDX: 0000000000000000 RSI: ffffffff81631851 RDI: 0000000000000001
RBP: ffff8801b05877c8 R08: ffff880199d40300 R09: ffffed003b5c4fc0
R10: ffffed003b5c4fc0 R11: ffff8801dae27e07 R12: 0000000000000000
R13: ffff88019c1e13c0 R14: dffffc0000000000 R15: 0000000020e01000
FS:  00007fca32251700(0000) GS:ffff8801dae00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f04c540d000 CR3: 00000001ac1f0000 CR4: 00000000001426f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
Call Trace:
  unmap_single_vma+0x1a0/0x310 mm/memory.c:1553
  zap_page_range_single+0x3cc/0x580 mm/memory.c:1644
  unmap_mapping_range_vma mm/memory.c:2792 [inline]
  unmap_mapping_range_tree mm/memory.c:2813 [inline]
  unmap_mapping_pages+0x3a7/0x5b0 mm/memory.c:2845
  unmap_mapping_range+0x48/0x60 mm/memory.c:2880
  truncate_pagecache+0x54/0x90 mm/truncate.c:800
  truncate_setsize+0x70/0xb0 mm/truncate.c:826
  simple_setattr+0xe9/0x110 fs/libfs.c:409
  notify_change+0xf13/0x10f0 fs/attr.c:335
  do_truncate+0x1ac/0x2b0 fs/open.c:63
  do_sys_ftruncate+0x492/0x560 fs/open.c:205
  __do_sys_ftruncate fs/open.c:215 [inline]
  __se_sys_ftruncate fs/open.c:213 [inline]
  __x64_sys_ftruncate+0x59/0x80 fs/open.c:213
  do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
  entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x455ba9
Code: 1d ba fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7  
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff  
ff 0f 83 eb b9 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fca32250c68 EFLAGS: 00000246 ORIG_RAX: 000000000000004d
RAX: ffffffffffffffda RBX: 00007fca322516d4 RCX: 0000000000455ba9
RDX: 0000000000000000 RSI: 0000000000000006 RDI: 0000000000000010
RBP: 000000000072bea0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 00000000004bbd22 R14: 00000000004c90e8 R15: 0000000000000000
Modules linked in:
Dumping ftrace buffer:
    (ftrace buffer empty)
---[ end trace a92a1c818e9b1620 ]---
RIP: 0010:zap_pmd_range mm/memory.c:1421 [inline]
RIP: 0010:zap_pud_range mm/memory.c:1466 [inline]
RIP: 0010:zap_p4d_range mm/memory.c:1487 [inline]
RIP: 0010:unmap_page_range+0x1c18/0x2220 mm/memory.c:1508
Code: ff 31 ff 4c 89 e6 42 c6 04 33 f8 e8 92 dd d0 ff 4d 85 e4 0f 85 4a eb  
ff ff e8 54 dc d0 ff 48 8b bd 10 fc ff ff e8 82 95 fe ff <0f> 0b e8 41 dc  
d0 ff 0f 0b 4c 89 ad 18 fc ff ff c7 85 7c fb ff ff
RSP: 0018:ffff8801b0587330 EFLAGS: 00010286
RAX: 000000000000013c RBX: 1ffff100360b0e9c RCX: ffffc90002620000
RDX: 0000000000000000 RSI: ffffffff81631851 RDI: 0000000000000001
RBP: ffff8801b05877c8 R08: ffff880199d40300 R09: ffffed003b5c4fc0
R10: ffffed003b5c4fc0 R11: ffff8801dae27e07 R12: 0000000000000000
R13: ffff88019c1e13c0 R14: dffffc0000000000 R15: 0000000020e01000
FS:  00007fca32251700(0000) GS:ffff8801dae00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f04c540d000 CR3: 00000001ac1f0000 CR4: 00000000001426f0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400


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

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

* Re: kernel BUG at mm/memory.c:LINE!
  2018-07-09  5:51 kernel BUG at mm/memory.c:LINE! syzbot
@ 2018-07-09 10:15 ` Kirill A. Shutemov
  2018-07-09 10:48   ` Dmitry Vyukov
  0 siblings, 1 reply; 9+ messages in thread
From: Kirill A. Shutemov @ 2018-07-09 10:15 UTC (permalink / raw)
  To: syzbot
  Cc: akpm, dan.j.williams, jglisse, kirill.shutemov, ldufour,
	linux-kernel, linux-mm, mhocko, minchan, ross.zwisler,
	syzkaller-bugs, willy, ying.huang

On Sun, Jul 08, 2018 at 10:51:03PM -0700, syzbot wrote:
> Hello,
> 
> syzbot found the following crash on:
> 
> HEAD commit:    b2d44d145d2a Merge tag '4.18-rc3-smb3fixes' of git://git.s..
> git tree:       upstream
> console output: https://syzkaller.appspot.com/x/log.txt?x=11d07748400000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=2ca6c7a31d407f86
> dashboard link: https://syzkaller.appspot.com/bug?extid=3f84280d52be9b7083cc
> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
> 
> Unfortunately, I don't have any reproducer for this crash yet.
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+3f84280d52be9b7083cc@syzkaller.appspotmail.com
> 
> next ffff8801ce5e7040 prev ffff8801d20eca50 mm ffff88019c1e13c0
> prot 27 anon_vma ffff88019680cdd8 vm_ops 0000000000000000
> pgoff 0 file ffff8801b2ec2d00 private_data 0000000000000000
> flags: 0xff(read|write|exec|shared|mayread|maywrite|mayexec|mayshare)
> ------------[ cut here ]------------
> kernel BUG at mm/memory.c:1422!

Looks like vma_is_anonymous() false-positive.

Any clues what file is it? I would guess some kind of socket, but it's not
clear from log which exactly.

-- 
 Kirill A. Shutemov

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

* Re: kernel BUG at mm/memory.c:LINE!
  2018-07-09 10:15 ` Kirill A. Shutemov
@ 2018-07-09 10:48   ` Dmitry Vyukov
  2018-07-09 10:52     ` Dmitry Vyukov
  0 siblings, 1 reply; 9+ messages in thread
From: Dmitry Vyukov @ 2018-07-09 10:48 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: syzbot, Andrew Morton, Dan Williams, Jerome Glisse,
	Kirill A. Shutemov, ldufour, LKML, Linux-MM, Michal Hocko,
	Minchan Kim, Ross Zwisler, syzkaller-bugs, Matthew Wilcox,
	ying.huang

On Mon, Jul 9, 2018 at 12:15 PM, Kirill A. Shutemov
<kirill@shutemov.name> wrote:
> On Sun, Jul 08, 2018 at 10:51:03PM -0700, syzbot wrote:
>> Hello,
>>
>> syzbot found the following crash on:
>>
>> HEAD commit:    b2d44d145d2a Merge tag '4.18-rc3-smb3fixes' of git://git.s..
>> git tree:       upstream
>> console output: https://syzkaller.appspot.com/x/log.txt?x=11d07748400000
>> kernel config:  https://syzkaller.appspot.com/x/.config?x=2ca6c7a31d407f86
>> dashboard link: https://syzkaller.appspot.com/bug?extid=3f84280d52be9b7083cc
>> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
>>
>> Unfortunately, I don't have any reproducer for this crash yet.
>>
>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> Reported-by: syzbot+3f84280d52be9b7083cc@syzkaller.appspotmail.com
>>
>> next ffff8801ce5e7040 prev ffff8801d20eca50 mm ffff88019c1e13c0
>> prot 27 anon_vma ffff88019680cdd8 vm_ops 0000000000000000
>> pgoff 0 file ffff8801b2ec2d00 private_data 0000000000000000
>> flags: 0xff(read|write|exec|shared|mayread|maywrite|mayexec|mayshare)
>> ------------[ cut here ]------------
>> kernel BUG at mm/memory.c:1422!
>
> Looks like vma_is_anonymous() false-positive.
>
> Any clues what file is it? I would guess some kind of socket, but it's not
> clear from log which exactly.


From the log it looks like it was this program (number 3 matches Comm:
syz-executor3):

08:39:32 executing program 3:
r0 = socket$nl_route(0x10, 0x3, 0x0)
bind$netlink(r0, &(0x7f00000002c0)={0x10, 0x0, 0x0, 0x100000}, 0xc)
getsockname(r0, &(0x7f0000000000)=@pppol2tpv3in6={0x0, 0x0, {0x0,
<r1=>0xffffffffffffffff, 0x0, 0x0, 0x0, 0x0, {0x0, 0x0, 0x0,
@loopback}}}, &(0x7f00000000c0)=0x3a)
mmap(&(0x7f0000e00000/0x200000)=nil, 0x200000, 0x7fdff, 0x11, r1, 0x0)
ioctl$FIBMAP(r0, 0x1, &(0x7f0000000100)=0x9)
r2 = socket$inet6(0xa, 0x1000000000002, 0x0)
ioctl(r2, 0x8912, &(0x7f00000001c0)="796d05ad441e829115ac7fd77200")
r3 = syz_open_dev$vcsa(&(0x7f0000000140)='/dev/vcsa#\x00', 0x3, 0x2)
ioctl$VHOST_SET_VRING_ENDIAN(r3, 0x4008af13, &(0x7f0000000180)={0x0, 0x8})
sendto$inet(0xffffffffffffffff, &(0x7f0000a88f88), 0xffffffffffffff31,
0x0, &(0x7f0000e68000)={0x2, 0x0, @multicast2=0xe0000002},
0xfffffffffffffeb3)
ftruncate(r1, 0x6)
mmap(&(0x7f0000e00000/0x200000)=nil, 0x200000, 0x0, 0x11, r0, 0x0)
setsockopt$SO_TIMESTAMPING(r1, 0x1, 0x25, &(0x7f0000000080)=0x804, 0x4)

But take what happens here with a grain of salt, it can pretend that
it's doing one thing, but actually do something different.
So that r1 passed to ftruncate is something that getsockname returned
somewhere in the middle of address. And since the socket is not
actually ppp, it can be just some bytes in the middle of netlink
address, that than happened to be small and match some existing fd...

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

* Re: kernel BUG at mm/memory.c:LINE!
  2018-07-09 10:48   ` Dmitry Vyukov
@ 2018-07-09 10:52     ` Dmitry Vyukov
  2018-07-09 14:21       ` Kirill A. Shutemov
  0 siblings, 1 reply; 9+ messages in thread
From: Dmitry Vyukov @ 2018-07-09 10:52 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: syzbot, Andrew Morton, Dan Williams, Jerome Glisse,
	Kirill A. Shutemov, ldufour, LKML, Linux-MM, Michal Hocko,
	Minchan Kim, Ross Zwisler, syzkaller-bugs, Matthew Wilcox,
	ying.huang

On Mon, Jul 9, 2018 at 12:48 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Mon, Jul 9, 2018 at 12:15 PM, Kirill A. Shutemov
> <kirill@shutemov.name> wrote:
>> On Sun, Jul 08, 2018 at 10:51:03PM -0700, syzbot wrote:
>>> Hello,
>>>
>>> syzbot found the following crash on:
>>>
>>> HEAD commit:    b2d44d145d2a Merge tag '4.18-rc3-smb3fixes' of git://git.s..
>>> git tree:       upstream
>>> console output: https://syzkaller.appspot.com/x/log.txt?x=11d07748400000
>>> kernel config:  https://syzkaller.appspot.com/x/.config?x=2ca6c7a31d407f86
>>> dashboard link: https://syzkaller.appspot.com/bug?extid=3f84280d52be9b7083cc
>>> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
>>>
>>> Unfortunately, I don't have any reproducer for this crash yet.
>>>
>>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>>> Reported-by: syzbot+3f84280d52be9b7083cc@syzkaller.appspotmail.com
>>>
>>> next ffff8801ce5e7040 prev ffff8801d20eca50 mm ffff88019c1e13c0
>>> prot 27 anon_vma ffff88019680cdd8 vm_ops 0000000000000000
>>> pgoff 0 file ffff8801b2ec2d00 private_data 0000000000000000
>>> flags: 0xff(read|write|exec|shared|mayread|maywrite|mayexec|mayshare)
>>> ------------[ cut here ]------------
>>> kernel BUG at mm/memory.c:1422!
>>
>> Looks like vma_is_anonymous() false-positive.
>>
>> Any clues what file is it? I would guess some kind of socket, but it's not
>> clear from log which exactly.
>
>
> From the log it looks like it was this program (number 3 matches Comm:
> syz-executor3):
>
> 08:39:32 executing program 3:
> r0 = socket$nl_route(0x10, 0x3, 0x0)
> bind$netlink(r0, &(0x7f00000002c0)={0x10, 0x0, 0x0, 0x100000}, 0xc)
> getsockname(r0, &(0x7f0000000000)=@pppol2tpv3in6={0x0, 0x0, {0x0,
> <r1=>0xffffffffffffffff, 0x0, 0x0, 0x0, 0x0, {0x0, 0x0, 0x0,
> @loopback}}}, &(0x7f00000000c0)=0x3a)
> mmap(&(0x7f0000e00000/0x200000)=nil, 0x200000, 0x7fdff, 0x11, r1, 0x0)
> ioctl$FIBMAP(r0, 0x1, &(0x7f0000000100)=0x9)
> r2 = socket$inet6(0xa, 0x1000000000002, 0x0)
> ioctl(r2, 0x8912, &(0x7f00000001c0)="796d05ad441e829115ac7fd77200")
> r3 = syz_open_dev$vcsa(&(0x7f0000000140)='/dev/vcsa#\x00', 0x3, 0x2)
> ioctl$VHOST_SET_VRING_ENDIAN(r3, 0x4008af13, &(0x7f0000000180)={0x0, 0x8})
> sendto$inet(0xffffffffffffffff, &(0x7f0000a88f88), 0xffffffffffffff31,
> 0x0, &(0x7f0000e68000)={0x2, 0x0, @multicast2=0xe0000002},
> 0xfffffffffffffeb3)
> ftruncate(r1, 0x6)
> mmap(&(0x7f0000e00000/0x200000)=nil, 0x200000, 0x0, 0x11, r0, 0x0)
> setsockopt$SO_TIMESTAMPING(r1, 0x1, 0x25, &(0x7f0000000080)=0x804, 0x4)
>
> But take what happens here with a grain of salt, it can pretend that
> it's doing one thing, but actually do something different.
> So that r1 passed to ftruncate is something that getsockname returned
> somewhere in the middle of address. And since the socket is not
> actually ppp, it can be just some bytes in the middle of netlink
> address, that than happened to be small and match some existing fd...


This also happened only once so far:
https://syzkaller.appspot.com/bug?extid=3f84280d52be9b7083cc
and I can't reproduce it rerunning this program. So it's either a very
subtle race, or fd in the middle of netlink address magically matched
some fd once, or something else...

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

* Re: kernel BUG at mm/memory.c:LINE!
  2018-07-09 10:52     ` Dmitry Vyukov
@ 2018-07-09 14:21       ` Kirill A. Shutemov
  2018-07-09 15:25         ` Kirill A. Shutemov
  0 siblings, 1 reply; 9+ messages in thread
From: Kirill A. Shutemov @ 2018-07-09 14:21 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: syzbot, Andrew Morton, Dan Williams, Jerome Glisse,
	Kirill A. Shutemov, ldufour, LKML, Linux-MM, Michal Hocko,
	Minchan Kim, Ross Zwisler, syzkaller-bugs, Matthew Wilcox,
	ying.huang

On Mon, Jul 09, 2018 at 12:52:21PM +0200, Dmitry Vyukov wrote:
> On Mon, Jul 9, 2018 at 12:48 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> > On Mon, Jul 9, 2018 at 12:15 PM, Kirill A. Shutemov
> > <kirill@shutemov.name> wrote:
> >> On Sun, Jul 08, 2018 at 10:51:03PM -0700, syzbot wrote:
> >>> Hello,
> >>>
> >>> syzbot found the following crash on:
> >>>
> >>> HEAD commit:    b2d44d145d2a Merge tag '4.18-rc3-smb3fixes' of git://git.s..
> >>> git tree:       upstream
> >>> console output: https://syzkaller.appspot.com/x/log.txt?x=11d07748400000
> >>> kernel config:  https://syzkaller.appspot.com/x/.config?x=2ca6c7a31d407f86
> >>> dashboard link: https://syzkaller.appspot.com/bug?extid=3f84280d52be9b7083cc
> >>> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
> >>>
> >>> Unfortunately, I don't have any reproducer for this crash yet.
> >>>
> >>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> >>> Reported-by: syzbot+3f84280d52be9b7083cc@syzkaller.appspotmail.com
> >>>
> >>> next ffff8801ce5e7040 prev ffff8801d20eca50 mm ffff88019c1e13c0
> >>> prot 27 anon_vma ffff88019680cdd8 vm_ops 0000000000000000
> >>> pgoff 0 file ffff8801b2ec2d00 private_data 0000000000000000
> >>> flags: 0xff(read|write|exec|shared|mayread|maywrite|mayexec|mayshare)
> >>> ------------[ cut here ]------------
> >>> kernel BUG at mm/memory.c:1422!
> >>
> >> Looks like vma_is_anonymous() false-positive.
> >>
> >> Any clues what file is it? I would guess some kind of socket, but it's not
> >> clear from log which exactly.
> >
> >
> > From the log it looks like it was this program (number 3 matches Comm:
> > syz-executor3):
> >
> > 08:39:32 executing program 3:
> > r0 = socket$nl_route(0x10, 0x3, 0x0)
> > bind$netlink(r0, &(0x7f00000002c0)={0x10, 0x0, 0x0, 0x100000}, 0xc)
> > getsockname(r0, &(0x7f0000000000)=@pppol2tpv3in6={0x0, 0x0, {0x0,
> > <r1=>0xffffffffffffffff, 0x0, 0x0, 0x0, 0x0, {0x0, 0x0, 0x0,
> > @loopback}}}, &(0x7f00000000c0)=0x3a)
> > mmap(&(0x7f0000e00000/0x200000)=nil, 0x200000, 0x7fdff, 0x11, r1, 0x0)
> > ioctl$FIBMAP(r0, 0x1, &(0x7f0000000100)=0x9)
> > r2 = socket$inet6(0xa, 0x1000000000002, 0x0)
> > ioctl(r2, 0x8912, &(0x7f00000001c0)="796d05ad441e829115ac7fd77200")
> > r3 = syz_open_dev$vcsa(&(0x7f0000000140)='/dev/vcsa#\x00', 0x3, 0x2)
> > ioctl$VHOST_SET_VRING_ENDIAN(r3, 0x4008af13, &(0x7f0000000180)={0x0, 0x8})
> > sendto$inet(0xffffffffffffffff, &(0x7f0000a88f88), 0xffffffffffffff31,
> > 0x0, &(0x7f0000e68000)={0x2, 0x0, @multicast2=0xe0000002},
> > 0xfffffffffffffeb3)
> > ftruncate(r1, 0x6)
> > mmap(&(0x7f0000e00000/0x200000)=nil, 0x200000, 0x0, 0x11, r0, 0x0)
> > setsockopt$SO_TIMESTAMPING(r1, 0x1, 0x25, &(0x7f0000000080)=0x804, 0x4)
> >
> > But take what happens here with a grain of salt, it can pretend that
> > it's doing one thing, but actually do something different.
> > So that r1 passed to ftruncate is something that getsockname returned
> > somewhere in the middle of address. And since the socket is not
> > actually ppp, it can be just some bytes in the middle of netlink
> > address, that than happened to be small and match some existing fd...
> 
> 
> This also happened only once so far:
> https://syzkaller.appspot.com/bug?extid=3f84280d52be9b7083cc
> and I can't reproduce it rerunning this program. So it's either a very
> subtle race, or fd in the middle of netlink address magically matched
> some fd once, or something else...

Okay, I've got it reproduced. See below.

The problem is that kcov doesn't set vm_ops for the VMA and it makes
kernel think that the VMA is anonymous.

It's not necessary the way it was triggered by syzkaller. I just found
that kcov's ->mmap doesn't set vm_ops. There can more such cases.
vma_is_anonymous() is what we need to fix.

( Although, I found logic around mmaping the file second time questinable
  at best. It seems broken to me. )

It is known that vma_is_anonymous() can produce false-positives. It tried
to fix it once[1], but it back-fired[2].

I'll look at this again.

[1] https://lkml.kernel.org/r/1437133993-91885-1-git-send-email-kirill.shutemov@linux.intel.com
[2] https://lkml.kernel.org/r/20150914105346.GB23878@arm.com

#include <stdio.h>
#include <stddef.h>
#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/ioctl.h>
#include <sys/mman.h>
#include <unistd.h>
#include <fcntl.h>

#define KCOV_INIT_TRACE			_IOR('c', 1, unsigned long)
#define KCOV_ENABLE			_IO('c', 100)
#define KCOV_DISABLE			_IO('c', 101)
#define COVER_SIZE			(1024<<10)

#define KCOV_TRACE_PC  0
#define KCOV_TRACE_CMP 1

int main(int argc, char **argv)
{
    int fd;
    unsigned long *cover, n, i;

    system("mount -t debugfs none /sys/kernel/debug");
    fd = open("/sys/kernel/debug/kcov", O_RDWR);
    ioctl(fd, KCOV_INIT_TRACE, COVER_SIZE);
    cover = mmap(NULL, COVER_SIZE * sizeof(unsigned long),
		    PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
    munmap(cover, COVER_SIZE * sizeof(unsigned long));
    cover = mmap(NULL, COVER_SIZE * sizeof(unsigned long),
    			     PROT_READ | PROT_WRITE, MAP_PRIVATE, fd, 0);
    memset(cover, 0, COVER_SIZE * sizeof(unsigned long));
    ftruncate(fd, 3UL << 20);
    return 0;
}

-- 
 Kirill A. Shutemov

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

* Re: kernel BUG at mm/memory.c:LINE!
  2018-07-09 14:21       ` Kirill A. Shutemov
@ 2018-07-09 15:25         ` Kirill A. Shutemov
  2018-07-09 17:23           ` Dmitry Vyukov
  0 siblings, 1 reply; 9+ messages in thread
From: Kirill A. Shutemov @ 2018-07-09 15:25 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: syzbot, Andrew Morton, Dan Williams, Jerome Glisse,
	Kirill A. Shutemov, ldufour, LKML, Linux-MM, Michal Hocko,
	Minchan Kim, Ross Zwisler, syzkaller-bugs, Matthew Wilcox,
	ying.huang

On Mon, Jul 09, 2018 at 05:21:55PM +0300, Kirill A. Shutemov wrote:
> > This also happened only once so far:
> > https://syzkaller.appspot.com/bug?extid=3f84280d52be9b7083cc
> > and I can't reproduce it rerunning this program. So it's either a very
> > subtle race, or fd in the middle of netlink address magically matched
> > some fd once, or something else...
> 
> Okay, I've got it reproduced. See below.
> 
> The problem is that kcov doesn't set vm_ops for the VMA and it makes
> kernel think that the VMA is anonymous.
> 
> It's not necessary the way it was triggered by syzkaller. I just found
> that kcov's ->mmap doesn't set vm_ops. There can more such cases.
> vma_is_anonymous() is what we need to fix.
> 
> ( Although, I found logic around mmaping the file second time questinable
>   at best. It seems broken to me. )
> 
> It is known that vma_is_anonymous() can produce false-positives. It tried
> to fix it once[1], but it back-fired[2].
> 
> I'll look at this again.

Below is a patch that seems work. But it definately requires more testing.

Dmitry, could you give it a try in syzkaller?

diff --git a/drivers/char/mem.c b/drivers/char/mem.c
index ffeb60d3434c..f0a8b0b1768b 100644
--- a/drivers/char/mem.c
+++ b/drivers/char/mem.c
@@ -708,6 +708,7 @@ static int mmap_zero(struct file *file, struct vm_area_struct *vma)
 #endif
 	if (vma->vm_flags & VM_SHARED)
 		return shmem_zero_setup(vma);
+	vma->vm_ops = &anon_vm_ops;
 	return 0;
 }
 
diff --git a/fs/exec.c b/fs/exec.c
index 2d4e0075bd24..a1a246062561 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -307,6 +307,7 @@ static int __bprm_mm_init(struct linux_binprm *bprm)
 	 * configured yet.
 	 */
 	BUILD_BUG_ON(VM_STACK_FLAGS & VM_STACK_INCOMPLETE_SETUP);
+	vma->vm_ops = &anon_vm_ops;
 	vma->vm_end = STACK_TOP_MAX;
 	vma->vm_start = vma->vm_end - PAGE_SIZE;
 	vma->vm_flags = VM_SOFTDIRTY | VM_STACK_FLAGS | VM_STACK_INCOMPLETE_SETUP;
diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
index 2015d8c45e4a..945c3d306d8f 100644
--- a/fs/kernfs/file.c
+++ b/fs/kernfs/file.c
@@ -336,9 +336,6 @@ static void kernfs_vma_open(struct vm_area_struct *vma)
 	struct file *file = vma->vm_file;
 	struct kernfs_open_file *of = kernfs_of(file);
 
-	if (!of->vm_ops)
-		return;
-
 	if (!kernfs_get_active(of->kn))
 		return;
 
@@ -354,9 +351,6 @@ static vm_fault_t kernfs_vma_fault(struct vm_fault *vmf)
 	struct kernfs_open_file *of = kernfs_of(file);
 	vm_fault_t ret;
 
-	if (!of->vm_ops)
-		return VM_FAULT_SIGBUS;
-
 	if (!kernfs_get_active(of->kn))
 		return VM_FAULT_SIGBUS;
 
@@ -374,9 +368,6 @@ static vm_fault_t kernfs_vma_page_mkwrite(struct vm_fault *vmf)
 	struct kernfs_open_file *of = kernfs_of(file);
 	vm_fault_t ret;
 
-	if (!of->vm_ops)
-		return VM_FAULT_SIGBUS;
-
 	if (!kernfs_get_active(of->kn))
 		return VM_FAULT_SIGBUS;
 
@@ -397,9 +388,6 @@ static int kernfs_vma_access(struct vm_area_struct *vma, unsigned long addr,
 	struct kernfs_open_file *of = kernfs_of(file);
 	int ret;
 
-	if (!of->vm_ops)
-		return -EINVAL;
-
 	if (!kernfs_get_active(of->kn))
 		return -EINVAL;
 
@@ -419,9 +407,6 @@ static int kernfs_vma_set_policy(struct vm_area_struct *vma,
 	struct kernfs_open_file *of = kernfs_of(file);
 	int ret;
 
-	if (!of->vm_ops)
-		return 0;
-
 	if (!kernfs_get_active(of->kn))
 		return -EINVAL;
 
@@ -440,9 +425,6 @@ static struct mempolicy *kernfs_vma_get_policy(struct vm_area_struct *vma,
 	struct kernfs_open_file *of = kernfs_of(file);
 	struct mempolicy *pol;
 
-	if (!of->vm_ops)
-		return vma->vm_policy;
-
 	if (!kernfs_get_active(of->kn))
 		return vma->vm_policy;
 
@@ -511,7 +493,7 @@ static int kernfs_fop_mmap(struct file *file, struct vm_area_struct *vma)
 	 * So error if someone is trying to use close.
 	 */
 	rc = -EINVAL;
-	if (vma->vm_ops && vma->vm_ops->close)
+	if (vma->vm_ops->close)
 		goto out_put;
 
 	rc = 0;
diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
index e9679016271f..e959623123e4 100644
--- a/fs/proc/task_mmu.c
+++ b/fs/proc/task_mmu.c
@@ -326,7 +326,7 @@ show_map_vma(struct seq_file *m, struct vm_area_struct *vma, int is_pid)
 		goto done;
 	}
 
-	if (vma->vm_ops && vma->vm_ops->name) {
+	if (vma->vm_ops->name) {
 		name = vma->vm_ops->name(vma);
 		if (name)
 			goto done;
diff --git a/include/linux/mm.h b/include/linux/mm.h
index a0fbb9ffe380..f1db03c919c3 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1536,9 +1536,11 @@ int clear_page_dirty_for_io(struct page *page);
 
 int get_cmdline(struct task_struct *task, char *buffer, int buflen);
 
+extern const struct vm_operations_struct anon_vm_ops;
+
 static inline bool vma_is_anonymous(struct vm_area_struct *vma)
 {
-	return !vma->vm_ops;
+	return vma->vm_ops == &anon_vm_ops;
 }
 
 #ifdef CONFIG_SHMEM
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 8f0434a9951a..2e35401a5c68 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7269,7 +7269,7 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
 
 		goto got_name;
 	} else {
-		if (vma->vm_ops && vma->vm_ops->name) {
+		if (vma->vm_ops->name) {
 			name = (char *) vma->vm_ops->name(vma);
 			if (name)
 				goto cpy_name;
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 039ddbc574e9..2065acc5a6aa 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -637,7 +637,7 @@ EXPORT_SYMBOL_GPL(linear_hugepage_index);
  */
 unsigned long vma_kernel_pagesize(struct vm_area_struct *vma)
 {
-	if (vma->vm_ops && vma->vm_ops->pagesize)
+	if (vma->vm_ops->pagesize)
 		return vma->vm_ops->pagesize(vma);
 	return PAGE_SIZE;
 }
diff --git a/mm/khugepaged.c b/mm/khugepaged.c
index d7b2a4bf8671..5ae34097aed1 100644
--- a/mm/khugepaged.c
+++ b/mm/khugepaged.c
@@ -440,7 +440,7 @@ int khugepaged_enter_vma_merge(struct vm_area_struct *vma,
 		 * page fault if needed.
 		 */
 		return 0;
-	if (vma->vm_ops || (vm_flags & VM_NO_KHUGEPAGED))
+	if (!vma_is_anonymous(vma) || (vm_flags & VM_NO_KHUGEPAGED))
 		/* khugepaged not yet working on file or special mappings */
 		return 0;
 	hstart = (vma->vm_start + ~HPAGE_PMD_MASK) & HPAGE_PMD_MASK;
@@ -831,7 +831,7 @@ static bool hugepage_vma_check(struct vm_area_struct *vma)
 		return IS_ALIGNED((vma->vm_start >> PAGE_SHIFT) - vma->vm_pgoff,
 				HPAGE_PMD_NR);
 	}
-	if (!vma->anon_vma || vma->vm_ops)
+	if (!vma->anon_vma || !vma_is_anonymous(vma))
 		return false;
 	if (is_vma_temporary_stack(vma))
 		return false;
diff --git a/mm/memory.c b/mm/memory.c
index 7206a634270b..02fbef2bd024 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -768,7 +768,7 @@ static void print_bad_pte(struct vm_area_struct *vma, unsigned long addr,
 		 (void *)addr, vma->vm_flags, vma->anon_vma, mapping, index);
 	pr_alert("file:%pD fault:%pf mmap:%pf readpage:%pf\n",
 		 vma->vm_file,
-		 vma->vm_ops ? vma->vm_ops->fault : NULL,
+		 vma->vm_ops->fault,
 		 vma->vm_file ? vma->vm_file->f_op->mmap : NULL,
 		 mapping ? mapping->a_ops->readpage : NULL);
 	dump_stack();
@@ -825,7 +825,7 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr,
 	if (IS_ENABLED(CONFIG_ARCH_HAS_PTE_SPECIAL)) {
 		if (likely(!pte_special(pte)))
 			goto check_pfn;
-		if (vma->vm_ops && vma->vm_ops->find_special_page)
+		if (vma->vm_ops->find_special_page)
 			return vma->vm_ops->find_special_page(vma, addr);
 		if (vma->vm_flags & (VM_PFNMAP | VM_MIXEDMAP))
 			return NULL;
@@ -2404,7 +2404,7 @@ static void fault_dirty_shared_page(struct vm_area_struct *vma,
 {
 	struct address_space *mapping;
 	bool dirtied;
-	bool page_mkwrite = vma->vm_ops && vma->vm_ops->page_mkwrite;
+	bool page_mkwrite = vma->vm_ops->page_mkwrite;
 
 	dirtied = set_page_dirty(page);
 	VM_BUG_ON_PAGE(PageAnon(page), page);
@@ -2648,7 +2648,7 @@ static int wp_pfn_shared(struct vm_fault *vmf)
 {
 	struct vm_area_struct *vma = vmf->vma;
 
-	if (vma->vm_ops && vma->vm_ops->pfn_mkwrite) {
+	if (vma->vm_ops->pfn_mkwrite) {
 		int ret;
 
 		pte_unmap_unlock(vmf->pte, vmf->ptl);
@@ -2669,7 +2669,7 @@ static int wp_page_shared(struct vm_fault *vmf)
 
 	get_page(vmf->page);
 
-	if (vma->vm_ops && vma->vm_ops->page_mkwrite) {
+	if (vma->vm_ops->page_mkwrite) {
 		int tmp;
 
 		pte_unmap_unlock(vmf->pte, vmf->ptl);
@@ -4439,7 +4439,7 @@ int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
 			vma = find_vma(mm, addr);
 			if (!vma || vma->vm_start > addr)
 				break;
-			if (vma->vm_ops && vma->vm_ops->access)
+			if (vma->vm_ops->access)
 				ret = vma->vm_ops->access(vma, addr, buf,
 							  len, write);
 			if (ret <= 0)
diff --git a/mm/mempolicy.c b/mm/mempolicy.c
index 9ac49ef17b4e..f0fcf70bcec7 100644
--- a/mm/mempolicy.c
+++ b/mm/mempolicy.c
@@ -651,13 +651,13 @@ static int vma_replace_policy(struct vm_area_struct *vma,
 	pr_debug("vma %lx-%lx/%lx vm_ops %p vm_file %p set_policy %p\n",
 		 vma->vm_start, vma->vm_end, vma->vm_pgoff,
 		 vma->vm_ops, vma->vm_file,
-		 vma->vm_ops ? vma->vm_ops->set_policy : NULL);
+		 vma->vm_ops->set_policy);
 
 	new = mpol_dup(pol);
 	if (IS_ERR(new))
 		return PTR_ERR(new);
 
-	if (vma->vm_ops && vma->vm_ops->set_policy) {
+	if (vma->vm_ops->set_policy) {
 		err = vma->vm_ops->set_policy(vma, new);
 		if (err)
 			goto err_out;
@@ -845,7 +845,7 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
 			up_read(&mm->mmap_sem);
 			return -EFAULT;
 		}
-		if (vma->vm_ops && vma->vm_ops->get_policy)
+		if (vma->vm_ops->get_policy)
 			pol = vma->vm_ops->get_policy(vma, addr);
 		else
 			pol = vma->vm_policy;
@@ -1617,7 +1617,7 @@ struct mempolicy *__get_vma_policy(struct vm_area_struct *vma,
 	struct mempolicy *pol = NULL;
 
 	if (vma) {
-		if (vma->vm_ops && vma->vm_ops->get_policy) {
+		if (vma->vm_ops->get_policy) {
 			pol = vma->vm_ops->get_policy(vma, addr);
 		} else if (vma->vm_policy) {
 			pol = vma->vm_policy;
@@ -1663,7 +1663,7 @@ bool vma_policy_mof(struct vm_area_struct *vma)
 {
 	struct mempolicy *pol;
 
-	if (vma->vm_ops && vma->vm_ops->get_policy) {
+	if (vma->vm_ops->get_policy) {
 		bool ret = false;
 
 		pol = vma->vm_ops->get_policy(vma, vma->vm_start);
diff --git a/mm/mmap.c b/mm/mmap.c
index d1eb87ef4b1a..516fb5c5bfe5 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -71,6 +71,8 @@ int mmap_rnd_compat_bits __read_mostly = CONFIG_ARCH_MMAP_RND_COMPAT_BITS;
 static bool ignore_rlimit_data;
 core_param(ignore_rlimit_data, ignore_rlimit_data, bool, 0644);
 
+const struct vm_operations_struct anon_vm_ops = {};
+
 static void unmap_region(struct mm_struct *mm,
 		struct vm_area_struct *vma, struct vm_area_struct *prev,
 		unsigned long start, unsigned long end);
@@ -177,7 +179,7 @@ static struct vm_area_struct *remove_vma(struct vm_area_struct *vma)
 	struct vm_area_struct *next = vma->vm_next;
 
 	might_sleep();
-	if (vma->vm_ops && vma->vm_ops->close)
+	if (vma->vm_ops->close)
 		vma->vm_ops->close(vma);
 	if (vma->vm_file)
 		fput(vma->vm_file);
@@ -561,6 +563,8 @@ static unsigned long count_vma_pages_range(struct mm_struct *mm,
 void __vma_link_rb(struct mm_struct *mm, struct vm_area_struct *vma,
 		struct rb_node **rb_link, struct rb_node *rb_parent)
 {
+	WARN_ONCE(!vma->vm_ops, "missing vma->vm_ops");
+
 	/* Update tracking information for the gap following the new vma. */
 	if (vma->vm_next)
 		vma_gap_update(vma->vm_next);
@@ -996,7 +1000,7 @@ static inline int is_mergeable_vma(struct vm_area_struct *vma,
 		return 0;
 	if (vma->vm_file != file)
 		return 0;
-	if (vma->vm_ops && vma->vm_ops->close)
+	if (vma->vm_ops->close)
 		return 0;
 	if (!is_mergeable_vm_userfaultfd_ctx(vma, vm_userfaultfd_ctx))
 		return 0;
@@ -1636,7 +1640,7 @@ int vma_wants_writenotify(struct vm_area_struct *vma, pgprot_t vm_page_prot)
 		return 0;
 
 	/* The backer wishes to know when pages are first written to? */
-	if (vm_ops && (vm_ops->page_mkwrite || vm_ops->pfn_mkwrite))
+	if (vm_ops->page_mkwrite || vm_ops->pfn_mkwrite)
 		return 1;
 
 	/* The open routine did something to the protections that pgprot_modify
@@ -1774,12 +1778,20 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
 		 */
 		WARN_ON_ONCE(addr != vma->vm_start);
 
+		/* All mappings must have ->vm_ops set */
+		if (!vma->vm_ops) {
+			static const struct vm_operations_struct dummy_ops = {};
+			vma->vm_ops = &dummy_ops;
+		}
+
 		addr = vma->vm_start;
 		vm_flags = vma->vm_flags;
 	} else if (vm_flags & VM_SHARED) {
 		error = shmem_zero_setup(vma);
 		if (error)
 			goto free_vma;
+	} else {
+		vma->vm_ops = &anon_vm_ops;
 	}
 
 	vma_link(mm, vma, prev, rb_link, rb_parent);
@@ -2614,7 +2626,7 @@ int __split_vma(struct mm_struct *mm, struct vm_area_struct *vma,
 	struct vm_area_struct *new;
 	int err;
 
-	if (vma->vm_ops && vma->vm_ops->split) {
+	if (vma->vm_ops->split) {
 		err = vma->vm_ops->split(vma, addr);
 		if (err)
 			return err;
@@ -2647,7 +2659,7 @@ int __split_vma(struct mm_struct *mm, struct vm_area_struct *vma,
 	if (new->vm_file)
 		get_file(new->vm_file);
 
-	if (new->vm_ops && new->vm_ops->open)
+	if (new->vm_ops->open)
 		new->vm_ops->open(new);
 
 	if (new_below)
@@ -2661,7 +2673,7 @@ int __split_vma(struct mm_struct *mm, struct vm_area_struct *vma,
 		return 0;
 
 	/* Clean everything up if vma_adjust failed. */
-	if (new->vm_ops && new->vm_ops->close)
+	if (new->vm_ops->close)
 		new->vm_ops->close(new);
 	if (new->vm_file)
 		fput(new->vm_file);
@@ -2999,6 +3011,7 @@ static int do_brk_flags(unsigned long addr, unsigned long request, unsigned long
 
 	INIT_LIST_HEAD(&vma->anon_vma_chain);
 	vma->vm_mm = mm;
+	vma->vm_ops = &anon_vm_ops;
 	vma->vm_start = addr;
 	vma->vm_end = addr + len;
 	vma->vm_pgoff = pgoff;
@@ -3221,7 +3234,7 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
 			goto out_free_mempol;
 		if (new_vma->vm_file)
 			get_file(new_vma->vm_file);
-		if (new_vma->vm_ops && new_vma->vm_ops->open)
+		if (new_vma->vm_ops->open)
 			new_vma->vm_ops->open(new_vma);
 		vma_link(mm, new_vma, prev, rb_link, rb_parent);
 		*need_rmap_locks = false;
diff --git a/mm/mremap.c b/mm/mremap.c
index 5c2e18505f75..7ab222c283de 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -302,7 +302,7 @@ static unsigned long move_vma(struct vm_area_struct *vma,
 				     need_rmap_locks);
 	if (moved_len < old_len) {
 		err = -ENOMEM;
-	} else if (vma->vm_ops && vma->vm_ops->mremap) {
+	} else if (vma->vm_ops->mremap) {
 		err = vma->vm_ops->mremap(new_vma);
 	}
 
diff --git a/mm/nommu.c b/mm/nommu.c
index 4452d8bd9ae4..e7f447bfd704 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -764,7 +764,7 @@ static void delete_vma_from_mm(struct vm_area_struct *vma)
  */
 static void delete_vma(struct mm_struct *mm, struct vm_area_struct *vma)
 {
-	if (vma->vm_ops && vma->vm_ops->close)
+	if (vma->vm_ops->close)
 		vma->vm_ops->close(vma);
 	if (vma->vm_file)
 		fput(vma->vm_file);
@@ -1489,7 +1489,7 @@ int split_vma(struct mm_struct *mm, struct vm_area_struct *vma,
 		region->vm_pgoff = new->vm_pgoff += npages;
 	}
 
-	if (new->vm_ops && new->vm_ops->open)
+	if (new->vm_ops->open)
 		new->vm_ops->open(new);
 
 	delete_vma_from_mm(vma);
diff --git a/mm/shmem.c b/mm/shmem.c
index 2cab84403055..bf991c9230b3 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -1424,6 +1424,7 @@ static void shmem_pseudo_vma_init(struct vm_area_struct *vma,
 	/* Bias interleave by inode number to distribute better across nodes */
 	vma->vm_pgoff = index + info->vfs_inode.i_ino;
 	vma->vm_policy = mpol_shared_policy_lookup(&info->policy, index);
+	vma->vm_ops = &anon_vm_ops;
 }
 
 static void shmem_pseudo_vma_destroy(struct vm_area_struct *vma)
-- 
 Kirill A. Shutemov

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

* Re: kernel BUG at mm/memory.c:LINE!
  2018-07-09 15:25         ` Kirill A. Shutemov
@ 2018-07-09 17:23           ` Dmitry Vyukov
  2018-07-09 22:07             ` Kirill A. Shutemov
  0 siblings, 1 reply; 9+ messages in thread
From: Dmitry Vyukov @ 2018-07-09 17:23 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: syzbot, Andrew Morton, Dan Williams, Jerome Glisse,
	Kirill A. Shutemov, ldufour, LKML, Linux-MM, Michal Hocko,
	Minchan Kim, Ross Zwisler, syzkaller-bugs, Matthew Wilcox,
	ying.huang

On Mon, Jul 9, 2018 at 5:25 PM, Kirill A. Shutemov <kirill@shutemov.name> wrote:
> On Mon, Jul 09, 2018 at 05:21:55PM +0300, Kirill A. Shutemov wrote:
>> > This also happened only once so far:
>> > https://syzkaller.appspot.com/bug?extid=3f84280d52be9b7083cc
>> > and I can't reproduce it rerunning this program. So it's either a very
>> > subtle race, or fd in the middle of netlink address magically matched
>> > some fd once, or something else...
>>
>> Okay, I've got it reproduced. See below.
>>
>> The problem is that kcov doesn't set vm_ops for the VMA and it makes
>> kernel think that the VMA is anonymous.
>>
>> It's not necessary the way it was triggered by syzkaller. I just found
>> that kcov's ->mmap doesn't set vm_ops. There can more such cases.
>> vma_is_anonymous() is what we need to fix.
>>
>> ( Although, I found logic around mmaping the file second time questinable
>>   at best. It seems broken to me. )
>>
>> It is known that vma_is_anonymous() can produce false-positives. It tried
>> to fix it once[1], but it back-fired[2].
>>
>> I'll look at this again.
>
> Below is a patch that seems work. But it definately requires more testing.
>
> Dmitry, could you give it a try in syzkaller?

Trying.

Not sure what you expect from this. Either way it will be hundreds of
crashes before vs hundreds of crashes after ;)

But one that started popping up is this, looks like it's somewhere
around the code your patch touches:

kasan: CONFIG_KASAN_INLINE enabled
kasan: GPF could be caused by NULL-ptr deref or user memory access
general protection fault: 0000 [#1] SMP KASAN
CPU: 0 PID: 6711 Comm: syz-executor3 Not tainted 4.18.0-rc4+ #43
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
RIP: 0010:__get_vma_policy+0x61/0x160 mm/mempolicy.c:1620
Code: c1 ea 03 80 3c 02 00 0f 85 01 01 00 00 4c 8b a3 90 00 00 00 48
b8 00 00 00 00 00 fc ff df 49 8d 7c 24 68 48 89 fa 48 c1 ea 03 <80> 3c
02 00 0f 85 d0 00 00 00 4d 8b 64 24 68 4d 85 e4 74 22 e8 76
RSP: 0018:ffff880045e3f6c0 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: ffff880045e3fa38 RCX: ffffc9000bf9b000
RDX: 000000000000000d RSI: ffffffff81ad55f2 RDI: 0000000000000068
RBP: ffff880045e3f6d8 R08: ffff880045e3f8a0 R09: ffffed0008bc7f14
R10: fffffbfff108356c R11: ffffffff8841ab63 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
FS:  00007fe8e4089700(0000) GS:ffff88006c800000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000703de8 CR3: 00000000578d8002 CR4: 00000000001606f0
Call Trace:
 get_vma_policy mm/mempolicy.c:1654 [inline]
 huge_node+0x30/0x450 mm/mempolicy.c:1859
 alloc_buddy_huge_page_with_mpol mm/hugetlb.c:1623 [inline]
 alloc_huge_page+0x994/0x1020 mm/hugetlb.c:2051
 hugetlbfs_fallocate+0x81c/0x11f0 fs/hugetlbfs/inode.c:642
 vfs_fallocate+0x348/0x700 fs/open.c:319
 ksys_fallocate+0x46/0x80 fs/open.c:342
 __do_sys_fallocate fs/open.c:350 [inline]
 __se_sys_fallocate fs/open.c:348 [inline]
 __x64_sys_fallocate+0x97/0xf0 fs/open.c:348
 do_syscall_64+0x192/0x760 arch/x86/entry/common.c:290
 entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x455939
Code: 6d b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48
89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d
01 f0 ff ff 0f 83 3b b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007fe8e4088c78 EFLAGS: 00000246 ORIG_RAX: 000000000000011d
RAX: ffffffffffffffda RBX: 000000000070bea0 RCX: 0000000000455939
RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000013
RBP: 00007fe8e40896d4 R08: 0000000000000000 R09: 0000000000000000
R10: 00000000000009a5 R11: 0000000000000246 R12: 00000000ffffffff
R13: 00000000004a343f R14: 00000000006dc468 R15: 0000000000000000
Modules linked in:
Dumping ftrace buffer:
   (ftrace buffer empty)
---[ end trace e788cb1334786adc ]---
RIP: 0010:__get_vma_policy+0x61/0x160 mm/mempolicy.c:1620
Code: c1 ea 03 80 3c 02 00 0f 85 01 01 00 00 4c 8b a3 90 00 00 00 48
b8 00 00 00 00 00 fc ff df 49 8d 7c 24 68 48 89 fa 48 c1 ea 03 <80> 3c
02 00 0f 85 d0 00 00 00 4d 8b 64 24 68 4d 85 e4 74 22 e8 76
RSP: 0018:ffff880045e3f6c0 EFLAGS: 00010202
RAX: dffffc0000000000 RBX: ffff880045e3fa38 RCX: ffffc9000bf9b000
RDX: 000000000000000d RSI: ffffffff81ad55f2 RDI: 0000000000000068
RBP: ffff880045e3f6d8 R08: ffff880045e3f8a0 R09: ffffed0008bc7f14
R10: fffffbfff108356c R11: ffffffff8841ab63 R12: 0000000000000000
R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
FS:  00007fe8e4089700(0000) GS:ffff88006c800000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000000000703de8 CR3: 00000000578d8002 CR4: 00000000001606f0





> diff --git a/drivers/char/mem.c b/drivers/char/mem.c
> index ffeb60d3434c..f0a8b0b1768b 100644
> --- a/drivers/char/mem.c
> +++ b/drivers/char/mem.c
> @@ -708,6 +708,7 @@ static int mmap_zero(struct file *file, struct vm_area_struct *vma)
>  #endif
>         if (vma->vm_flags & VM_SHARED)
>                 return shmem_zero_setup(vma);
> +       vma->vm_ops = &anon_vm_ops;
>         return 0;
>  }
>
> diff --git a/fs/exec.c b/fs/exec.c
> index 2d4e0075bd24..a1a246062561 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -307,6 +307,7 @@ static int __bprm_mm_init(struct linux_binprm *bprm)
>          * configured yet.
>          */
>         BUILD_BUG_ON(VM_STACK_FLAGS & VM_STACK_INCOMPLETE_SETUP);
> +       vma->vm_ops = &anon_vm_ops;
>         vma->vm_end = STACK_TOP_MAX;
>         vma->vm_start = vma->vm_end - PAGE_SIZE;
>         vma->vm_flags = VM_SOFTDIRTY | VM_STACK_FLAGS | VM_STACK_INCOMPLETE_SETUP;
> diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c
> index 2015d8c45e4a..945c3d306d8f 100644
> --- a/fs/kernfs/file.c
> +++ b/fs/kernfs/file.c
> @@ -336,9 +336,6 @@ static void kernfs_vma_open(struct vm_area_struct *vma)
>         struct file *file = vma->vm_file;
>         struct kernfs_open_file *of = kernfs_of(file);
>
> -       if (!of->vm_ops)
> -               return;
> -
>         if (!kernfs_get_active(of->kn))
>                 return;
>
> @@ -354,9 +351,6 @@ static vm_fault_t kernfs_vma_fault(struct vm_fault *vmf)
>         struct kernfs_open_file *of = kernfs_of(file);
>         vm_fault_t ret;
>
> -       if (!of->vm_ops)
> -               return VM_FAULT_SIGBUS;
> -
>         if (!kernfs_get_active(of->kn))
>                 return VM_FAULT_SIGBUS;
>
> @@ -374,9 +368,6 @@ static vm_fault_t kernfs_vma_page_mkwrite(struct vm_fault *vmf)
>         struct kernfs_open_file *of = kernfs_of(file);
>         vm_fault_t ret;
>
> -       if (!of->vm_ops)
> -               return VM_FAULT_SIGBUS;
> -
>         if (!kernfs_get_active(of->kn))
>                 return VM_FAULT_SIGBUS;
>
> @@ -397,9 +388,6 @@ static int kernfs_vma_access(struct vm_area_struct *vma, unsigned long addr,
>         struct kernfs_open_file *of = kernfs_of(file);
>         int ret;
>
> -       if (!of->vm_ops)
> -               return -EINVAL;
> -
>         if (!kernfs_get_active(of->kn))
>                 return -EINVAL;
>
> @@ -419,9 +407,6 @@ static int kernfs_vma_set_policy(struct vm_area_struct *vma,
>         struct kernfs_open_file *of = kernfs_of(file);
>         int ret;
>
> -       if (!of->vm_ops)
> -               return 0;
> -
>         if (!kernfs_get_active(of->kn))
>                 return -EINVAL;
>
> @@ -440,9 +425,6 @@ static struct mempolicy *kernfs_vma_get_policy(struct vm_area_struct *vma,
>         struct kernfs_open_file *of = kernfs_of(file);
>         struct mempolicy *pol;
>
> -       if (!of->vm_ops)
> -               return vma->vm_policy;
> -
>         if (!kernfs_get_active(of->kn))
>                 return vma->vm_policy;
>
> @@ -511,7 +493,7 @@ static int kernfs_fop_mmap(struct file *file, struct vm_area_struct *vma)
>          * So error if someone is trying to use close.
>          */
>         rc = -EINVAL;
> -       if (vma->vm_ops && vma->vm_ops->close)
> +       if (vma->vm_ops->close)
>                 goto out_put;
>
>         rc = 0;
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index e9679016271f..e959623123e4 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -326,7 +326,7 @@ show_map_vma(struct seq_file *m, struct vm_area_struct *vma, int is_pid)
>                 goto done;
>         }
>
> -       if (vma->vm_ops && vma->vm_ops->name) {
> +       if (vma->vm_ops->name) {
>                 name = vma->vm_ops->name(vma);
>                 if (name)
>                         goto done;
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index a0fbb9ffe380..f1db03c919c3 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1536,9 +1536,11 @@ int clear_page_dirty_for_io(struct page *page);
>
>  int get_cmdline(struct task_struct *task, char *buffer, int buflen);
>
> +extern const struct vm_operations_struct anon_vm_ops;
> +
>  static inline bool vma_is_anonymous(struct vm_area_struct *vma)
>  {
> -       return !vma->vm_ops;
> +       return vma->vm_ops == &anon_vm_ops;
>  }
>
>  #ifdef CONFIG_SHMEM
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 8f0434a9951a..2e35401a5c68 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7269,7 +7269,7 @@ static void perf_event_mmap_event(struct perf_mmap_event *mmap_event)
>
>                 goto got_name;
>         } else {
> -               if (vma->vm_ops && vma->vm_ops->name) {
> +               if (vma->vm_ops->name) {
>                         name = (char *) vma->vm_ops->name(vma);
>                         if (name)
>                                 goto cpy_name;
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 039ddbc574e9..2065acc5a6aa 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -637,7 +637,7 @@ EXPORT_SYMBOL_GPL(linear_hugepage_index);
>   */
>  unsigned long vma_kernel_pagesize(struct vm_area_struct *vma)
>  {
> -       if (vma->vm_ops && vma->vm_ops->pagesize)
> +       if (vma->vm_ops->pagesize)
>                 return vma->vm_ops->pagesize(vma);
>         return PAGE_SIZE;
>  }
> diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> index d7b2a4bf8671..5ae34097aed1 100644
> --- a/mm/khugepaged.c
> +++ b/mm/khugepaged.c
> @@ -440,7 +440,7 @@ int khugepaged_enter_vma_merge(struct vm_area_struct *vma,
>                  * page fault if needed.
>                  */
>                 return 0;
> -       if (vma->vm_ops || (vm_flags & VM_NO_KHUGEPAGED))
> +       if (!vma_is_anonymous(vma) || (vm_flags & VM_NO_KHUGEPAGED))
>                 /* khugepaged not yet working on file or special mappings */
>                 return 0;
>         hstart = (vma->vm_start + ~HPAGE_PMD_MASK) & HPAGE_PMD_MASK;
> @@ -831,7 +831,7 @@ static bool hugepage_vma_check(struct vm_area_struct *vma)
>                 return IS_ALIGNED((vma->vm_start >> PAGE_SHIFT) - vma->vm_pgoff,
>                                 HPAGE_PMD_NR);
>         }
> -       if (!vma->anon_vma || vma->vm_ops)
> +       if (!vma->anon_vma || !vma_is_anonymous(vma))
>                 return false;
>         if (is_vma_temporary_stack(vma))
>                 return false;
> diff --git a/mm/memory.c b/mm/memory.c
> index 7206a634270b..02fbef2bd024 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -768,7 +768,7 @@ static void print_bad_pte(struct vm_area_struct *vma, unsigned long addr,
>                  (void *)addr, vma->vm_flags, vma->anon_vma, mapping, index);
>         pr_alert("file:%pD fault:%pf mmap:%pf readpage:%pf\n",
>                  vma->vm_file,
> -                vma->vm_ops ? vma->vm_ops->fault : NULL,
> +                vma->vm_ops->fault,
>                  vma->vm_file ? vma->vm_file->f_op->mmap : NULL,
>                  mapping ? mapping->a_ops->readpage : NULL);
>         dump_stack();
> @@ -825,7 +825,7 @@ struct page *_vm_normal_page(struct vm_area_struct *vma, unsigned long addr,
>         if (IS_ENABLED(CONFIG_ARCH_HAS_PTE_SPECIAL)) {
>                 if (likely(!pte_special(pte)))
>                         goto check_pfn;
> -               if (vma->vm_ops && vma->vm_ops->find_special_page)
> +               if (vma->vm_ops->find_special_page)
>                         return vma->vm_ops->find_special_page(vma, addr);
>                 if (vma->vm_flags & (VM_PFNMAP | VM_MIXEDMAP))
>                         return NULL;
> @@ -2404,7 +2404,7 @@ static void fault_dirty_shared_page(struct vm_area_struct *vma,
>  {
>         struct address_space *mapping;
>         bool dirtied;
> -       bool page_mkwrite = vma->vm_ops && vma->vm_ops->page_mkwrite;
> +       bool page_mkwrite = vma->vm_ops->page_mkwrite;
>
>         dirtied = set_page_dirty(page);
>         VM_BUG_ON_PAGE(PageAnon(page), page);
> @@ -2648,7 +2648,7 @@ static int wp_pfn_shared(struct vm_fault *vmf)
>  {
>         struct vm_area_struct *vma = vmf->vma;
>
> -       if (vma->vm_ops && vma->vm_ops->pfn_mkwrite) {
> +       if (vma->vm_ops->pfn_mkwrite) {
>                 int ret;
>
>                 pte_unmap_unlock(vmf->pte, vmf->ptl);
> @@ -2669,7 +2669,7 @@ static int wp_page_shared(struct vm_fault *vmf)
>
>         get_page(vmf->page);
>
> -       if (vma->vm_ops && vma->vm_ops->page_mkwrite) {
> +       if (vma->vm_ops->page_mkwrite) {
>                 int tmp;
>
>                 pte_unmap_unlock(vmf->pte, vmf->ptl);
> @@ -4439,7 +4439,7 @@ int __access_remote_vm(struct task_struct *tsk, struct mm_struct *mm,
>                         vma = find_vma(mm, addr);
>                         if (!vma || vma->vm_start > addr)
>                                 break;
> -                       if (vma->vm_ops && vma->vm_ops->access)
> +                       if (vma->vm_ops->access)
>                                 ret = vma->vm_ops->access(vma, addr, buf,
>                                                           len, write);
>                         if (ret <= 0)
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 9ac49ef17b4e..f0fcf70bcec7 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -651,13 +651,13 @@ static int vma_replace_policy(struct vm_area_struct *vma,
>         pr_debug("vma %lx-%lx/%lx vm_ops %p vm_file %p set_policy %p\n",
>                  vma->vm_start, vma->vm_end, vma->vm_pgoff,
>                  vma->vm_ops, vma->vm_file,
> -                vma->vm_ops ? vma->vm_ops->set_policy : NULL);
> +                vma->vm_ops->set_policy);
>
>         new = mpol_dup(pol);
>         if (IS_ERR(new))
>                 return PTR_ERR(new);
>
> -       if (vma->vm_ops && vma->vm_ops->set_policy) {
> +       if (vma->vm_ops->set_policy) {
>                 err = vma->vm_ops->set_policy(vma, new);
>                 if (err)
>                         goto err_out;
> @@ -845,7 +845,7 @@ static long do_get_mempolicy(int *policy, nodemask_t *nmask,
>                         up_read(&mm->mmap_sem);
>                         return -EFAULT;
>                 }
> -               if (vma->vm_ops && vma->vm_ops->get_policy)
> +               if (vma->vm_ops->get_policy)
>                         pol = vma->vm_ops->get_policy(vma, addr);
>                 else
>                         pol = vma->vm_policy;
> @@ -1617,7 +1617,7 @@ struct mempolicy *__get_vma_policy(struct vm_area_struct *vma,
>         struct mempolicy *pol = NULL;
>
>         if (vma) {
> -               if (vma->vm_ops && vma->vm_ops->get_policy) {
> +               if (vma->vm_ops->get_policy) {
>                         pol = vma->vm_ops->get_policy(vma, addr);
>                 } else if (vma->vm_policy) {
>                         pol = vma->vm_policy;
> @@ -1663,7 +1663,7 @@ bool vma_policy_mof(struct vm_area_struct *vma)
>  {
>         struct mempolicy *pol;
>
> -       if (vma->vm_ops && vma->vm_ops->get_policy) {
> +       if (vma->vm_ops->get_policy) {
>                 bool ret = false;
>
>                 pol = vma->vm_ops->get_policy(vma, vma->vm_start);
> diff --git a/mm/mmap.c b/mm/mmap.c
> index d1eb87ef4b1a..516fb5c5bfe5 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -71,6 +71,8 @@ int mmap_rnd_compat_bits __read_mostly = CONFIG_ARCH_MMAP_RND_COMPAT_BITS;
>  static bool ignore_rlimit_data;
>  core_param(ignore_rlimit_data, ignore_rlimit_data, bool, 0644);
>
> +const struct vm_operations_struct anon_vm_ops = {};
> +
>  static void unmap_region(struct mm_struct *mm,
>                 struct vm_area_struct *vma, struct vm_area_struct *prev,
>                 unsigned long start, unsigned long end);
> @@ -177,7 +179,7 @@ static struct vm_area_struct *remove_vma(struct vm_area_struct *vma)
>         struct vm_area_struct *next = vma->vm_next;
>
>         might_sleep();
> -       if (vma->vm_ops && vma->vm_ops->close)
> +       if (vma->vm_ops->close)
>                 vma->vm_ops->close(vma);
>         if (vma->vm_file)
>                 fput(vma->vm_file);
> @@ -561,6 +563,8 @@ static unsigned long count_vma_pages_range(struct mm_struct *mm,
>  void __vma_link_rb(struct mm_struct *mm, struct vm_area_struct *vma,
>                 struct rb_node **rb_link, struct rb_node *rb_parent)
>  {
> +       WARN_ONCE(!vma->vm_ops, "missing vma->vm_ops");
> +
>         /* Update tracking information for the gap following the new vma. */
>         if (vma->vm_next)
>                 vma_gap_update(vma->vm_next);
> @@ -996,7 +1000,7 @@ static inline int is_mergeable_vma(struct vm_area_struct *vma,
>                 return 0;
>         if (vma->vm_file != file)
>                 return 0;
> -       if (vma->vm_ops && vma->vm_ops->close)
> +       if (vma->vm_ops->close)
>                 return 0;
>         if (!is_mergeable_vm_userfaultfd_ctx(vma, vm_userfaultfd_ctx))
>                 return 0;
> @@ -1636,7 +1640,7 @@ int vma_wants_writenotify(struct vm_area_struct *vma, pgprot_t vm_page_prot)
>                 return 0;
>
>         /* The backer wishes to know when pages are first written to? */
> -       if (vm_ops && (vm_ops->page_mkwrite || vm_ops->pfn_mkwrite))
> +       if (vm_ops->page_mkwrite || vm_ops->pfn_mkwrite)
>                 return 1;
>
>         /* The open routine did something to the protections that pgprot_modify
> @@ -1774,12 +1778,20 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
>                  */
>                 WARN_ON_ONCE(addr != vma->vm_start);
>
> +               /* All mappings must have ->vm_ops set */
> +               if (!vma->vm_ops) {
> +                       static const struct vm_operations_struct dummy_ops = {};
> +                       vma->vm_ops = &dummy_ops;
> +               }
> +
>                 addr = vma->vm_start;
>                 vm_flags = vma->vm_flags;
>         } else if (vm_flags & VM_SHARED) {
>                 error = shmem_zero_setup(vma);
>                 if (error)
>                         goto free_vma;
> +       } else {
> +               vma->vm_ops = &anon_vm_ops;
>         }
>
>         vma_link(mm, vma, prev, rb_link, rb_parent);
> @@ -2614,7 +2626,7 @@ int __split_vma(struct mm_struct *mm, struct vm_area_struct *vma,
>         struct vm_area_struct *new;
>         int err;
>
> -       if (vma->vm_ops && vma->vm_ops->split) {
> +       if (vma->vm_ops->split) {
>                 err = vma->vm_ops->split(vma, addr);
>                 if (err)
>                         return err;
> @@ -2647,7 +2659,7 @@ int __split_vma(struct mm_struct *mm, struct vm_area_struct *vma,
>         if (new->vm_file)
>                 get_file(new->vm_file);
>
> -       if (new->vm_ops && new->vm_ops->open)
> +       if (new->vm_ops->open)
>                 new->vm_ops->open(new);
>
>         if (new_below)
> @@ -2661,7 +2673,7 @@ int __split_vma(struct mm_struct *mm, struct vm_area_struct *vma,
>                 return 0;
>
>         /* Clean everything up if vma_adjust failed. */
> -       if (new->vm_ops && new->vm_ops->close)
> +       if (new->vm_ops->close)
>                 new->vm_ops->close(new);
>         if (new->vm_file)
>                 fput(new->vm_file);
> @@ -2999,6 +3011,7 @@ static int do_brk_flags(unsigned long addr, unsigned long request, unsigned long
>
>         INIT_LIST_HEAD(&vma->anon_vma_chain);
>         vma->vm_mm = mm;
> +       vma->vm_ops = &anon_vm_ops;
>         vma->vm_start = addr;
>         vma->vm_end = addr + len;
>         vma->vm_pgoff = pgoff;
> @@ -3221,7 +3234,7 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
>                         goto out_free_mempol;
>                 if (new_vma->vm_file)
>                         get_file(new_vma->vm_file);
> -               if (new_vma->vm_ops && new_vma->vm_ops->open)
> +               if (new_vma->vm_ops->open)
>                         new_vma->vm_ops->open(new_vma);
>                 vma_link(mm, new_vma, prev, rb_link, rb_parent);
>                 *need_rmap_locks = false;
> diff --git a/mm/mremap.c b/mm/mremap.c
> index 5c2e18505f75..7ab222c283de 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -302,7 +302,7 @@ static unsigned long move_vma(struct vm_area_struct *vma,
>                                      need_rmap_locks);
>         if (moved_len < old_len) {
>                 err = -ENOMEM;
> -       } else if (vma->vm_ops && vma->vm_ops->mremap) {
> +       } else if (vma->vm_ops->mremap) {
>                 err = vma->vm_ops->mremap(new_vma);
>         }
>
> diff --git a/mm/nommu.c b/mm/nommu.c
> index 4452d8bd9ae4..e7f447bfd704 100644
> --- a/mm/nommu.c
> +++ b/mm/nommu.c
> @@ -764,7 +764,7 @@ static void delete_vma_from_mm(struct vm_area_struct *vma)
>   */
>  static void delete_vma(struct mm_struct *mm, struct vm_area_struct *vma)
>  {
> -       if (vma->vm_ops && vma->vm_ops->close)
> +       if (vma->vm_ops->close)
>                 vma->vm_ops->close(vma);
>         if (vma->vm_file)
>                 fput(vma->vm_file);
> @@ -1489,7 +1489,7 @@ int split_vma(struct mm_struct *mm, struct vm_area_struct *vma,
>                 region->vm_pgoff = new->vm_pgoff += npages;
>         }
>
> -       if (new->vm_ops && new->vm_ops->open)
> +       if (new->vm_ops->open)
>                 new->vm_ops->open(new);
>
>         delete_vma_from_mm(vma);
> diff --git a/mm/shmem.c b/mm/shmem.c
> index 2cab84403055..bf991c9230b3 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -1424,6 +1424,7 @@ static void shmem_pseudo_vma_init(struct vm_area_struct *vma,
>         /* Bias interleave by inode number to distribute better across nodes */
>         vma->vm_pgoff = index + info->vfs_inode.i_ino;
>         vma->vm_policy = mpol_shared_policy_lookup(&info->policy, index);
> +       vma->vm_ops = &anon_vm_ops;
>  }
>
>  static void shmem_pseudo_vma_destroy(struct vm_area_struct *vma)
> --
>  Kirill A. Shutemov
>
> --
> 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/20180709152508.smwg252x57pnfkoq%40kshutemo-mobl1.
> For more options, visit https://groups.google.com/d/optout.

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

* Re: kernel BUG at mm/memory.c:LINE!
  2018-07-09 17:23           ` Dmitry Vyukov
@ 2018-07-09 22:07             ` Kirill A. Shutemov
  2018-07-10 10:02               ` Dmitry Vyukov
  0 siblings, 1 reply; 9+ messages in thread
From: Kirill A. Shutemov @ 2018-07-09 22:07 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: syzbot, Andrew Morton, Dan Williams, Jerome Glisse,
	Kirill A. Shutemov, ldufour, LKML, Linux-MM, Michal Hocko,
	Minchan Kim, Ross Zwisler, syzkaller-bugs, Matthew Wilcox,
	ying.huang

On Mon, Jul 09, 2018 at 07:23:15PM +0200, Dmitry Vyukov wrote:
> On Mon, Jul 9, 2018 at 5:25 PM, Kirill A. Shutemov <kirill@shutemov.name> wrote:
> > On Mon, Jul 09, 2018 at 05:21:55PM +0300, Kirill A. Shutemov wrote:
> >> > This also happened only once so far:
> >> > https://syzkaller.appspot.com/bug?extid=3f84280d52be9b7083cc
> >> > and I can't reproduce it rerunning this program. So it's either a very
> >> > subtle race, or fd in the middle of netlink address magically matched
> >> > some fd once, or something else...
> >>
> >> Okay, I've got it reproduced. See below.
> >>
> >> The problem is that kcov doesn't set vm_ops for the VMA and it makes
> >> kernel think that the VMA is anonymous.
> >>
> >> It's not necessary the way it was triggered by syzkaller. I just found
> >> that kcov's ->mmap doesn't set vm_ops. There can more such cases.
> >> vma_is_anonymous() is what we need to fix.
> >>
> >> ( Although, I found logic around mmaping the file second time questinable
> >>   at best. It seems broken to me. )
> >>
> >> It is known that vma_is_anonymous() can produce false-positives. It tried
> >> to fix it once[1], but it back-fired[2].
> >>
> >> I'll look at this again.
> >
> > Below is a patch that seems work. But it definately requires more testing.
> >
> > Dmitry, could you give it a try in syzkaller?
> 
> Trying.
> 
> Not sure what you expect from this. Either way it will be hundreds of
> crashes before vs hundreds of crashes after ;)
> 
> But one that started popping up is this, looks like it's somewhere
> around the code your patch touches:
> 
> kasan: CONFIG_KASAN_INLINE enabled
> kasan: GPF could be caused by NULL-ptr deref or user memory access
> general protection fault: 0000 [#1] SMP KASAN
> CPU: 0 PID: 6711 Comm: syz-executor3 Not tainted 4.18.0-rc4+ #43
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
> RIP: 0010:__get_vma_policy+0x61/0x160 mm/mempolicy.c:1620

Right, my bad. Here's fixup.

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index d508c7844681..12b2b3c7f51e 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -597,6 +597,7 @@ static long hugetlbfs_fallocate(struct file *file, int mode, loff_t offset,
 	memset(&pseudo_vma, 0, sizeof(struct vm_area_struct));
 	pseudo_vma.vm_flags = (VM_HUGETLB | VM_MAYSHARE | VM_SHARED);
 	pseudo_vma.vm_file = file;
+	pseudo_vma.vm_ops = &anon_vm_ops;

 	for (index = start; index < end; index++) {
 		/*
-- 
 Kirill A. Shutemov

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

* Re: kernel BUG at mm/memory.c:LINE!
  2018-07-09 22:07             ` Kirill A. Shutemov
@ 2018-07-10 10:02               ` Dmitry Vyukov
  0 siblings, 0 replies; 9+ messages in thread
From: Dmitry Vyukov @ 2018-07-10 10:02 UTC (permalink / raw)
  To: Kirill A. Shutemov
  Cc: syzbot, Andrew Morton, Dan Williams, Jerome Glisse,
	Kirill A. Shutemov, ldufour, LKML, Linux-MM, Michal Hocko,
	Minchan Kim, Ross Zwisler, syzkaller-bugs, Matthew Wilcox,
	ying.huang

On Tue, Jul 10, 2018 at 12:07 AM, Kirill A. Shutemov
<kirill@shutemov.name> wrote:
> On Mon, Jul 09, 2018 at 07:23:15PM +0200, Dmitry Vyukov wrote:
>> On Mon, Jul 9, 2018 at 5:25 PM, Kirill A. Shutemov <kirill@shutemov.name> wrote:
>> > On Mon, Jul 09, 2018 at 05:21:55PM +0300, Kirill A. Shutemov wrote:
>> >> > This also happened only once so far:
>> >> > https://syzkaller.appspot.com/bug?extid=3f84280d52be9b7083cc
>> >> > and I can't reproduce it rerunning this program. So it's either a very
>> >> > subtle race, or fd in the middle of netlink address magically matched
>> >> > some fd once, or something else...
>> >>
>> >> Okay, I've got it reproduced. See below.
>> >>
>> >> The problem is that kcov doesn't set vm_ops for the VMA and it makes
>> >> kernel think that the VMA is anonymous.
>> >>
>> >> It's not necessary the way it was triggered by syzkaller. I just found
>> >> that kcov's ->mmap doesn't set vm_ops. There can more such cases.
>> >> vma_is_anonymous() is what we need to fix.
>> >>
>> >> ( Although, I found logic around mmaping the file second time questinable
>> >>   at best. It seems broken to me. )
>> >>
>> >> It is known that vma_is_anonymous() can produce false-positives. It tried
>> >> to fix it once[1], but it back-fired[2].
>> >>
>> >> I'll look at this again.
>> >
>> > Below is a patch that seems work. But it definately requires more testing.
>> >
>> > Dmitry, could you give it a try in syzkaller?
>>
>> Trying.
>>
>> Not sure what you expect from this. Either way it will be hundreds of
>> crashes before vs hundreds of crashes after ;)
>>
>> But one that started popping up is this, looks like it's somewhere
>> around the code your patch touches:
>>
>> kasan: CONFIG_KASAN_INLINE enabled
>> kasan: GPF could be caused by NULL-ptr deref or user memory access
>> general protection fault: 0000 [#1] SMP KASAN
>> CPU: 0 PID: 6711 Comm: syz-executor3 Not tainted 4.18.0-rc4+ #43
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1 04/01/2014
>> RIP: 0010:__get_vma_policy+0x61/0x160 mm/mempolicy.c:1620
>
> Right, my bad. Here's fixup.
>
> diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
> index d508c7844681..12b2b3c7f51e 100644
> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -597,6 +597,7 @@ static long hugetlbfs_fallocate(struct file *file, int mode, loff_t offset,
>         memset(&pseudo_vma, 0, sizeof(struct vm_area_struct));
>         pseudo_vma.vm_flags = (VM_HUGETLB | VM_MAYSHARE | VM_SHARED);
>         pseudo_vma.vm_file = file;
> +       pseudo_vma.vm_ops = &anon_vm_ops;
>
>         for (index = start; index < end; index++) {
>                 /*


With this change I don't see anything that stands out, just a typical
mix of crashes like these:

BUG: unable to handle kernel paging request in kfree
INFO: task hung in flush_work
KASAN: slab-out-of-bounds Read in fscache_alloc_cookie
KASAN: use-after-free Read in __queue_work
general protection fault in encode_rpcb_string
lost connection to test machine
no output from test machine
unregister_netdevice: waiting for DEV to become free

So I guess this can be qualified as +1 for the patch.

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

end of thread, other threads:[~2018-07-10 10:03 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-09  5:51 kernel BUG at mm/memory.c:LINE! syzbot
2018-07-09 10:15 ` Kirill A. Shutemov
2018-07-09 10:48   ` Dmitry Vyukov
2018-07-09 10:52     ` Dmitry Vyukov
2018-07-09 14:21       ` Kirill A. Shutemov
2018-07-09 15:25         ` Kirill A. Shutemov
2018-07-09 17:23           ` Dmitry Vyukov
2018-07-09 22:07             ` Kirill A. Shutemov
2018-07-10 10:02               ` Dmitry Vyukov

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