Received-SPF: pass (google.com: domain of linux-kernel+bounces-106708-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Mon, 18 Mar 2024 14:34:27 -0700
In-Reply-To: <b7561e6d6d357fcd8ec1a1257aaf2f97d971061c.camel@infradead.org>
Precedence: bulk
Mime-Version: 1.0
References: <0000000000005fa5cc0613f1cebd@google.com> <b7561e6d6d357fcd8ec1a1257aaf2f97d971061c.camel@infradead.org>
Message-ID: <ZfizYzC9-9Qo47tE@google.com>
Subject: Re: [syzbot] [kvm?] WARNING in __kvm_gpc_refresh
From: Sean Christopherson <seanjc@google.com>
To: David Woodhouse <dwmw2@infradead.org>
Cc: syzbot <syzbot+106a4f72b0474e1d1b33@syzkaller.appspotmail.com>, 
	kvm@vger.kernel.org, linux-kernel@vger.kernel.org, pbonzini@redhat.com, 
	syzkaller-bugs@googlegroups.com, paul <paul@xen.org>
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

On Mon, Mar 18, 2024, David Woodhouse wrote:
> On Mon, 2024-03-18 at 09:25 -0700, syzbot wrote:
> > Hello,
> >=20
> > syzbot found the following issue on:
> >=20
> > HEAD commit:=C2=A0=C2=A0=C2=A0 277100b3d5fe Merge tag 'block-6.9-202403=
15' of git://git.k..
> > git tree:=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=3D17c96aa5180=
000
> > kernel config:=C2=A0 https://syzkaller.appspot.com/x/.config?x=3D1c6662=
240382da2
> > dashboard link: https://syzkaller.appspot.com/bug?extid=3D106a4f72b0474=
e1d1b33
> > compiler:=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 gcc (Debian 12.2.0-14) 12=
2.0, GNU ld (GNU Binutils for Debian) 2.40
> > syz repro:=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 https://syzkaller.appspot.com/=
x/repro.syz?x=3D14358231180000
> > C reproducer:=C2=A0=C2=A0 https://syzkaller.appspot.com/x/repro.c?x=3D1=
10ed231180000
> >=20
> > 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/vmli=
nux-277100b3.xz
> > kernel image: https://storage.googleapis.com/syzbot-assets/68ec7230df0f=
/bzImage-277100b3.xz
>=20
> static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, uns=
igned long uhva,
>                              unsigned long len)
> {
>         unsigned long page_offset;
>         bool unmap_old =3D false;
>         unsigned long old_uhva;
>         kvm_pfn_t old_pfn;
>         bool hva_change =3D false;
>         void *old_khva;
>         int ret;
>=20
>         /* Either gpa or uhva must be valid, but not both */
>         if (WARN_ON_ONCE(kvm_is_error_gpa(gpa) =3D=3D kvm_is_error_hva(uh=
va)))
>                 return -EINVAL;
>=20
> 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.
>=20
> 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 acti=
vation
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().