* 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox