WARNING in __x86_set_memory_region

[syzbot]

Hello,

syzkaller hit the following crash on
0a07b238e5f488b459b6113a62e06b6aab017f71
git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master
compiler: gcc (GCC) 7.1.1 20170620
.config is attached
Raw console output is attached.

syzkaller reproducer is attached. See https://goo.gl/kgGztJ
for information about syzkaller reproducers


------------[ cut here ]------------
WARNING: CPU: 1 PID: 6842 at arch/x86/kvm/x86.c:8152
__x86_set_memory_region+0x541/0x6c0 arch/x86/kvm/x86.c:8152
Kernel panic - not syncing: panic_on_warn set ...

CPU: 1 PID: 6842 Comm: syz-executor7 Not tainted 4.13.0-rc2+ #10
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:16 [inline]
dump_stack+0x194/0x257 lib/dump_stack.c:52
panic+0x1e4/0x417 kernel/panic.c:180
__warn+0x1c4/0x1d9 kernel/panic.c:541
report_bug+0x211/0x2d0 lib/bug.c:183
fixup_bug+0x40/0x90 arch/x86/kernel/traps.c:190
do_trap_no_signal arch/x86/kernel/traps.c:224 [inline]
do_trap+0x260/0x390 arch/x86/kernel/traps.c:273
do_error_trap+0x120/0x390 arch/x86/kernel/traps.c:310
do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:323
invalid_op+0x1e/0x30 arch/x86/entry/entry_64.S:846
RIP: 0010:__x86_set_memory_region+0x541/0x6c0 arch/x86/kvm/x86.c:8152
RSP: 0018:ffff8801d39277f0 EFLAGS: 00010297
RAX: ffff8801c26ee100 RBX: ffff8801d39278c0 RCX: 1ffff100384ddd2e
RDX: 0000000000000000 RSI: 0000000000000002 RDI: 0000000000000286
RBP: ffff8801d39278e8 R08: 0000000000000001 R09: 1ffff1003a724eb9
R10: ffff8801d3927590 R11: 0000000000000001 R12: ffff8801d3927880
R13: 1ffff1003a724f04 R14: 00000000000101ff R15: 0000000000000000
x86_set_memory_region+0x3e/0x60 arch/x86/kvm/x86.c:8164
kvm_arch_destroy_vm+0x7c4/0x990 arch/x86/kvm/x86.c:8180
kvm_destroy_vm arch/x86/kvm/../../../virt/kvm/kvm_main.c:770 [inline]
kvm_put_kvm+0x5d7/0xa90 arch/x86/kvm/../../../virt/kvm/kvm_main.c:792
kvm_vcpu_release+0x7b/0xa0 arch/x86/kvm/../../../virt/kvm/kvm_main.c:2412
__fput+0x327/0x7e0 fs/file_table.c:210
____fput+0x15/0x20 fs/file_table.c:246
task_work_run+0x18a/0x260 kernel/task_work.c:116
tracehook_notify_resume include/linux/tracehook.h:191 [inline]
exit_to_usermode_loop+0x26d/0x2d0 arch/x86/entry/common.c:161
prepare_exit_to_usermode arch/x86/entry/common.c:194 [inline]
syscall_return_slowpath+0x3a7/0x450 arch/x86/entry/common.c:263
entry_SYSCALL_64_fastpath+0xbc/0xbe
RIP: 0033:0x4512c9
RSP: 002b:00007f7577010c08 EFLAGS: 00000216 ORIG_RAX: 0000000000000021
RAX: 000000000000000a RBX: 0000000000718000 RCX: 00000000004512c9
RDX: 0000000000000000 RSI: 000000000000000a RDI: 0000000000000007
RBP: 00000000000004b0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000216 R12: 00000000004b6699
R13: 00000000ffffffff R14: 0000000000000007 R15: 000000000000000a
Dumping ftrace buffer:
(ftrace buffer empty)
Kernel Offset: disabled
Rebooting in 86400 seconds..


---
This bug is generated by a dumb bot. It may contain errors.
See https://goo.gl/tpsmEJ for details.
Direct all questions to syzk...@googlegroups.com.
Please credit me with: Reported-by: syzbot <syzk...@googlegroups.com>

syzbot will keep track of this bug report.
Once a fix for this bug is committed, please reply to this email with:
#syz fix: exact-commit-title
To mark this as a duplicate of another syzbot report, please reply with:
#syz dup: exact-subject-of-another-report
If it's a one-off invalid bug report, please reply with:
#syz invalid
Note: if the crash happens again, it will cause creation of a new bug
report.
Note: all commands must start from beginning of the line.

[syzbot]

syzkaller has found reproducer for the following crash on
5a3517e009e979f21977d362212b7729c5165d92
git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/master

compiler: gcc (GCC) 7.1.1 20170620
.config is attached
Raw console output is attached.

C reproducer is attached

syzkaller reproducer is attached. See https://goo.gl/kgGztJ
for information about syzkaller reproducers


------------[ cut here ]------------

WARNING: CPU: 1 PID: 6656 at arch/x86/kvm/x86.c:8243
__x86_set_memory_region+0x56e/0x7a0 arch/x86/kvm/x86.c:8243

Kernel panic - not syncing: panic_on_warn set ...

CPU: 1 PID: 6656 Comm: syzkaller223273 Not tainted
4.14.0-rc7-next-20171103+ #38

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+0x194/0x257 lib/dump_stack.c:53
panic+0x1e4/0x41c kernel/panic.c:183
__warn+0x1c4/0x1e0 kernel/panic.c:546
report_bug+0x211/0x2d0 lib/bug.c:184
fixup_bug+0x40/0x90 arch/x86/kernel/traps.c:177
do_trap_no_signal arch/x86/kernel/traps.c:211 [inline]
do_trap+0x260/0x390 arch/x86/kernel/traps.c:260
do_error_trap+0x120/0x390 arch/x86/kernel/traps.c:297
do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:310
invalid_op+0x18/0x20 arch/x86/entry/entry_64.S:906
RIP: 0010:__x86_set_memory_region+0x56e/0x7a0 arch/x86/kvm/x86.c:8243
RSP: 0018:ffff8801c12b7690 EFLAGS: 00010293
RAX: ffff8801c89b85c0 RBX: ffff8801c12b7798 RCX: ffffffff810b4d6e
RDX: 0000000000000000 RSI: 0000000000000002 RDI: ffffed0038256ea0
RBP: ffff8801c12b77c0 R08: 0000000000000001 R09: 1ffff10038256e85
R10: ffff8801c89b85c0 R11: 0000000000000001 R12: ffff8801c12b7758
R13: 1ffff10038256ed7 R14: ffff8801bdc62ac0 R15: 0000000000000000
x86_set_memory_region+0x3e/0x60 arch/x86/kvm/x86.c:8255
kvm_arch_destroy_vm+0x7c4/0x990 arch/x86/kvm/x86.c:8271
kvm_destroy_vm arch/x86/kvm/../../../virt/kvm/kvm_main.c:726 [inline]
kvm_put_kvm+0x695/0xde0 arch/x86/kvm/../../../virt/kvm/kvm_main.c:747
kvm_vcpu_release+0x7b/0xa0 arch/x86/kvm/../../../virt/kvm/kvm_main.c:2371
__fput+0x333/0x7f0 fs/file_table.c:210
____fput+0x15/0x20 fs/file_table.c:244
task_work_run+0x199/0x270 kernel/task_work.c:113
tracehook_notify_resume include/linux/tracehook.h:191 [inline]
exit_to_usermode_loop+0x296/0x310 arch/x86/entry/common.c:162
prepare_exit_to_usermode arch/x86/entry/common.c:197 [inline]
syscall_return_slowpath+0x42f/0x510 arch/x86/entry/common.c:266
entry_SYSCALL_64_fastpath+0xbc/0xbe
RIP: 0033:0x445ba9
RSP: 002b:00007f8d49285dc8 EFLAGS: 00000202 ORIG_RAX: 0000000000000124
RAX: 0000000000000005 RBX: 0000000000000000 RCX: 0000000000445ba9
RDX: 0000000000000000 RSI: 0000000000000005 RDI: 0000000000000003
RBP: 0000000000000000 R08: 00007f8d49286700 R09: 00007f8d49286700
R10: 00007f8d49286700 R11: 0000000000000202 R12: 0000000000000000
R13: 00007ffee68801ef R14: 00007f8d492869c0 R15: 0000000000000000

[Eric Biggers]

From: Eric Biggers <ebig...@google.com>

On x86, special KVM memslots such as the TSS region have anonymous
memory mappings created on behalf of userspace, and these mappings are
removed when the VM is destroyed.

It is however possible for removing these mappings via vm_munmap() to
fail. This can most easily happen if the thread receives SIGKILL while
it's waiting to acquire ->mmap_sem. This triggers the 'WARN_ON(r < 0)'
in __x86_set_memory_region(). syzkaller was able to hit this, using
'exit()' to send the SIGKILL. Note that while the vm_munmap() failure
results in the mapping not being removed immediately, it is not leaked
forever but rather will be freed when the process exits.

It's not really possible to handle this failure properly, so almost
every other caller of vm_munmap() doesn't check the return value. It's
a limitation of having the kernel manage these mappings rather than
userspace.

So just remove the WARN_ON() so that users can't spam the kernel log
with this warning.

Fixes: f0d648bdf0a5 ("KVM: x86: map/unmap private slots in __x86_set_memory_region")
Reported-by: syzbot <syzk...@googlegroups.com>
Signed-off-by: Eric Biggers <ebig...@google.com>
---
arch/x86/kvm/x86.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index c53298dfbf50..53b57f18baec 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8272,10 +8272,8 @@ int __x86_set_memory_region(struct kvm *kvm, int id, gpa_t gpa, u32 size)
return r;
}

- if (!size) {
- r = vm_munmap(old.userspace_addr, old.npages * PAGE_SIZE);
- WARN_ON(r < 0);
- }
+ if (!size)
+ vm_munmap(old.userspace_addr, old.npages * PAGE_SIZE);

return 0;
}
--
2.16.0.rc1.238.g530d649a79-goog

[Radim Krčmář]

2018-01-31 17:30-0800, Eric Biggers:

> From: Eric Biggers <ebig...@google.com>
>
> On x86, special KVM memslots such as the TSS region have anonymous
> memory mappings created on behalf of userspace, and these mappings are
> removed when the VM is destroyed.
>
> It is however possible for removing these mappings via vm_munmap() to
> fail. This can most easily happen if the thread receives SIGKILL while
> it's waiting to acquire ->mmap_sem. This triggers the 'WARN_ON(r < 0)'
> in __x86_set_memory_region(). syzkaller was able to hit this, using
> 'exit()' to send the SIGKILL. Note that while the vm_munmap() failure
> results in the mapping not being removed immediately, it is not leaked
> forever but rather will be freed when the process exits.
>
> It's not really possible to handle this failure properly, so almost

We could check "r < 0 && r != -EINTR" to get rid of the easily
triggerable warning.

> every other caller of vm_munmap() doesn't check the return value. It's
> a limitation of having the kernel manage these mappings rather than
> userspace.
>
> So just remove the WARN_ON() so that users can't spam the kernel log
> with this warning.
>
> Fixes: f0d648bdf0a5 ("KVM: x86: map/unmap private slots in __x86_set_memory_region")
> Reported-by: syzbot <syzk...@googlegroups.com>
> Signed-off-by: Eric Biggers <ebig...@google.com>
> ---

Removing it altogether doesn't sound that bad, though ...
Queued, thanks.

[Paolo Bonzini]

On 01/02/2018 10:33, Radim Krčmář wrote:
> 2018-01-31 17:30-0800, Eric Biggers:
>> From: Eric Biggers <ebig...@google.com>
>>
>> On x86, special KVM memslots such as the TSS region have anonymous
>> memory mappings created on behalf of userspace, and these mappings are
>> removed when the VM is destroyed.
>>
>> It is however possible for removing these mappings via vm_munmap() to
>> fail. This can most easily happen if the thread receives SIGKILL while
>> it's waiting to acquire ->mmap_sem. This triggers the 'WARN_ON(r < 0)'
>> in __x86_set_memory_region(). syzkaller was able to hit this, using
>> 'exit()' to send the SIGKILL. Note that while the vm_munmap() failure
>> results in the mapping not being removed immediately, it is not leaked
>> forever but rather will be freed when the process exits.
>>
>> It's not really possible to handle this failure properly, so almost
>
> We could check "r < 0 && r != -EINTR" to get rid of the easily
> triggerable warning.

Considering that vm_munmap uses down_write_killable, that would be
preferrable I think.

Paolo

[Eric Biggers]

On Thu, Feb 01, 2018 at 12:12:00PM -0500, Paolo Bonzini wrote:
> On 01/02/2018 10:33, Radim Krčmář wrote:
> > 2018-01-31 17:30-0800, Eric Biggers:
> >> From: Eric Biggers <ebig...@google.com>
> >>
> >> On x86, special KVM memslots such as the TSS region have anonymous
> >> memory mappings created on behalf of userspace, and these mappings are
> >> removed when the VM is destroyed.
> >>
> >> It is however possible for removing these mappings via vm_munmap() to
> >> fail. This can most easily happen if the thread receives SIGKILL while
> >> it's waiting to acquire ->mmap_sem. This triggers the 'WARN_ON(r < 0)'
> >> in __x86_set_memory_region(). syzkaller was able to hit this, using
> >> 'exit()' to send the SIGKILL. Note that while the vm_munmap() failure
> >> results in the mapping not being removed immediately, it is not leaked
> >> forever but rather will be freed when the process exits.
> >>
> >> It's not really possible to handle this failure properly, so almost
> >
> > We could check "r < 0 && r != -EINTR" to get rid of the easily
> > triggerable warning.
>
> Considering that vm_munmap uses down_write_killable, that would be
> preferrable I think.
>

Don't be so sure that vm_munmap() can't fail for other reasons as well :-)
Remember, userspace can mess around with its address space.

And indeed, looking closer, I see there was a previous report of this same WARN
on an older kernel which in vm_munmap() still had down_write() instead of
down_write_killable(). The reproducer in that case concurrently called
personality(ADDR_LIMIT_3GB) to reduce its address limit after the mapping was
already created above 3 GiB. Then the vm_munmap() returned EINVAL since
'start > TASK_SIZE'.

So I don't think we should check for specific error codes. We could make it a
pr_warn_ratelimited() though, if we still want some notification that there was
a problem without implying it is a kernel bug as WARN_ON() does.

- Eric

[Eric Biggers]

Fix is in kvm/queue now:

#syz fix: KVM/x86: remove WARN_ON() for when vm_munmap() fails

- Eric