[PATCH] KVM: Array access out of bounds

[PATCH] KVM: Array access out of bounds

[Liu Jing]

In the kvmppc_mmu_book3s_64_xlate function,
r = be64_to_cpu(pteg[i+1]); i used is 16 after the last loop and adding 1 will cross the line.

Signed-off-by: Liu Jing <liujing@cmss.chinamobile.com>

diff --git a/arch/powerpc/kvm/book3s_64_mmu.c b/arch/powerpc/kvm/book3s_64_mmu.c
index 61290282fd9e..75d2b284c4b4 100644
--- a/arch/powerpc/kvm/book3s_64_mmu.c
+++ b/arch/powerpc/kvm/book3s_64_mmu.c
@@ -284,11 +284,16 @@ static int kvmppc_mmu_book3s_64_xlate(struct kvm_vcpu *vcpu, gva_t eaddr,
 		second = true;
 		goto do_second;
 	}
+	if (i < 14) {
+		r = be64_to_cpu(pteg[i+1]);
+		pp = (r & HPTE_R_PP) | key;
+		if (r & HPTE_R_PP0)
+			pp |= 8;
+	} else {
+		dprintk("KVM: Index out of bounds!\n");
+		goto no_page_found;
+	}
 
-	r = be64_to_cpu(pteg[i+1]);
-	pp = (r & HPTE_R_PP) | key;
-	if (r & HPTE_R_PP0)
-		pp |= 8;
 
 	gpte->eaddr = eaddr;
 	gpte->vpage = kvmppc_mmu_book3s_64_ea_to_vp(vcpu, eaddr, data);
-- 
2.27.0

Re: [PATCH] KVM: Array access out of bounds

[Paolo Bonzini]

On 10/23/24 14:01, Liu Jing wrote:
> In the kvmppc_mmu_book3s_64_xlate function,
> r = be64_to_cpu(pteg[i+1]); i used is 16 after the last loop and adding 1 will cross the line.
> 
> Signed-off-by: Liu Jing <liujing@cmss.chinamobile.com>
> 
> diff --git a/arch/powerpc/kvm/book3s_64_mmu.c b/arch/powerpc/kvm/book3s_64_mmu.c
> index 61290282fd9e..75d2b284c4b4 100644
> --- a/arch/powerpc/kvm/book3s_64_mmu.c
> +++ b/arch/powerpc/kvm/book3s_64_mmu.c
> @@ -284,11 +284,16 @@ static int kvmppc_mmu_book3s_64_xlate(struct kvm_vcpu *vcpu, gva_t eaddr,
>   		second = true;
>   		goto do_second;
>   	}
> +	if (i < 14) {

This should be i <= 14 (not "<").  And in fact, if you get here you must 
have found == true, and therefore i is indeed <= 14.  The code right 
above is this:

         if (!found) {
                 if (second)
                         goto no_page_found;
                 v_val |= HPTE_V_SECONDARY;
                 second = true;
                 goto do_second;
         }

and  "found = true" is set just before a break statement.

Paolo

> +		r = be64_to_cpu(pteg[i+1]);
> +		pp = (r & HPTE_R_PP) | key;
> +		if (r & HPTE_R_PP0)
> +			pp |= 8;
> +	} else {
> +		dprintk("KVM: Index out of bounds!\n");
> +		goto no_page_found;
> +	}
>   
> -	r = be64_to_cpu(pteg[i+1]);
> -	pp = (r & HPTE_R_PP) | key;
> -	if (r & HPTE_R_PP0)
> -		pp |= 8;
>   
>   	gpte->eaddr = eaddr;
>   	gpte->vpage = kvmppc_mmu_book3s_64_ea_to_vp(vcpu, eaddr, data);