diff mbox series

[1/3] KVM: arm64: Fix AArch32 register narrowing on userspace write

Message ID 20240524141956.1450304-2-maz@kernel.org (mailing list archive)
State New
Headers show
Series KVM/arm64 fixes for AArch32 handling | expand

Commit Message

Marc Zyngier May 24, 2024, 2:19 p.m. UTC
When userspace writes to once of the core registers, we make
sure to narrow the corresponding GPRs if PSTATE indicates
an AArch32 context.

The code tries to check whether the context is EL0 or EL1 so
that it narrows the correct registers. But it does so by checking
the full PSTATE instead of PSTATE.M.

As a consequence, and if we are restoring an AArch32 EL0 context
in a 64bit guest, and that PSTATE has *any* bit set outside of
PSTATE.M, we narrow *all* registers instead of only the first 15,
destroying the 64bit state.

Obviously, this is not something the guest is likely to enjoy.

Correctly masking PSTATE to only evaluate PSTATE.M fixes it.

Fixes: 90c1f934ed71 ("KVM: arm64: Get rid of the AArch32 register mapping code")
Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Cc: stable@vger.kernel.org
---
 arch/arm64/kvm/guest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Nina Schoetterl-Glausch May 24, 2024, 5:18 p.m. UTC | #1
On Fri, 2024-05-24 at 15:19 +0100, Marc Zyngier wrote:
> When userspace writes to once of the core registers, we make
> sure to narrow the corresponding GPRs if PSTATE indicates
> an AArch32 context.
> 
> The code tries to check whether the context is EL0 or EL1 so
> that it narrows the correct registers. But it does so by checking
> the full PSTATE instead of PSTATE.M.
> 
> As a consequence, and if we are restoring an AArch32 EL0 context
> in a 64bit guest, and that PSTATE has *any* bit set outside of
> PSTATE.M, we narrow *all* registers instead of only the first 15,
> destroying the 64bit state.
> 
> Obviously, this is not something the guest is likely to enjoy.
> 
> Correctly masking PSTATE to only evaluate PSTATE.M fixes it.
> 
> Fixes: 90c1f934ed71 ("KVM: arm64: Get rid of the AArch32 register mapping code")
> Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Cc: stable@vger.kernel.org
> ---
>  arch/arm64/kvm/guest.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index e2f762d959bb..d9617b11f7a8 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -276,7 +276,7 @@ static int set_core_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
>  	if (*vcpu_cpsr(vcpu) & PSR_MODE32_BIT) {
>  		int i, nr_reg;
>  
> -		switch (*vcpu_cpsr(vcpu)) {
> +		switch (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK) {
>  		/*
>  		 * Either we are dealing with user mode, and only the
>  		 * first 15 registers (+ PC) must be narrowed to 32bit.

Reviewed-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
Oliver Upton May 24, 2024, 7:11 p.m. UTC | #2
On Fri, May 24, 2024 at 03:19:54PM +0100, Marc Zyngier wrote:
> When userspace writes to once of the core registers, we make

s/once/one/

> sure to narrow the corresponding GPRs if PSTATE indicates
> an AArch32 context.
> 
> The code tries to check whether the context is EL0 or EL1 so
> that it narrows the correct registers. But it does so by checking
> the full PSTATE instead of PSTATE.M.
> 
> As a consequence, and if we are restoring an AArch32 EL0 context
> in a 64bit guest, and that PSTATE has *any* bit set outside of
> PSTATE.M, we narrow *all* registers instead of only the first 15,
> destroying the 64bit state.
> 
> Obviously, this is not something the guest is likely to enjoy.
> 
> Correctly masking PSTATE to only evaluate PSTATE.M fixes it.
> 
> Fixes: 90c1f934ed71 ("KVM: arm64: Get rid of the AArch32 register mapping code")
> Reported-by: Nina Schoetterl-Glausch <nsg@linux.ibm.com>
> Signed-off-by: Marc Zyngier <maz@kernel.org>
> Cc: stable@vger.kernel.org
> ---
>  arch/arm64/kvm/guest.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
> index e2f762d959bb..d9617b11f7a8 100644
> --- a/arch/arm64/kvm/guest.c
> +++ b/arch/arm64/kvm/guest.c
> @@ -276,7 +276,7 @@ static int set_core_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
>  	if (*vcpu_cpsr(vcpu) & PSR_MODE32_BIT) {
>  		int i, nr_reg;
>  
> -		switch (*vcpu_cpsr(vcpu)) {
> +		switch (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK) {
>  		/*
>  		 * Either we are dealing with user mode, and only the
>  		 * first 15 registers (+ PC) must be narrowed to 32bit.
> -- 
> 2.39.2
>
diff mbox series

Patch

diff --git a/arch/arm64/kvm/guest.c b/arch/arm64/kvm/guest.c
index e2f762d959bb..d9617b11f7a8 100644
--- a/arch/arm64/kvm/guest.c
+++ b/arch/arm64/kvm/guest.c
@@ -276,7 +276,7 @@  static int set_core_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
 	if (*vcpu_cpsr(vcpu) & PSR_MODE32_BIT) {
 		int i, nr_reg;
 
-		switch (*vcpu_cpsr(vcpu)) {
+		switch (*vcpu_cpsr(vcpu) & PSR_AA32_MODE_MASK) {
 		/*
 		 * Either we are dealing with user mode, and only the
 		 * first 15 registers (+ PC) must be narrowed to 32bit.