* kvm: access to invalid memory in mmu_zap_unsync_children
@ 2016-01-15 16:54 Sasha Levin
2016-01-15 17:06 ` Paolo Bonzini
0 siblings, 1 reply; 4+ messages in thread
From: Sasha Levin @ 2016-01-15 16:54 UTC (permalink / raw)
To: Gleb Natapov, Paolo Bonzini; +Cc: LKML, LKML, Dmitry Vyukov, syzkaller
Hi all,
While fuzzing with syzkaller on the latest -next kernel running on a KVM tools
guest, I've hit the following invalid memory access:
[ 547.956284] UBSAN: Undefined behaviour in arch/x86/kvm/mmu.c:2011:17
[ 547.956940] index 3 is out of range for type 'kvm_mmu_page *[3]'
[ 547.957567] CPU: 0 PID: 21577 Comm: syz-executor Tainted: G D 4.4.0-next-20160114-sasha-00021-gf1273d1-dirty #2798
[ 547.958739] 1ffff1001819be5c 000000002fa0e55b ffff8800c0cdf360 ffffffff83433c4e
[ 547.972448] 0000000041b58ab3 ffffffff8f960c38 ffffffff83433b86 ffff8800c0cdf328
[ 547.973277] 0000000000000001 000000002fa0e55b ffffffff8feb8440 ffff8800c0cdf3f0
[ 547.974102] Call Trace:
[ 547.974424] dump_stack (lib/dump_stack.c:52)
[ 547.975774] ubsan_epilogue (lib/ubsan.c:165)
[ 547.976408] __ubsan_handle_out_of_bounds (lib/ubsan.c:382)
[ 547.980877] mmu_zap_unsync_children (arch/x86/kvm/mmu.c:2011 arch/x86/kvm/mmu.c:2272)
[ 548.014193] kvm_mmu_prepare_zap_page (arch/x86/kvm/mmu.c:2294)
[ 548.018228] kvm_mmu_invalidate_zap_all_pages (arch/x86/kvm/mmu.c:4737 arch/x86/kvm/mmu.c:4776)
[ 548.018994] kvm_arch_flush_shadow_all (arch/x86/kvm/x86.c:8050)
[ 548.019696] kvm_mmu_notifier_release (include/linux/rcupdate.h:495 include/linux/srcu.h:237 arch/x86/kvm/../../../virt/kvm/kvm_main.c:447)
[ 548.021657] __mmu_notifier_release (mm/mmu_notifier.c:66 (discriminator 19))
[ 548.024574] exit_mmap (include/linux/mmu_notifier.h:235 mm/mmap.c:2819)
[ 548.058887] mmput (kernel/fork.c:707)
[ 548.059572] do_exit (./arch/x86/include/asm/bitops.h:311 include/linux/thread_info.h:92 kernel/exit.c:437 kernel/exit.c:735)
[ 548.061733] do_group_exit (include/linux/sched.h:813 kernel/exit.c:861)
[ 548.062400] get_signal (kernel/signal.c:2327)
[ 548.063063] do_signal (arch/x86/kernel/signal.c:781)
[ 548.071394] exit_to_usermode_loop (arch/x86/entry/common.c:247)
[ 548.072066] syscall_return_slowpath (arch/x86/entry/common.c:282 arch/x86/entry/common.c:344)
[ 548.072824] int_ret_from_sys_call (arch/x86/entry/entry_64.S:282)
Thanks,
Sashagl
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: kvm: access to invalid memory in mmu_zap_unsync_children
2016-01-15 16:54 kvm: access to invalid memory in mmu_zap_unsync_children Sasha Levin
@ 2016-01-15 17:06 ` Paolo Bonzini
2016-01-18 5:00 ` Xiao Guangrong
0 siblings, 1 reply; 4+ messages in thread
From: Paolo Bonzini @ 2016-01-15 17:06 UTC (permalink / raw)
To: Sasha Levin, Gleb Natapov
Cc: LKML, Dmitry Vyukov, syzkaller, Marcelo Tosatti,
Takuya Yoshikawa, Xiao Guangrong
On 15/01/2016 17:54, Sasha Levin wrote:
> Hi all,
>
> While fuzzing with syzkaller on the latest -next kernel running on a KVM tools
> guest, I've hit the following invalid memory access:
>
> [ 547.956284] UBSAN: Undefined behaviour in arch/x86/kvm/mmu.c:2011:17
>
> [ 547.956940] index 3 is out of range for type 'kvm_mmu_page *[3]'
>
> [ 547.957567] CPU: 0 PID: 21577 Comm: syz-executor Tainted: G D 4.4.0-next-20160114-sasha-00021-gf1273d1-dirty #2798
>
> [ 547.958739] 1ffff1001819be5c 000000002fa0e55b ffff8800c0cdf360 ffffffff83433c4e
>
> [ 547.972448] 0000000041b58ab3 ffffffff8f960c38 ffffffff83433b86 ffff8800c0cdf328
>
> [ 547.973277] 0000000000000001 000000002fa0e55b ffffffff8feb8440 ffff8800c0cdf3f0
>
> [ 547.974102] Call Trace:
>
> [ 547.974424] dump_stack (lib/dump_stack.c:52)
> [ 547.975774] ubsan_epilogue (lib/ubsan.c:165)
> [ 547.976408] __ubsan_handle_out_of_bounds (lib/ubsan.c:382)
> [ 547.980877] mmu_zap_unsync_children (arch/x86/kvm/mmu.c:2011 arch/x86/kvm/mmu.c:2272)
Marcelo/Takuya/Xiao,
do you know what's the point in the assignment in kvm_mmu_pages_init?
It seems to me that it should be
parents->parent[0] = NULL;
since the only user of the ->parent[] array, mmu_pages_clear_parents,
walks the array up from parents->parent[0].
Any other opinions?
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: kvm: access to invalid memory in mmu_zap_unsync_children
2016-01-15 17:06 ` Paolo Bonzini
@ 2016-01-18 5:00 ` Xiao Guangrong
2016-01-18 6:25 ` Takuya Yoshikawa
0 siblings, 1 reply; 4+ messages in thread
From: Xiao Guangrong @ 2016-01-18 5:00 UTC (permalink / raw)
To: Paolo Bonzini, Sasha Levin, Gleb Natapov
Cc: LKML, Dmitry Vyukov, syzkaller, Marcelo Tosatti, Takuya Yoshikawa
On 01/16/2016 01:06 AM, Paolo Bonzini wrote:
>
>
> On 15/01/2016 17:54, Sasha Levin wrote:
>> Hi all,
>>
>> While fuzzing with syzkaller on the latest -next kernel running on a KVM tools
>> guest, I've hit the following invalid memory access:
>>
>> [ 547.956284] UBSAN: Undefined behaviour in arch/x86/kvm/mmu.c:2011:17
>>
>> [ 547.956940] index 3 is out of range for type 'kvm_mmu_page *[3]'
>>
>> [ 547.957567] CPU: 0 PID: 21577 Comm: syz-executor Tainted: G D 4.4.0-next-20160114-sasha-00021-gf1273d1-dirty #2798
>>
>> [ 547.958739] 1ffff1001819be5c 000000002fa0e55b ffff8800c0cdf360 ffffffff83433c4e
>>
>> [ 547.972448] 0000000041b58ab3 ffffffff8f960c38 ffffffff83433b86 ffff8800c0cdf328
>>
>> [ 547.973277] 0000000000000001 000000002fa0e55b ffffffff8feb8440 ffff8800c0cdf3f0
>>
>> [ 547.974102] Call Trace:
>>
>> [ 547.974424] dump_stack (lib/dump_stack.c:52)
>> [ 547.975774] ubsan_epilogue (lib/ubsan.c:165)
>> [ 547.976408] __ubsan_handle_out_of_bounds (lib/ubsan.c:382)
>> [ 547.980877] mmu_zap_unsync_children (arch/x86/kvm/mmu.c:2011 arch/x86/kvm/mmu.c:2272)
>
> Marcelo/Takuya/Xiao,
>
> do you know what's the point in the assignment in kvm_mmu_pages_init?
>
> It seems to me that it should be
>
> parents->parent[0] = NULL;
>
> since the only user of the ->parent[] array, mmu_pages_clear_parents,
> walks the array up from parents->parent[0].
>
Yes, it is bugly and it surprised me that it was not triggered in nested env.
> Any other opinions?
The idea we use the array as [PT64_ROOT_LEVEL-1] is because we never take
the last level (level = 1) into account.
I think this diff can fix this, but it has not tested yet.
+#define INVALID_INDEX (-1)
+
static int mmu_unsync_walk(struct kvm_mmu_page *sp,
struct kvm_mmu_pages *pvec)
{
if (!sp->unsync_children)
return 0;
- mmu_pages_add(pvec, sp, 0);
+ /*
+ * do not count the index in the parent of the sp we're
+ * walking start from.
+ */
+ mmu_pages_add(pvec, sp, INVALID_INDEX);
return __mmu_unsync_walk(sp, pvec);
}
@@ -1980,8 +1986,11 @@ static int mmu_pages_next(struct kvm_mmu_pages *pvec,
return n;
}
- parents->parent[sp->role.level-2] = sp;
- parents->idx[sp->role.level-1] = pvec->page[n].idx;
+ parents->parent[sp->role.level - 2] = sp;
+
+ /* skip setting idex of the sp we start from. */
+ if (pvec->page[n].idx != INVALID_INDEX)
+ parents->idx[sp->role.level - 1] = pvec->page[n].idx;
}
return n;
@@ -1999,6 +2008,7 @@ static void mmu_pages_clear_parents(struct mmu_page_path *parents)
if (!sp)
return;
+ WARN_ON(idx != INVALID_INDEX);
clear_unsync_child_bit(sp, idx);
level++;
} while (level < PT64_ROOT_LEVEL-1 && !sp->unsync_children);
@@ -2008,7 +2018,7 @@ static void kvm_mmu_pages_init(struct kvm_mmu_page *parent,
struct mmu_page_path *parents,
struct kvm_mmu_pages *pvec)
{
- parents->parent[parent->role.level-1] = NULL;
+ parents->parent[parent->role.level - 2] = NULL;
pvec->nr = 0;
}
>
> Paolo
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: kvm: access to invalid memory in mmu_zap_unsync_children
2016-01-18 5:00 ` Xiao Guangrong
@ 2016-01-18 6:25 ` Takuya Yoshikawa
0 siblings, 0 replies; 4+ messages in thread
From: Takuya Yoshikawa @ 2016-01-18 6:25 UTC (permalink / raw)
To: Xiao Guangrong, Paolo Bonzini, Sasha Levin, Gleb Natapov
Cc: LKML, Dmitry Vyukov, syzkaller, Marcelo Tosatti
On 2016/01/18 14:00, Xiao Guangrong wrote:
> On 01/16/2016 01:06 AM, Paolo Bonzini wrote:
>> On 15/01/2016 17:54, Sasha Levin wrote:
>>> Hi all,
>>>
>>> While fuzzing with syzkaller on the latest -next kernel running on a
>>> KVM tools
>>> guest, I've hit the following invalid memory access:
>>>
>>> [ 547.956284] UBSAN: Undefined behaviour in arch/x86/kvm/mmu.c:2011:17
>>>
>>> [ 547.956940] index 3 is out of range for type 'kvm_mmu_page *[3]'
>>>
>>> [ 547.957567] CPU: 0 PID: 21577 Comm: syz-executor Tainted: G
>>> D 4.4.0-next-20160114-sasha-00021-gf1273d1-dirty #2798
>>>
>>> [ 547.958739] 1ffff1001819be5c 000000002fa0e55b ffff8800c0cdf360
>>> ffffffff83433c4e
>>>
>>> [ 547.972448] 0000000041b58ab3 ffffffff8f960c38 ffffffff83433b86
>>> ffff8800c0cdf328
>>>
>>> [ 547.973277] 0000000000000001 000000002fa0e55b ffffffff8feb8440
>>> ffff8800c0cdf3f0
>>>
>>> [ 547.974102] Call Trace:
>>>
>>> [ 547.974424] dump_stack (lib/dump_stack.c:52)
>>> [ 547.975774] ubsan_epilogue (lib/ubsan.c:165)
>>> [ 547.976408] __ubsan_handle_out_of_bounds (lib/ubsan.c:382)
>>> [ 547.980877] mmu_zap_unsync_children (arch/x86/kvm/mmu.c:2011
>>> arch/x86/kvm/mmu.c:2272)
>>
>> Marcelo/Takuya/Xiao,
>>
>> do you know what's the point in the assignment in kvm_mmu_pages_init?
>>
>> It seems to me that it should be
>>
>> parents->parent[0] = NULL;
>>
>> since the only user of the ->parent[] array, mmu_pages_clear_parents,
>> walks the array up from parents->parent[0].
Triggering "index 3 is out of range for type 'kvm_mmu_page *[3]'"
in the first kvm_mmu_pages_init() call in mmu_zap_unsync_children()
means the parent passed in to mmu_zap_unsync_children() has its
->role.level set to PT64_ROOT_LEVEL.
Is this the problem being reported?
Maybe, I'm just confused by the incorrect line-numbers and it's
possible that the problem was triggered in the while loop.
> Yes, it is bugly and it surprised me that it was not triggered in nested
> env.
Yes, the code is not changed much from the following commit:
KVM: MMU: use page array in unsync walk
commit 60c8aec6e2c9923492dabbd6b67e34692bd26c20
What, including my recent cleanups, could change the situation to make
this happen? I'm not sure. It's almost seven years.
>> Any other opinions?
>
> The idea we use the array as [PT64_ROOT_LEVEL-1] is because we never take
> the last level (level = 1) into account.
Yes, this is reasonable.
> I think this diff can fix this, but it has not tested yet.
I'm still reading the code but not sure what changed the situation.
Takuya
> +#define INVALID_INDEX (-1)
> +
> static int mmu_unsync_walk(struct kvm_mmu_page *sp,
> struct kvm_mmu_pages *pvec)
> {
> if (!sp->unsync_children)
> return 0;
>
> - mmu_pages_add(pvec, sp, 0);
> + /*
> + * do not count the index in the parent of the sp we're
> + * walking start from.
> + */
> + mmu_pages_add(pvec, sp, INVALID_INDEX);
> return __mmu_unsync_walk(sp, pvec);
> }
>
> @@ -1980,8 +1986,11 @@ static int mmu_pages_next(struct kvm_mmu_pages
> *pvec,
> return n;
> }
>
> - parents->parent[sp->role.level-2] = sp;
> - parents->idx[sp->role.level-1] = pvec->page[n].idx;
> + parents->parent[sp->role.level - 2] = sp;
> +
> + /* skip setting idex of the sp we start from. */
> + if (pvec->page[n].idx != INVALID_INDEX)
> + parents->idx[sp->role.level - 1] = pvec->page[n].idx;
> }
>
> return n;
> @@ -1999,6 +2008,7 @@ static void mmu_pages_clear_parents(struct
> mmu_page_path *parents)
> if (!sp)
> return;
>
> + WARN_ON(idx != INVALID_INDEX);
> clear_unsync_child_bit(sp, idx);
> level++;
> } while (level < PT64_ROOT_LEVEL-1 && !sp->unsync_children);
> @@ -2008,7 +2018,7 @@ static void kvm_mmu_pages_init(struct kvm_mmu_page
> *parent,
> struct mmu_page_path *parents,
> struct kvm_mmu_pages *pvec)
> {
> - parents->parent[parent->role.level-1] = NULL;
> + parents->parent[parent->role.level - 2] = NULL;
> pvec->nr = 0;
> }
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-01-18 6:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-15 16:54 kvm: access to invalid memory in mmu_zap_unsync_children Sasha Levin
2016-01-15 17:06 ` Paolo Bonzini
2016-01-18 5:00 ` Xiao Guangrong
2016-01-18 6:25 ` Takuya Yoshikawa
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.