On Mon, Apr 01, 2024, Phi Nguyen wrote: > On 3/29/2024 11:55 AM, syzbot wrote: > > Hello, > > > > syzbot found the following issue on: > > > > HEAD commit: 928a87efa423 Merge tag 'gfs2-v6.8-fix' of git://git.kernel.. > > git tree: upstream > > console output: https://syzkaller.appspot.com/x/log.txt?x=127c0546180000 > > kernel config: https://syzkaller.appspot.com/x/.config?x=f64ec427e98bccd7 > > dashboard link: https://syzkaller.appspot.com/bug?extid=dc308fcfcd53f987de73 > > compiler: gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40 > > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=110481f1180000 > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=177049a5180000 > > > > Downloadable assets: > > disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/7bc7510fe41f/non_bootable_disk-928a87ef.raw.xz > > vmlinux: https://storage.googleapis.com/syzbot-assets/7979568a5a16/vmlinux-928a87ef.xz > > kernel image: https://storage.googleapis.com/syzbot-assets/1bc6e1d480e3/bzImage-928a87ef.xz > > > > IMPORTANT: if you fix the issue, please add the following tag to the commit: > > Reported-by: syzbot+dc308fcfcd53f987de73@xxxxxxxxxxxxxxxxxxxxxxxxx ... > > This report 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@xxxxxxxxxxxxxxxx. > > > > syzbot will keep track of this issue. See: > > https://goo.gl/tpsmEJ#status for how to communicate with syzbot. > > > > If the report is already addressed, let syzbot know by replying with: > > #syz fix: exact-commit-title > > > > If you want syzbot to run the reproducer, reply with: > > #syz test: git://repo/address.git branch-or-commit-hash > > If you attach or paste a git patch, syzbot will apply it before testing. > > > > If you want to overwrite report's subsystems, reply with: > > #syz set subsystems: new-subsystem > > (See the list of subsystem names on the web dashboard) > > > > If the report is a duplicate of another one, reply with: > > #syz dup: exact-subject-of-another-report > > > > If you want to undo deduplication, reply with: > > #syz undup > > > Shadow TDP > > #syz test: > git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master > diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c > index 992e651540e8..b4275dc22d21 100644 > --- a/arch/x86/kvm/mmu/mmu.c > +++ b/arch/x86/kvm/mmu/mmu.c > @@ -3591,7 +3591,7 @@ static void mmu_free_root_page(struct kvm *kvm, hpa_t *root_hpa, > void kvm_mmu_free_roots(struct kvm *kvm, struct kvm_mmu *mmu, > ulong roots_to_free) > { > - bool is_tdp_mmu = tdp_mmu_enabled && mmu->root_role.direct; > + bool is_tdp_mmu = tdp_mmu_enabled; This isn't a proper fix. It would actually make a relative benign bug (taking mmu_lock for write instead of read) far worse (taking mmu_lock for read instead of write). The reproducer doesn't fail because it's not actually running nested VMs, just doing fun things with KVM_SET_CPUID2. The issue is that kvm_mmu_after_set_cpuid() clobbers the entire role, which results in mmu->root_role.direct being garbage. That results in a false negative, but as above it's quite benign as it simply means KVM takes mmu_lock for write, when acquiring for read would suffice. kvm_mmu_page_role.invalid already exists, we just have never used it for the root_role. Unless I'm missing something, the below is the simplest fix. I'll post a patch tomorrow, assuming testing goes well. diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index 77d1072b130d..2a6c573e0c63 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -5540,9 +5540,9 @@ void kvm_mmu_after_set_cpuid(struct kvm_vcpu *vcpu) * that problem is swept under the rug; KVM's CPUID API is horrific and * it's all but impossible to solve it without introducing a new API. */ - vcpu->arch.root_mmu.root_role.word = 0; - vcpu->arch.guest_mmu.root_role.word = 0; - vcpu->arch.nested_mmu.root_role.word = 0; + vcpu->arch.root_mmu.root_role.invalid = 1; + vcpu->arch.guest_mmu.root_role.invalid = 1; + vcpu->arch.nested_mmu.root_role.invalid = 1; vcpu->arch.root_mmu.cpu_role.ext.valid = 0; vcpu->arch.guest_mmu.cpu_role.ext.valid = 0; vcpu->arch.nested_mmu.cpu_role.ext.valid = 0;
