linux-kernel - Re: [syzbot] [kvm?] WARNING in __kvm_gpc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Mon, 18 Mar 2024 14:34:27 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: David Woodhouse <dwmw2@...radead.org>
Cc: syzbot <syzbot+106a4f72b0474e1d1b33@...kaller.appspotmail.com>, 
	kvm@...r.kernel.org, linux-kernel@...r.kernel.org, pbonzini@...hat.com, 
	syzkaller-bugs@...glegroups.com, paul <paul@....org>
Subject: Re: [syzbot] [kvm?] WARNING in __kvm_gpc_refresh

On Mon, Mar 18, 2024, David Woodhouse wrote:
> On Mon, 2024-03-18 at 09:25 -0700, syzbot wrote:
> > Hello,
> > 
> > syzbot found the following issue on:
> > 
> > HEAD commit:    277100b3d5fe Merge tag 'block-6.9-20240315' of git://git.k..
> > git tree:       upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=17c96aa5180000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=1c6662240382da2
> > dashboard link: https://syzkaller.appspot.com/bug?extid=106a4f72b0474e1d1b33
> > compiler:       gcc (Debian 12.2.0-14) 122.0, GNU ld (GNU Binutils for Debian) 2.40
> > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14358231180000
> > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=110ed231180000
> > 
> > Downloadable assets:
> > disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/7bc7510fe41f/non_bootable_disk-277100b3.raw.xz
> > vmlinux: https://storage.googleapis.com/syzbot-assets/6872e049b27c/vmlinux-277100b3.xz
> > kernel image: https://storage.googleapis.com/syzbot-assets/68ec7230df0f/bzImage-277100b3.xz
> 
> static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long uhva,
>                              unsigned long len)
> {
>         unsigned long page_offset;
>         bool unmap_old = false;
>         unsigned long old_uhva;
>         kvm_pfn_t old_pfn;
>         bool hva_change = false;
>         void *old_khva;
>         int ret;
> 
>         /* Either gpa or uhva must be valid, but not both */
>         if (WARN_ON_ONCE(kvm_is_error_gpa(gpa) == kvm_is_error_hva(uhva)))
>                 return -EINVAL;
> 
> Hm, that comment doesn't match the code. It says "not both", but the
> code also catches the "neither" case. I think the gpa is in %rbx and
> uhva is in %r12, so this is indeed the 'neither' case.
> 
> Is it expected that we can end up with a cache marked active, but with
> the address not valid? Maybe through a race condition with deactive? or
> more likely than that?

It's the darn PV system time MSR, which allows the guest to triggering activation
with any GPA value.  That results in the cache being marked active without KVM
ever setting the GPA (or any other fields).  The fix I'm testing is to move the
offset+len check up into activate() and refresh().