[PATCH] KVM: arm64: Fix misuse of KVM_ARM_VCPU_POWER_OFF bit index

[PATCH] KVM: arm64: Fix misuse of KVM_ARM_VCPU_POWER_OFF bit index

[Oliver Upton]

KVM_ARM_VCPU_POWER_OFF is as bit index, _not_ a literal bitmask.
Nonetheless, commit e3c1c0cae31e ("KVM: arm64: Relax invariance
of KVM_ARM_VCPU_POWER_OFF") started using it that way, meaning that
powering off a vCPU with the KVM_ARM_VCPU_INIT ioctl is completely
broken.

Fix it by using a shifted bit for the bitwise operations instead.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Fixes: e3c1c0cae31e ("KVM: arm64: Relax invariance of KVM_ARM_VCPU_POWER_OFF")
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/arm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 80ca7ec269dc..7d20bfd77e1c 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1250,8 +1250,8 @@ static int kvm_arch_vcpu_ioctl_vcpu_init(struct kvm_vcpu *vcpu,
 	 * reflecting it in the finalized feature set, thus limiting its scope
 	 * to a single KVM_ARM_VCPU_INIT call.
 	 */
-	if (init->features[0] & KVM_ARM_VCPU_POWER_OFF) {
-		init->features[0] &= ~KVM_ARM_VCPU_POWER_OFF;
+	if (init->features[0] & (1UL << KVM_ARM_VCPU_POWER_OFF)) {
+		init->features[0] &= ~(1UL << KVM_ARM_VCPU_POWER_OFF);
 		power_off = true;
 	}
 

base-commit: 686672407e6eaf8c874d4a6bf315da798f281045
-- 
2.41.0.185.g7c58973941-goog

Re: [PATCH] KVM: arm64: Fix misuse of KVM_ARM_VCPU_POWER_OFF bit index

[Marc Zyngier]

On 2023-06-22 17:09, Oliver Upton wrote:
> KVM_ARM_VCPU_POWER_OFF is as bit index, _not_ a literal bitmask.
> Nonetheless, commit e3c1c0cae31e ("KVM: arm64: Relax invariance
> of KVM_ARM_VCPU_POWER_OFF") started using it that way, meaning that
> powering off a vCPU with the KVM_ARM_VCPU_INIT ioctl is completely
> broken.
> 
> Fix it by using a shifted bit for the bitwise operations instead.
> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Fixes: e3c1c0cae31e ("KVM: arm64: Relax invariance of 
> KVM_ARM_VCPU_POWER_OFF")
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  arch/arm64/kvm/arm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 80ca7ec269dc..7d20bfd77e1c 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -1250,8 +1250,8 @@ static int kvm_arch_vcpu_ioctl_vcpu_init(struct
> kvm_vcpu *vcpu,
>  	 * reflecting it in the finalized feature set, thus limiting its 
> scope
>  	 * to a single KVM_ARM_VCPU_INIT call.
>  	 */
> -	if (init->features[0] & KVM_ARM_VCPU_POWER_OFF) {
> -		init->features[0] &= ~KVM_ARM_VCPU_POWER_OFF;
> +	if (init->features[0] & (1UL << KVM_ARM_VCPU_POWER_OFF)) {
> +		init->features[0] &= ~(1UL << KVM_ARM_VCPU_POWER_OFF);
>  		power_off = true;
>  	}

Huhuh. Maybe use BIT() instead? Otherwise:

Acked-by: Marc Zyngier <maz@kernel.org>

         M.
-- 
Jazz is not dead. It just smells funny...

Re: [PATCH] KVM: arm64: Fix misuse of KVM_ARM_VCPU_POWER_OFF bit index

[Oliver Upton]

On Thu, 22 Jun 2023 16:09:22 +0000, Oliver Upton wrote:
> KVM_ARM_VCPU_POWER_OFF is as bit index, _not_ a literal bitmask.
> Nonetheless, commit e3c1c0cae31e ("KVM: arm64: Relax invariance
> of KVM_ARM_VCPU_POWER_OFF") started using it that way, meaning that
> powering off a vCPU with the KVM_ARM_VCPU_INIT ioctl is completely
> broken.
> 
> Fix it by using a shifted bit for the bitwise operations instead.
> 
> [...]

Applied, w/ Marc's suggestion to just use BIT() instead.

[1/1] KVM: arm64: Fix misuse of KVM_ARM_VCPU_POWER_OFF bit index
      https://git.kernel.org/kvmarm/kvmarm/c/192df2aa0113

--
Best,
Oliver