Re: [syzbot] [kvm?] WARNING in __kvm_gpc_refresh

[David Woodhouse]

On Mon, 2024-03-18 at 14:34 -0700, Sean Christopherson wrote:
> On Mon, Mar 18, 2024, David Woodhouse wrote:
> > 
> >         /* 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().

Not sure I even want a gpc of length 1 to work at INVALID_GPA; I don't
think it's the offset+length check we want to be looking at?

If we've activated the gpc with gpa==INVALID_GPA, surely the right
thing to do is just let it fail (perhaps with an explicit check or just
letting the memslot lookup fail). After fixing that WARN_ON be

   if (WARN_ON_ONCE(!kvm_is_error_gpa(gpa) && !kvm_is_error_hva(uhva)))

Attachment: smime.p7s
Description: S/MIME cryptographic signature