arm64: KVM: define ESR_ELx_EC_* constants as UL

[PATCH v2] arm64: KVM: define ESR_ELx_EC_* constants as UL

Posted by Anastasia Belova 2 months, 2 weeks ago

Add explicit casting to prevent expantion of 32th bit of
u32 into highest half of u64 in several places.

For example, in inject_abt64:
ESR_ELx_EC_DABT_LOW << ESR_ELx_EC_SHIFT = 0x24 << 26.
This operation's result is int with 1 in 32th bit.
While casting this value into u64 (esr is u64) 1
fills 32 highest bits.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Fixes: aa8eff9bfbd5 ("arm64: KVM: fault injection into a guest")
Signed-off-by: Anastasia Belova <abelova@astralinux.ru>
---
v2: move casting from usage to definition
 arch/arm64/include/asm/esr.h | 88 ++++++++++++++++++------------------
 1 file changed, 44 insertions(+), 44 deletions(-)

diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index 56c148890daf..2f3d56857a97 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -10,63 +10,63 @@
 #include <asm/memory.h>
 #include <asm/sysreg.h>
 
-#define ESR_ELx_EC_UNKNOWN	(0x00)
-#define ESR_ELx_EC_WFx		(0x01)
+#define ESR_ELx_EC_UNKNOWN	UL(0x00)
+#define ESR_ELx_EC_WFx		UL(0x01)
 /* Unallocated EC: 0x02 */
-#define ESR_ELx_EC_CP15_32	(0x03)
-#define ESR_ELx_EC_CP15_64	(0x04)
-#define ESR_ELx_EC_CP14_MR	(0x05)
-#define ESR_ELx_EC_CP14_LS	(0x06)
-#define ESR_ELx_EC_FP_ASIMD	(0x07)
-#define ESR_ELx_EC_CP10_ID	(0x08)	/* EL2 only */
-#define ESR_ELx_EC_PAC		(0x09)	/* EL2 and above */
+#define ESR_ELx_EC_CP15_32	UL(0x03)
+#define ESR_ELx_EC_CP15_64	UL(0x04)
+#define ESR_ELx_EC_CP14_MR	UL(0x05)
+#define ESR_ELx_EC_CP14_LS	UL(0x06)
+#define ESR_ELx_EC_FP_ASIMD	UL(0x07)
+#define ESR_ELx_EC_CP10_ID	UL(0x08)	/* EL2 only */
+#define ESR_ELx_EC_PAC		UL(0x09)	/* EL2 and above */
 /* Unallocated EC: 0x0A - 0x0B */
-#define ESR_ELx_EC_CP14_64	(0x0C)
-#define ESR_ELx_EC_BTI		(0x0D)
-#define ESR_ELx_EC_ILL		(0x0E)
+#define ESR_ELx_EC_CP14_64	UL(0x0C)
+#define ESR_ELx_EC_BTI		UL(0x0D)
+#define ESR_ELx_EC_ILL		UL(0x0E)
 /* Unallocated EC: 0x0F - 0x10 */
-#define ESR_ELx_EC_SVC32	(0x11)
-#define ESR_ELx_EC_HVC32	(0x12)	/* EL2 only */
-#define ESR_ELx_EC_SMC32	(0x13)	/* EL2 and above */
+#define ESR_ELx_EC_SVC32	UL(0x11)
+#define ESR_ELx_EC_HVC32	UL(0x12)	/* EL2 only */
+#define ESR_ELx_EC_SMC32	UL(0x13)	/* EL2 and above */
 /* Unallocated EC: 0x14 */
-#define ESR_ELx_EC_SVC64	(0x15)
-#define ESR_ELx_EC_HVC64	(0x16)	/* EL2 and above */
-#define ESR_ELx_EC_SMC64	(0x17)	/* EL2 and above */
-#define ESR_ELx_EC_SYS64	(0x18)
-#define ESR_ELx_EC_SVE		(0x19)
-#define ESR_ELx_EC_ERET		(0x1a)	/* EL2 only */
+#define ESR_ELx_EC_SVC64	UL(0x15)
+#define ESR_ELx_EC_HVC64	UL(0x16)	/* EL2 and above */
+#define ESR_ELx_EC_SMC64	UL(0x17)	/* EL2 and above */
+#define ESR_ELx_EC_SYS64	UL(0x18)
+#define ESR_ELx_EC_SVE		UL(0x19)
+#define ESR_ELx_EC_ERET		UL(0x1a)	/* EL2 only */
 /* Unallocated EC: 0x1B */
-#define ESR_ELx_EC_FPAC		(0x1C)	/* EL1 and above */
-#define ESR_ELx_EC_SME		(0x1D)
+#define ESR_ELx_EC_FPAC		UL(0x1C)	/* EL1 and above */
+#define ESR_ELx_EC_SME		UL(0x1D)
 /* Unallocated EC: 0x1E */
-#define ESR_ELx_EC_IMP_DEF	(0x1f)	/* EL3 only */
-#define ESR_ELx_EC_IABT_LOW	(0x20)
-#define ESR_ELx_EC_IABT_CUR	(0x21)
-#define ESR_ELx_EC_PC_ALIGN	(0x22)
+#define ESR_ELx_EC_IMP_DEF	UL(0x1f)	/* EL3 only */
+#define ESR_ELx_EC_IABT_LOW	UL(0x20)
+#define ESR_ELx_EC_IABT_CUR	UL(0x21)
+#define ESR_ELx_EC_PC_ALIGN	UL(0x22)
 /* Unallocated EC: 0x23 */
-#define ESR_ELx_EC_DABT_LOW	(0x24)
-#define ESR_ELx_EC_DABT_CUR	(0x25)
-#define ESR_ELx_EC_SP_ALIGN	(0x26)
-#define ESR_ELx_EC_MOPS		(0x27)
-#define ESR_ELx_EC_FP_EXC32	(0x28)
+#define ESR_ELx_EC_DABT_LOW	UL(0x24)
+#define ESR_ELx_EC_DABT_CUR	UL(0x25)
+#define ESR_ELx_EC_SP_ALIGN	UL(0x26)
+#define ESR_ELx_EC_MOPS		UL(0x27)
+#define ESR_ELx_EC_FP_EXC32	UL(0x28)
 /* Unallocated EC: 0x29 - 0x2B */
-#define ESR_ELx_EC_FP_EXC64	(0x2C)
+#define ESR_ELx_EC_FP_EXC64	UL(0x2C)
 /* Unallocated EC: 0x2D - 0x2E */
-#define ESR_ELx_EC_SERROR	(0x2F)
-#define ESR_ELx_EC_BREAKPT_LOW	(0x30)
-#define ESR_ELx_EC_BREAKPT_CUR	(0x31)
-#define ESR_ELx_EC_SOFTSTP_LOW	(0x32)
-#define ESR_ELx_EC_SOFTSTP_CUR	(0x33)
-#define ESR_ELx_EC_WATCHPT_LOW	(0x34)
-#define ESR_ELx_EC_WATCHPT_CUR	(0x35)
+#define ESR_ELx_EC_SERROR	UL(0x2F)
+#define ESR_ELx_EC_BREAKPT_LOW	UL(0x30)
+#define ESR_ELx_EC_BREAKPT_CUR	UL(0x31)
+#define ESR_ELx_EC_SOFTSTP_LOW	UL(0x32)
+#define ESR_ELx_EC_SOFTSTP_CUR	UL(0x33)
+#define ESR_ELx_EC_WATCHPT_LOW	UL(0x34)
+#define ESR_ELx_EC_WATCHPT_CUR	UL(0x35)
 /* Unallocated EC: 0x36 - 0x37 */
-#define ESR_ELx_EC_BKPT32	(0x38)
+#define ESR_ELx_EC_BKPT32	UL(0x38)
 /* Unallocated EC: 0x39 */
-#define ESR_ELx_EC_VECTOR32	(0x3A)	/* EL2 only */
+#define ESR_ELx_EC_VECTOR32	UL(0x3A)	/* EL2 only */
 /* Unallocated EC: 0x3B */
-#define ESR_ELx_EC_BRK64	(0x3C)
+#define ESR_ELx_EC_BRK64	UL(0x3C)
 /* Unallocated EC: 0x3D - 0x3F */
-#define ESR_ELx_EC_MAX		(0x3F)
+#define ESR_ELx_EC_MAX		UL(0x3F)
 
 #define ESR_ELx_EC_SHIFT	(26)
 #define ESR_ELx_EC_WIDTH	(6)
-- 
2.30.2

Re: [PATCH v2] arm64: KVM: define ESR_ELx_EC_* constants as UL

Posted by Will Deacon 2 months, 2 weeks ago

On Tue, 10 Sep 2024 11:50:16 +0300, Anastasia Belova wrote:
> Add explicit casting to prevent expantion of 32th bit of
> u32 into highest half of u64 in several places.
> 
> For example, in inject_abt64:
> ESR_ELx_EC_DABT_LOW << ESR_ELx_EC_SHIFT = 0x24 << 26.
> This operation's result is int with 1 in 32th bit.
> While casting this value into u64 (esr is u64) 1
> fills 32 highest bits.
> 
> [...]

Applied to arm64 (for-next/misc), thanks!

[1/1] arm64: KVM: define ESR_ELx_EC_* constants as UL
      https://git.kernel.org/arm64/c/b6db3eb6c373

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

Re: [PATCH v2] arm64: KVM: define ESR_ELx_EC_* constants as UL

Posted by Marc Zyngier 2 months, 2 weeks ago

On Tue, 10 Sep 2024 09:50:16 +0100,
Anastasia Belova <abelova@astralinux.ru> wrote:
> 
> Add explicit casting to prevent expantion of 32th bit of
> u32 into highest half of u64 in several places.
> 
> For example, in inject_abt64:
> ESR_ELx_EC_DABT_LOW << ESR_ELx_EC_SHIFT = 0x24 << 26.
> This operation's result is int with 1 in 32th bit.
> While casting this value into u64 (esr is u64) 1
> fills 32 highest bits.
> 
> Found by Linux Verification Center (linuxtesting.org) with SVACE.
> 
> Fixes: aa8eff9bfbd5 ("arm64: KVM: fault injection into a guest")
> Signed-off-by: Anastasia Belova <abelova@astralinux.ru>

nit: the subject line is misleading, as this doesn't only affect KVM,
but the whole of the arm64 port (the exception classes form a generic
architectural construct).

This also probably deserve a Cc stable.

Will, Catalin: I'm happy to queue this in the KVM tree, but if you are
taking it directly:

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

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Re: [PATCH v2] arm64: KVM: define ESR_ELx_EC_* constants as UL

Posted by Will Deacon 2 months, 2 weeks ago

On Tue, Sep 10, 2024 at 10:08:49AM +0100, Marc Zyngier wrote:
> On Tue, 10 Sep 2024 09:50:16 +0100,
> Anastasia Belova <abelova@astralinux.ru> wrote:
> > 
> > Add explicit casting to prevent expantion of 32th bit of
> > u32 into highest half of u64 in several places.
> > 
> > For example, in inject_abt64:
> > ESR_ELx_EC_DABT_LOW << ESR_ELx_EC_SHIFT = 0x24 << 26.
> > This operation's result is int with 1 in 32th bit.
> > While casting this value into u64 (esr is u64) 1
> > fills 32 highest bits.
> > 
> > Found by Linux Verification Center (linuxtesting.org) with SVACE.
> > 
> > Fixes: aa8eff9bfbd5 ("arm64: KVM: fault injection into a guest")
> > Signed-off-by: Anastasia Belova <abelova@astralinux.ru>
> 
> nit: the subject line is misleading, as this doesn't only affect KVM,
> but the whole of the arm64 port (the exception classes form a generic
> architectural construct).

Weird, this v2 landed in my spam for some reason.

> This also probably deserve a Cc stable.
> 
> Will, Catalin: I'm happy to queue this in the KVM tree, but if you are
> taking it directly:
> 
> Acked-by: Marc Zyngier <maz@kernel.org>


I can take it via arm64. I assume it's ok to land in v6.12 (with the cc:
stable), or is there an urgency to landing this in v6.11? It looks it
was found using verification tools, rather than because of an actual
issue affecting users.

Will

Re: [PATCH v2] arm64: KVM: define ESR_ELx_EC_* constants as UL

Posted by Marc Zyngier 2 months, 2 weeks ago

On Tue, 10 Sep 2024 10:47:46 +0100,
Will Deacon <will@kernel.org> wrote:
> 
> On Tue, Sep 10, 2024 at 10:08:49AM +0100, Marc Zyngier wrote:
> > On Tue, 10 Sep 2024 09:50:16 +0100,
> > Anastasia Belova <abelova@astralinux.ru> wrote:
> > > 
> > > Add explicit casting to prevent expantion of 32th bit of
> > > u32 into highest half of u64 in several places.
> > > 
> > > For example, in inject_abt64:
> > > ESR_ELx_EC_DABT_LOW << ESR_ELx_EC_SHIFT = 0x24 << 26.
> > > This operation's result is int with 1 in 32th bit.
> > > While casting this value into u64 (esr is u64) 1
> > > fills 32 highest bits.
> > > 
> > > Found by Linux Verification Center (linuxtesting.org) with SVACE.
> > > 
> > > Fixes: aa8eff9bfbd5 ("arm64: KVM: fault injection into a guest")
> > > Signed-off-by: Anastasia Belova <abelova@astralinux.ru>
> > 
> > nit: the subject line is misleading, as this doesn't only affect KVM,
> > but the whole of the arm64 port (the exception classes form a generic
> > architectural construct).
> 
> Weird, this v2 landed in my spam for some reason.
> 
> > This also probably deserve a Cc stable.
> > 
> > Will, Catalin: I'm happy to queue this in the KVM tree, but if you are
> > taking it directly:
> > 
> > Acked-by: Marc Zyngier <maz@kernel.org>
> 
> 
> I can take it via arm64. I assume it's ok to land in v6.12 (with the cc:
> stable), or is there an urgency to landing this in v6.11? It looks it
> was found using verification tools, rather than because of an actual
> issue affecting users.

Yup, 6.12 is just fine. It would only affect ESR encodings that have
an ISS2 in the top 32 bits, and that's only a very small number of
features, most of which don't exist in HW yet.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.