From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 060671D5ABA; Tue, 15 Jul 2025 08:31:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1752568282; cv=none; b=u8SR3XxCSa8wI3xSW3RqfsdydfawdCwdPJ0vHQuRPiVFQSZemSRYpJ/mbsvWXH2dMR+/emEynpOnr0AM3Tnx4D9uX6ca+DkVnuBeQ69liK3MZGeLqzHIRMhK8y275Ds2AumYoiL2g2kcG3odNin+HFPLcP41B0dv6IjdiaTJEaw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1752568282; c=relaxed/simple; bh=vc8sRdp2JkS0f6MKjzIVxwH/P9BFLMdE+QPnjQlwYQI=; h=Date:Message-ID:From:To:Cc:Subject:In-Reply-To:References: MIME-Version:Content-Type; b=kiAzgo9VlsuAL7xvw52fDaRjjWcfKS2XOR67QmG+pyE+c0inFb5phMcFYS3V/jNsz04HPSzbGQTjrQNuWRDeM2ZvnLs/MoPo2/ieatIgBF1/Zydpq/2tfph5npuEgJlDaHSXP3SCPzuDKHOnubyE6ADVM6PM0Ocpaj4M8dWwn8Q= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=LlNIijNk; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="LlNIijNk" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 7E3FCC4CEE3; Tue, 15 Jul 2025 08:31:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1752568281; bh=vc8sRdp2JkS0f6MKjzIVxwH/P9BFLMdE+QPnjQlwYQI=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=LlNIijNklPQOePOENYJKmMSyRYEpLVgmZUM+p2trP4XEl+EVWsWzniSK914ybz6Nq Kndd87u/GPxZMCK7WuuBwYoQrdwOPrFPUML6HFiUF4+is3D/ZJJF4wXUgGx/Si4DL5 /RUg6+TYDFsEDievEqzV1QD6KK6WFMXGvzAoICOrl7tBLpxzexv0vx0eDFbk+LWzsQ nURsAZx2+jLx7hWOKZ4vU+Qkh8zpyelO/M+3Kt3xcJGXmY833vC4p7+T6WhmNMIa1Q GJNmYJpk4A7p6L7iOJWUIk1T2Pi4FcXwUVw5+EQfV7g0fUd5wCJQm1e47XHvt4S87z VEJjqpPrdWBkQ== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.95) (envelope-from ) id 1ubb4I-00Fndi-N4; Tue, 15 Jul 2025 09:31:18 +0100 Date: Tue, 15 Jul 2025 09:31:18 +0100 Message-ID: <86zfd597op.wl-maz@kernel.org> From: Marc Zyngier To: Oliver Upton Cc: kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, Joey Gouly , Suzuki K Poulose , Zenghui Yu , syzbot+4e09b1432de3774b86ae@syzkaller.appspotmail.com Subject: Re: [PATCH] KVM: arm64: Clear pending exception state before injecting a new one In-Reply-To: References: <20250714144636.3569479-1-maz@kernel.org> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/30.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) Precedence: bulk X-Mailing-List: kvm@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: oliver.upton@linux.dev, kvmarm@lists.linux.dev, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, joey.gouly@arm.com, suzuki.poulose@arm.com, yuzenghui@huawei.com, syzbot+4e09b1432de3774b86ae@syzkaller.appspotmail.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false On Tue, 15 Jul 2025 07:51:09 +0100, Oliver Upton wrote: > > Hey, > > On Mon, Jul 14, 2025 at 03:46:36PM +0100, Marc Zyngier wrote: > > Repeatedly injecting an exception from userspace without running > > the vcpu between calls results in a nasty warning, as we're not > > really keen on losing already pending exceptions. > > > > But this precaution doesn't really apply to userspace, who can > > do whatever it wants (within reason). So let's simply clear any > > previous exception state before injecting a new one. > > > > Note that this is done unconditionally, even if the injection > > ultimately fails. > > > > Reported-by: syzbot+4e09b1432de3774b86ae@syzkaller.appspotmail.com > > Signed-off-by: Marc Zyngier > > Thanks for taking a look at this. I think the correct fix is a bit more > involved, as: > > - ABI prior to my patches allowed dumb things like injecting both an > SEA and SError from the same ioctl. With your patch I think you could > still get the warning to fire with serror_pending && ext_dabt_pending > > - KVM_GET_VCPU_EVENTS is broken for 'pending' SEAs, as we assume > they're committed in the vCPU state immediately when they're actually > deferred to the next KVM_RUN. > > I thoroughly hate the fix I have but it should address both of these > issues. Although the pending PC adjustment flags seem more like a > liability than anything else if ioctls need to flush them before > returning to userspace. Might look at a larger cleanup down the road. > > Thanks, > Oliver > > From 149262689dfe881542f5c5b60f9ee308a00f0596 Mon Sep 17 00:00:00 2001 > From: Oliver Upton > Date: Mon, 14 Jul 2025 23:25:07 -0700 > Subject: [PATCH] KVM: arm64: Commit exceptions from KVM_SET_VCPU_EVENTS > immediately > > syzkaller has found that it can trip a warning in KVM's exception > emulation infrastructure by repeatedly injecting exceptions into the > guest. > > While it's unlikely that a reasonable VMM will do this, further > investigation of the issue reveals that KVM can potentially discard the > "pending" SEA state. While the handling of KVM_GET_VCPU_EVENTS presumes > that userspace-injected SEAs are realized immediately, in reality the > emulated exception entry is deferred until the next call to KVM_RUN. > > Hack-a-fix the immediate issues by committing the pending exceptions to > the vCPU's architectural state immediately in KVM_SET_VCPU_EVENTS. This > is no different to the way KVM-injected exceptions are handled in > KVM_RUN where we potentially call __kvm_adjust_pc() before returning to > userspace. > > Signed-off-by: Oliver Upton > --- > arch/arm64/kvm/guest.c | 28 +++++++++++++++++++++++++++- > 1 file changed, 27 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c > index e2702718d56d..16ba5e9ac86c 100644 > --- a/arch/arm64/kvm/guest.c > +++ b/arch/arm64/kvm/guest.c > @@ -834,6 +834,19 @@ int __kvm_arm_vcpu_get_events(struct kvm_vcpu *vcpu, > return 0; > } > > +static void commit_pending_events(struct kvm_vcpu *vcpu) > +{ > + if (!vcpu_get_flag(vcpu, PENDING_EXCEPTION)) > + return; > + > + /* > + * Reset the MMIO emulation state to avoid stepping PC after emulating > + * the exception entry. > + */ > + vcpu->mmio_needed = false; > + kvm_call_hyp(__kvm_adjust_pc, vcpu); > +} > + > int __kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu, > struct kvm_vcpu_events *events) > { > @@ -843,8 +856,15 @@ int __kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu, > u64 esr = events->exception.serror_esr; > int ret = 0; > > - if (ext_dabt_pending) > + /* > + * Immediately commit the pending SEA to the vCPU's architectural > + * state which is necessary since we do not return a pending SEA > + * to userspace via KVM_GET_VCPU_EVENTS. > + */ > + if (ext_dabt_pending) { > ret = kvm_inject_sea_dabt(vcpu, kvm_vcpu_get_hfar(vcpu)); > + commit_pending_events(vcpu); > + } > > if (ret < 0) > return ret; > @@ -863,6 +883,12 @@ int __kvm_arm_vcpu_set_events(struct kvm_vcpu *vcpu, > else > ret = kvm_inject_serror(vcpu); > > + /* > + * We could've decided that the SError is due for immediate software > + * injection; commit the exception in case userspace decides it wants > + * to inject more exceptions for some strange reason. > + */ > + commit_pending_events(vcpu); > return (ret < 0) ? ret : 0; > } Yup, this seems looks reasonable. I would have liked the commit helper to be used just before vcpu_put(), but obviously, that's not the right thing to do because of the mmio_needed field. Reviewed-by: Marc Zyngier M. -- Without deviation from the norm, progress is not possible.