From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 34FB1C8300A for ; Thu, 30 Apr 2020 12:45:59 +0000 (UTC) Received: from mm01.cs.columbia.edu (mm01.cs.columbia.edu [128.59.11.253]) by mail.kernel.org (Postfix) with ESMTP id B0EAD205ED for ; Thu, 30 Apr 2020 12:45:58 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="SrSWBQ6/" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org B0EAD205ED Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=kvmarm-bounces@lists.cs.columbia.edu Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 410254B426; Thu, 30 Apr 2020 08:45:58 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Authentication-Results: mm01.cs.columbia.edu (amavisd-new); dkim=softfail (fail, message has been altered) header.i=@kernel.org Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id YkDqBXDduWwD; Thu, 30 Apr 2020 08:45:57 -0400 (EDT) Received: from mm01.cs.columbia.edu (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 13ADC4B447; Thu, 30 Apr 2020 08:45:57 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by mm01.cs.columbia.edu (Postfix) with ESMTP id 315D44B3E0 for ; Thu, 30 Apr 2020 08:45:56 -0400 (EDT) X-Virus-Scanned: at lists.cs.columbia.edu Received: from mm01.cs.columbia.edu ([127.0.0.1]) by localhost (mm01.cs.columbia.edu [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Ch3EAM744v10 for ; Thu, 30 Apr 2020 08:45:55 -0400 (EDT) Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by mm01.cs.columbia.edu (Postfix) with ESMTPS id 078994B3D8 for ; Thu, 30 Apr 2020 08:45:55 -0400 (EDT) Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id B533B205ED; Thu, 30 Apr 2020 12:45:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1588250753; bh=zc3JaJmBWam2fMuCXHmaLPhGYq0c/2wb8dNG5ZJxYjY=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=SrSWBQ6/k8OapgpOLCoiM+liIHO6EsmvBRD1hgViJ9NLdalanmfu1pxx3VHLv7s9X 0Fgeyqk3gQ5TTsfP14dmOfXb8Z3NBuCu4vZReB4UTcU4qp8zLmqWpsLT4/kRktDDWP 9SotYYwXonLcbgVKfayQHS+n5UNfN8pvpT637B6M= Received: from disco-boy.misterjones.org ([51.254.78.96] helo=www.loen.fr) by disco-boy.misterjones.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1jU8Zb-0081rl-W3; Thu, 30 Apr 2020 13:45:52 +0100 MIME-Version: 1.0 Date: Thu, 30 Apr 2020 13:45:51 +0100 From: Marc Zyngier To: Will Deacon Subject: Re: [PATCH] KVM: arm64: Fix 32bit PC wrap-around In-Reply-To: <20200430123104.GB22842@willie-the-truck> References: <20200430101513.318541-1-maz@kernel.org> <20200430102556.GE19932@willie-the-truck> <897baec2a3fad776716bccf3027340fa@kernel.org> <20200430123104.GB22842@willie-the-truck> User-Agent: Roundcube Webmail/1.4.3 Message-ID: <1c0175a09a90d2b7c0243e5bcec7cc9a@kernel.org> X-Sender: maz@kernel.org X-SA-Exim-Connect-IP: 51.254.78.96 X-SA-Exim-Rcpt-To: will@kernel.org, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, james.morse@arm.com, julien.thierry.kdev@gmail.com, suzuki.poulose@arm.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Cc: kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu X-BeenThere: kvmarm@lists.cs.columbia.edu X-Mailman-Version: 2.1.14 Precedence: list List-Id: Where KVM/ARM decisions are made List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu On 2020-04-30 13:31, Will Deacon wrote: > On Thu, Apr 30, 2020 at 11:59:05AM +0100, Marc Zyngier wrote: >> On 2020-04-30 11:25, Will Deacon wrote: >> > On Thu, Apr 30, 2020 at 11:15:13AM +0100, Marc Zyngier wrote: >> > > In the unlikely event that a 32bit vcpu traps into the hypervisor >> > > on an instruction that is located right at the end of the 32bit >> > > range, the emulation of that instruction is going to increment >> > > PC past the 32bit range. This isn't great, as userspace can then >> > > observe this value and get a bit confused. >> > > >> > > Conversly, userspace can do things like (in the context of a 64bit >> > > guest that is capable of 32bit EL0) setting PSTATE to AArch64-EL0, >> > > set PC to a 64bit value, change PSTATE to AArch32-USR, and observe >> > > that PC hasn't been truncated. More confusion. >> > > >> > > Fix both by: >> > > - truncating PC increments for 32bit guests >> > > - sanitize PC every time a core reg is changed by userspace, and >> > > that PSTATE indicates a 32bit mode. >> > >> > It's not clear to me whether this needs a cc stable. What do you think? >> > I >> > suppose that it really depends on how confused e.g. QEMU gets. >> >> It isn't so much QEMU itself that I'm worried about (the emulation >> shouldn't >> really care about the PC), but the likes of GDB. So yes, a cc stable >> seems >> to >> be in order. > > Okey doke. > >> > > Signed-off-by: Marc Zyngier >> > > --- >> > > arch/arm64/kvm/guest.c | 4 ++++ >> > > virt/kvm/arm/hyp/aarch32.c | 8 ++++++-- >> > > 2 files changed, 10 insertions(+), 2 deletions(-) >> > > >> > > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c >> > > index 23ebe51410f0..2a159af82429 100644 >> > > --- a/arch/arm64/kvm/guest.c >> > > +++ b/arch/arm64/kvm/guest.c >> > > @@ -200,6 +200,10 @@ static int set_core_reg(struct kvm_vcpu *vcpu, >> > > const struct kvm_one_reg *reg) >> > > } >> > > >> > > memcpy((u32 *)regs + off, valp, KVM_REG_SIZE(reg->id)); >> > > + >> > > + if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK) >> > > + *vcpu_pc(vcpu) = lower_32_bits(*vcpu_pc(vcpu)); >> > >> > It seems slightly odd to me that we don't enforce this for *all* the >> > registers when running as a 32-bit guest. Couldn't userspace be equally >> > confused by a 64-bit lr or sp? >> >> Fair point. How about this on top, which wipes the upper 32 bits for >> each and every register in the current mode: >> >> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c >> index 2a159af82429..f958c3c7bf65 100644 >> --- a/arch/arm64/kvm/guest.c >> +++ b/arch/arm64/kvm/guest.c >> @@ -201,9 +201,12 @@ static int set_core_reg(struct kvm_vcpu *vcpu, >> const >> struct kvm_one_reg *reg) >> >> memcpy((u32 *)regs + off, valp, KVM_REG_SIZE(reg->id)); >> >> - if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK) >> - *vcpu_pc(vcpu) = lower_32_bits(*vcpu_pc(vcpu)); >> + if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK) { >> + int i; >> >> + for (i = 0; i < 16; i++) >> + *vcpu_reg32(vcpu, i) = (u32)*vcpu_reg32(vcpu, i); > > I think you're missing all the funny banked registers that live all the > way > up to x30 iirc. No, they are all indirected via vcpu_reg32(), which has the magic tables. And the whole point is that we only want to affect the current mode (no point in repainting the FIQ registers if the PSR says USR). Or am I missing something obvious? M. -- Jazz is not dead. It just smells funny... _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE, SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6DDD8C8300D for ; Thu, 30 Apr 2020 12:46:09 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3BF63205ED for ; Thu, 30 Apr 2020 12:46:09 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=lists.infradead.org header.i=@lists.infradead.org header.b="KVGRld7J"; dkim=fail reason="signature verification failed" (1024-bit key) header.d=kernel.org header.i=@kernel.org header.b="SrSWBQ6/" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3BF63205ED Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender:Content-Type: Content-Transfer-Encoding:Cc:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Message-ID:References:In-Reply-To:Subject:To:From: Date:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=wBiNLeykV8oQEhzxo+Rh/Cb0fJBp8y3zzhJngqHxdx4=; b=KVGRld7JVb6Ap84NnrnBloKUh J2ZYpEVkci2FPA0jQ08zigUxKCsQpHKqZLZTV3et+bPmKL/xA5jpljjXf657ePlKdd6t9cRe59H0Z Og7sGyRoORxXoBl3BWCAYWhmSPLLVXXx/m9ab61eypPO2vMhybPoncSuEoESgWtJPzAxMrIqPeMml O2yCBntNfpj9nvoMlKHMmElYcMiYIfJ9AO5NrHVhJL2pTeZK1o+yPR255MQbv71ZMw4Zukya/3lRQ JKiPZIaHJigjze1HkliCZZ2LlLzGHBQYMiLU66baTSY7Qoyf/bxrmwjUZLytgDyTuqDXM83GmmrWn iP3m4/Ezg==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.92.3 #3 (Red Hat Linux)) id 1jU8Zh-0008LQ-IM; Thu, 30 Apr 2020 12:45:57 +0000 Received: from mail.kernel.org ([198.145.29.99]) by bombadil.infradead.org with esmtps (Exim 4.92.3 #3 (Red Hat Linux)) id 1jU8Ze-0008Kh-8L for linux-arm-kernel@lists.infradead.org; Thu, 30 Apr 2020 12:45:55 +0000 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id B533B205ED; Thu, 30 Apr 2020 12:45:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1588250753; bh=zc3JaJmBWam2fMuCXHmaLPhGYq0c/2wb8dNG5ZJxYjY=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=SrSWBQ6/k8OapgpOLCoiM+liIHO6EsmvBRD1hgViJ9NLdalanmfu1pxx3VHLv7s9X 0Fgeyqk3gQ5TTsfP14dmOfXb8Z3NBuCu4vZReB4UTcU4qp8zLmqWpsLT4/kRktDDWP 9SotYYwXonLcbgVKfayQHS+n5UNfN8pvpT637B6M= Received: from disco-boy.misterjones.org ([51.254.78.96] helo=www.loen.fr) by disco-boy.misterjones.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1jU8Zb-0081rl-W3; Thu, 30 Apr 2020 13:45:52 +0100 MIME-Version: 1.0 Date: Thu, 30 Apr 2020 13:45:51 +0100 From: Marc Zyngier To: Will Deacon Subject: Re: [PATCH] KVM: arm64: Fix 32bit PC wrap-around In-Reply-To: <20200430123104.GB22842@willie-the-truck> References: <20200430101513.318541-1-maz@kernel.org> <20200430102556.GE19932@willie-the-truck> <897baec2a3fad776716bccf3027340fa@kernel.org> <20200430123104.GB22842@willie-the-truck> User-Agent: Roundcube Webmail/1.4.3 Message-ID: <1c0175a09a90d2b7c0243e5bcec7cc9a@kernel.org> X-Sender: maz@kernel.org X-SA-Exim-Connect-IP: 51.254.78.96 X-SA-Exim-Rcpt-To: will@kernel.org, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, james.morse@arm.com, julien.thierry.kdev@gmail.com, suzuki.poulose@arm.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20200430_054554_361683_67A31E21 X-CRM114-Status: GOOD ( 24.77 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: kvm@vger.kernel.org, Suzuki K Poulose , James Morse , linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, Julien Thierry Content-Transfer-Encoding: 7bit Content-Type: text/plain; charset="us-ascii"; Format="flowed" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+infradead-linux-arm-kernel=archiver.kernel.org@lists.infradead.org On 2020-04-30 13:31, Will Deacon wrote: > On Thu, Apr 30, 2020 at 11:59:05AM +0100, Marc Zyngier wrote: >> On 2020-04-30 11:25, Will Deacon wrote: >> > On Thu, Apr 30, 2020 at 11:15:13AM +0100, Marc Zyngier wrote: >> > > In the unlikely event that a 32bit vcpu traps into the hypervisor >> > > on an instruction that is located right at the end of the 32bit >> > > range, the emulation of that instruction is going to increment >> > > PC past the 32bit range. This isn't great, as userspace can then >> > > observe this value and get a bit confused. >> > > >> > > Conversly, userspace can do things like (in the context of a 64bit >> > > guest that is capable of 32bit EL0) setting PSTATE to AArch64-EL0, >> > > set PC to a 64bit value, change PSTATE to AArch32-USR, and observe >> > > that PC hasn't been truncated. More confusion. >> > > >> > > Fix both by: >> > > - truncating PC increments for 32bit guests >> > > - sanitize PC every time a core reg is changed by userspace, and >> > > that PSTATE indicates a 32bit mode. >> > >> > It's not clear to me whether this needs a cc stable. What do you think? >> > I >> > suppose that it really depends on how confused e.g. QEMU gets. >> >> It isn't so much QEMU itself that I'm worried about (the emulation >> shouldn't >> really care about the PC), but the likes of GDB. So yes, a cc stable >> seems >> to >> be in order. > > Okey doke. > >> > > Signed-off-by: Marc Zyngier >> > > --- >> > > arch/arm64/kvm/guest.c | 4 ++++ >> > > virt/kvm/arm/hyp/aarch32.c | 8 ++++++-- >> > > 2 files changed, 10 insertions(+), 2 deletions(-) >> > > >> > > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c >> > > index 23ebe51410f0..2a159af82429 100644 >> > > --- a/arch/arm64/kvm/guest.c >> > > +++ b/arch/arm64/kvm/guest.c >> > > @@ -200,6 +200,10 @@ static int set_core_reg(struct kvm_vcpu *vcpu, >> > > const struct kvm_one_reg *reg) >> > > } >> > > >> > > memcpy((u32 *)regs + off, valp, KVM_REG_SIZE(reg->id)); >> > > + >> > > + if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK) >> > > + *vcpu_pc(vcpu) = lower_32_bits(*vcpu_pc(vcpu)); >> > >> > It seems slightly odd to me that we don't enforce this for *all* the >> > registers when running as a 32-bit guest. Couldn't userspace be equally >> > confused by a 64-bit lr or sp? >> >> Fair point. How about this on top, which wipes the upper 32 bits for >> each and every register in the current mode: >> >> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c >> index 2a159af82429..f958c3c7bf65 100644 >> --- a/arch/arm64/kvm/guest.c >> +++ b/arch/arm64/kvm/guest.c >> @@ -201,9 +201,12 @@ static int set_core_reg(struct kvm_vcpu *vcpu, >> const >> struct kvm_one_reg *reg) >> >> memcpy((u32 *)regs + off, valp, KVM_REG_SIZE(reg->id)); >> >> - if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK) >> - *vcpu_pc(vcpu) = lower_32_bits(*vcpu_pc(vcpu)); >> + if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK) { >> + int i; >> >> + for (i = 0; i < 16; i++) >> + *vcpu_reg32(vcpu, i) = (u32)*vcpu_reg32(vcpu, i); > > I think you're missing all the funny banked registers that live all the > way > up to x30 iirc. No, they are all indirected via vcpu_reg32(), which has the magic tables. And the whole point is that we only want to affect the current mode (no point in repainting the FIQ registers if the PSR says USR). Or am I missing something obvious? M. -- Jazz is not dead. It just smells funny... _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.1 required=3.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY, SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2505CC8300B for ; Thu, 30 Apr 2020 12:45:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id F38992078D for ; Thu, 30 Apr 2020 12:45:55 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1588250756; bh=zc3JaJmBWam2fMuCXHmaLPhGYq0c/2wb8dNG5ZJxYjY=; h=Date:From:To:Cc:Subject:In-Reply-To:References:List-ID:From; b=ZxWrd1jJyjNYfhU9mYt8yLmrEaU9bzsKjBa85NhXa+zKFNRqLTrmpdjFqSwKIZguA ku4nKfZxoetMrmeXBrFs3uT2tQ/rJpnertxrvfPar25ieDxr0ggc/IwztDiEeVZuTh cU2MlP3ly64812LGjCAVlK39Bgy7cx7dx8KtZZ8I= Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726900AbgD3Mpz (ORCPT ); Thu, 30 Apr 2020 08:45:55 -0400 Received: from mail.kernel.org ([198.145.29.99]:36444 "EHLO mail.kernel.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726808AbgD3Mpy (ORCPT ); Thu, 30 Apr 2020 08:45:54 -0400 Received: from disco-boy.misterjones.org (disco-boy.misterjones.org [51.254.78.96]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id B533B205ED; Thu, 30 Apr 2020 12:45:53 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=default; t=1588250753; bh=zc3JaJmBWam2fMuCXHmaLPhGYq0c/2wb8dNG5ZJxYjY=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=SrSWBQ6/k8OapgpOLCoiM+liIHO6EsmvBRD1hgViJ9NLdalanmfu1pxx3VHLv7s9X 0Fgeyqk3gQ5TTsfP14dmOfXb8Z3NBuCu4vZReB4UTcU4qp8zLmqWpsLT4/kRktDDWP 9SotYYwXonLcbgVKfayQHS+n5UNfN8pvpT637B6M= Received: from disco-boy.misterjones.org ([51.254.78.96] helo=www.loen.fr) by disco-boy.misterjones.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.92) (envelope-from ) id 1jU8Zb-0081rl-W3; Thu, 30 Apr 2020 13:45:52 +0100 MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Thu, 30 Apr 2020 13:45:51 +0100 From: Marc Zyngier To: Will Deacon Cc: linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, James Morse , Julien Thierry , Suzuki K Poulose Subject: Re: [PATCH] KVM: arm64: Fix 32bit PC wrap-around In-Reply-To: <20200430123104.GB22842@willie-the-truck> References: <20200430101513.318541-1-maz@kernel.org> <20200430102556.GE19932@willie-the-truck> <897baec2a3fad776716bccf3027340fa@kernel.org> <20200430123104.GB22842@willie-the-truck> User-Agent: Roundcube Webmail/1.4.3 Message-ID: <1c0175a09a90d2b7c0243e5bcec7cc9a@kernel.org> X-Sender: maz@kernel.org X-SA-Exim-Connect-IP: 51.254.78.96 X-SA-Exim-Rcpt-To: will@kernel.org, linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, kvmarm@lists.cs.columbia.edu, james.morse@arm.com, julien.thierry.kdev@gmail.com, suzuki.poulose@arm.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org On 2020-04-30 13:31, Will Deacon wrote: > On Thu, Apr 30, 2020 at 11:59:05AM +0100, Marc Zyngier wrote: >> On 2020-04-30 11:25, Will Deacon wrote: >> > On Thu, Apr 30, 2020 at 11:15:13AM +0100, Marc Zyngier wrote: >> > > In the unlikely event that a 32bit vcpu traps into the hypervisor >> > > on an instruction that is located right at the end of the 32bit >> > > range, the emulation of that instruction is going to increment >> > > PC past the 32bit range. This isn't great, as userspace can then >> > > observe this value and get a bit confused. >> > > >> > > Conversly, userspace can do things like (in the context of a 64bit >> > > guest that is capable of 32bit EL0) setting PSTATE to AArch64-EL0, >> > > set PC to a 64bit value, change PSTATE to AArch32-USR, and observe >> > > that PC hasn't been truncated. More confusion. >> > > >> > > Fix both by: >> > > - truncating PC increments for 32bit guests >> > > - sanitize PC every time a core reg is changed by userspace, and >> > > that PSTATE indicates a 32bit mode. >> > >> > It's not clear to me whether this needs a cc stable. What do you think? >> > I >> > suppose that it really depends on how confused e.g. QEMU gets. >> >> It isn't so much QEMU itself that I'm worried about (the emulation >> shouldn't >> really care about the PC), but the likes of GDB. So yes, a cc stable >> seems >> to >> be in order. > > Okey doke. > >> > > Signed-off-by: Marc Zyngier >> > > --- >> > > arch/arm64/kvm/guest.c | 4 ++++ >> > > virt/kvm/arm/hyp/aarch32.c | 8 ++++++-- >> > > 2 files changed, 10 insertions(+), 2 deletions(-) >> > > >> > > diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c >> > > index 23ebe51410f0..2a159af82429 100644 >> > > --- a/arch/arm64/kvm/guest.c >> > > +++ b/arch/arm64/kvm/guest.c >> > > @@ -200,6 +200,10 @@ static int set_core_reg(struct kvm_vcpu *vcpu, >> > > const struct kvm_one_reg *reg) >> > > } >> > > >> > > memcpy((u32 *)regs + off, valp, KVM_REG_SIZE(reg->id)); >> > > + >> > > + if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK) >> > > + *vcpu_pc(vcpu) = lower_32_bits(*vcpu_pc(vcpu)); >> > >> > It seems slightly odd to me that we don't enforce this for *all* the >> > registers when running as a 32-bit guest. Couldn't userspace be equally >> > confused by a 64-bit lr or sp? >> >> Fair point. How about this on top, which wipes the upper 32 bits for >> each and every register in the current mode: >> >> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c >> index 2a159af82429..f958c3c7bf65 100644 >> --- a/arch/arm64/kvm/guest.c >> +++ b/arch/arm64/kvm/guest.c >> @@ -201,9 +201,12 @@ static int set_core_reg(struct kvm_vcpu *vcpu, >> const >> struct kvm_one_reg *reg) >> >> memcpy((u32 *)regs + off, valp, KVM_REG_SIZE(reg->id)); >> >> - if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK) >> - *vcpu_pc(vcpu) = lower_32_bits(*vcpu_pc(vcpu)); >> + if (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK) { >> + int i; >> >> + for (i = 0; i < 16; i++) >> + *vcpu_reg32(vcpu, i) = (u32)*vcpu_reg32(vcpu, i); > > I think you're missing all the funny banked registers that live all the > way > up to x30 iirc. No, they are all indirected via vcpu_reg32(), which has the magic tables. And the whole point is that we only want to affect the current mode (no point in repainting the FIQ registers if the PSR says USR). Or am I missing something obvious? M. -- Jazz is not dead. It just smells funny...