[v4.4,backport] arm64: KVM: fix VTTBR_BADDR_MASK BUG_ON off-by-one

Message ID 20171212125032.6292-2-christoffer.dall@linaro.org
State New
Headers show
Series
  • [v4.4,backport] arm64: KVM: fix VTTBR_BADDR_MASK BUG_ON off-by-one
Related show

Commit Message

Christoffer Dall Dec. 12, 2017, 12:50 p.m.
From: Kristina Martsenko <kristina.martsenko@arm.com>


Commit 26aa7b3b1c0fb3f1a6176a0c1847204ef4355693 upstream.

VTTBR_BADDR_MASK is used to sanity check the size and alignment of the
VTTBR address. It seems to currently be off by one, thereby only
allowing up to 47-bit addresses (instead of 48-bit) and also
insufficiently checking the alignment. This patch fixes it.

As an example, with 4k pages, before this patch we have:

  PHYS_MASK_SHIFT = 48
  VTTBR_X = 37 - 24 = 13
  VTTBR_BADDR_SHIFT = 13 - 1 = 12
  VTTBR_BADDR_MASK = ((1 << 35) - 1) << 12 = 0x00007ffffffff000

Which is wrong, because the mask doesn't allow bit 47 of the VTTBR
address to be set, and only requires the address to be 12-bit (4k)
aligned, while it actually needs to be 13-bit (8k) aligned because we
concatenate two 4k tables.

With this patch, the mask becomes 0x0000ffffffffe000, which is what we
want.

Fixes: 0369f6a34b9f ("arm64: KVM: EL2 register definitions")
Cc: <stable@vger.kernel.org> # 3.11.x
Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>

Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com>

Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>

---
 arch/arm64/include/asm/kvm_arm.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

-- 
2.14.2

Comments

Kristina Martsenko Dec. 12, 2017, 4:35 p.m. | #1
Hi Christoffer,

On 12/12/17 12:50, Christoffer Dall wrote:
> From: Kristina Martsenko <kristina.martsenko@arm.com>

> 

> Commit 26aa7b3b1c0fb3f1a6176a0c1847204ef4355693 upstream.

> 

> VTTBR_BADDR_MASK is used to sanity check the size and alignment of the

> VTTBR address. It seems to currently be off by one, thereby only

> allowing up to 47-bit addresses (instead of 48-bit) and also

> insufficiently checking the alignment. This patch fixes it.

> 

> As an example, with 4k pages, before this patch we have:

> 

>   PHYS_MASK_SHIFT = 48

>   VTTBR_X = 37 - 24 = 13

>   VTTBR_BADDR_SHIFT = 13 - 1 = 12

>   VTTBR_BADDR_MASK = ((1 << 35) - 1) << 12 = 0x00007ffffffff000

> 

> Which is wrong, because the mask doesn't allow bit 47 of the VTTBR

> address to be set, and only requires the address to be 12-bit (4k)

> aligned, while it actually needs to be 13-bit (8k) aligned because we

> concatenate two 4k tables.

> 

> With this patch, the mask becomes 0x0000ffffffffe000, which is what we

> want.

> 

> Fixes: 0369f6a34b9f ("arm64: KVM: EL2 register definitions")

> Cc: <stable@vger.kernel.org> # 3.11.x

> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

> Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>

> Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com>

> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>

> ---

>  arch/arm64/include/asm/kvm_arm.h | 3 +--

>  1 file changed, 1 insertion(+), 2 deletions(-)


Not sure if I'm missing something, but it seems like this patch (and the
v3.18 one) were already applied to the stable trees. It's two of the
arch/arm/ patches that failed to apply, not the arch/arm64/ ones.

Kristina

> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h

> index 2d960f8588b0..ef8e13d379cb 100644

> --- a/arch/arm64/include/asm/kvm_arm.h

> +++ b/arch/arm64/include/asm/kvm_arm.h

> @@ -164,8 +164,7 @@

>  #define VTTBR_X		(37 - VTCR_EL2_T0SZ_40B)

>  #endif

>  

> -#define VTTBR_BADDR_SHIFT (VTTBR_X - 1)

> -#define VTTBR_BADDR_MASK  (((UL(1) << (PHYS_MASK_SHIFT - VTTBR_X)) - 1) << VTTBR_BADDR_SHIFT)

> +#define VTTBR_BADDR_MASK  (((UL(1) << (PHYS_MASK_SHIFT - VTTBR_X)) - 1) << VTTBR_X)

>  #define VTTBR_VMID_SHIFT  (UL(48))

>  #define VTTBR_VMID_MASK	  (UL(0xFF) << VTTBR_VMID_SHIFT)

>  

>
Christoffer Dall Dec. 12, 2017, 4:49 p.m. | #2
On Tue, Dec 12, 2017 at 5:35 PM, Kristina Martsenko
<kristina.martsenko@arm.com> wrote:
> Hi Christoffer,

>

> On 12/12/17 12:50, Christoffer Dall wrote:

>> From: Kristina Martsenko <kristina.martsenko@arm.com>

>>

>> Commit 26aa7b3b1c0fb3f1a6176a0c1847204ef4355693 upstream.

>>

>> VTTBR_BADDR_MASK is used to sanity check the size and alignment of the

>> VTTBR address. It seems to currently be off by one, thereby only

>> allowing up to 47-bit addresses (instead of 48-bit) and also

>> insufficiently checking the alignment. This patch fixes it.

>>

>> As an example, with 4k pages, before this patch we have:

>>

>>   PHYS_MASK_SHIFT = 48

>>   VTTBR_X = 37 - 24 = 13

>>   VTTBR_BADDR_SHIFT = 13 - 1 = 12

>>   VTTBR_BADDR_MASK = ((1 << 35) - 1) << 12 = 0x00007ffffffff000

>>

>> Which is wrong, because the mask doesn't allow bit 47 of the VTTBR

>> address to be set, and only requires the address to be 12-bit (4k)

>> aligned, while it actually needs to be 13-bit (8k) aligned because we

>> concatenate two 4k tables.

>>

>> With this patch, the mask becomes 0x0000ffffffffe000, which is what we

>> want.

>>

>> Fixes: 0369f6a34b9f ("arm64: KVM: EL2 register definitions")

>> Cc: <stable@vger.kernel.org> # 3.11.x

>> Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

>> Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>

>> Signed-off-by: Kristina Martsenko <kristina.martsenko@arm.com>

>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com>

>> Signed-off-by: Christoffer Dall <christoffer.dall@linaro.org>

>> ---

>>  arch/arm64/include/asm/kvm_arm.h | 3 +--

>>  1 file changed, 1 insertion(+), 2 deletions(-)

>

> Not sure if I'm missing something, but it seems like this patch (and the

> v3.18 one) were already applied to the stable trees. It's two of the

> arch/arm/ patches that failed to apply, not the arch/arm64/ ones.

>


You're not missing anything - I accidentally copied the commit id from
the e-mail that told me it successfully applied the patch, duh.

I'll fix the other one.

-Christoffer

Patch

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 2d960f8588b0..ef8e13d379cb 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -164,8 +164,7 @@ 
 #define VTTBR_X		(37 - VTCR_EL2_T0SZ_40B)
 #endif
 
-#define VTTBR_BADDR_SHIFT (VTTBR_X - 1)
-#define VTTBR_BADDR_MASK  (((UL(1) << (PHYS_MASK_SHIFT - VTTBR_X)) - 1) << VTTBR_BADDR_SHIFT)
+#define VTTBR_BADDR_MASK  (((UL(1) << (PHYS_MASK_SHIFT - VTTBR_X)) - 1) << VTTBR_X)
 #define VTTBR_VMID_SHIFT  (UL(48))
 #define VTTBR_VMID_MASK	  (UL(0xFF) << VTTBR_VMID_SHIFT)