2021-01-08 16:16:07

by Steven Price

[permalink] [raw]
Subject: [PATCH] KVM: arm64: Compute TPIDR_EL2 ignoring MTE tag

KASAN in HW_TAGS mode will store MTE tags in the top byte of the
pointer. When computing the offset for TPIDR_EL2 we don't want anything
in the top byte, so remove the tag to ensure the computation is correct
no matter what the tag.

Fixes: 94ab5b61ee16 ("kasan, arm64: enable CONFIG_KASAN_HW_TAGS")
Signed-off-by: Steven Price <[email protected]>
---
Without this fix I can't boot a config with KASAN_HW_TAGS and KVM on an
MTE enabled host. I'm unsure if this should really be in
this_cpu_ptr_nvhe_sym().

arch/arm64/kvm/arm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 6e637d2b4cfb..3783082148bc 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1403,7 +1403,7 @@ static void cpu_init_hyp_mode(void)
* kernel's mapping to the linear mapping, and store it in tpidr_el2
* so that we can use adr_l to access per-cpu variables in EL2.
*/
- params->tpidr_el2 = (unsigned long)this_cpu_ptr_nvhe_sym(__per_cpu_start) -
+ params->tpidr_el2 = (unsigned long)kasan_reset_tag(this_cpu_ptr_nvhe_sym(__per_cpu_start)) -
(unsigned long)kvm_ksym_ref(CHOOSE_NVHE_SYM(__per_cpu_start));

params->mair_el2 = read_sysreg(mair_el1);
--
2.20.1


2021-01-08 16:56:01

by Marc Zyngier

[permalink] [raw]
Subject: Re: [PATCH] KVM: arm64: Compute TPIDR_EL2 ignoring MTE tag

Hi Steven,

On 2021-01-08 16:12, Steven Price wrote:
> KASAN in HW_TAGS mode will store MTE tags in the top byte of the
> pointer. When computing the offset for TPIDR_EL2 we don't want anything
> in the top byte, so remove the tag to ensure the computation is correct
> no matter what the tag.
>
> Fixes: 94ab5b61ee16 ("kasan, arm64: enable CONFIG_KASAN_HW_TAGS")
> Signed-off-by: Steven Price <[email protected]>
> ---
> Without this fix I can't boot a config with KASAN_HW_TAGS and KVM on an
> MTE enabled host. I'm unsure if this should really be in
> this_cpu_ptr_nvhe_sym().

this_cpu_ptr_nvhe_sym() should return something that is valid for
the EL1 kernel, so I guess untagging in the helper may not be
that useful.

However, I'm more concerned by anything at requires us to follow
pointers set up by EL1 at EL2. It looks to me that the only reason
the whole thing works is because kern_hyp_va() *accidentally* drops
tags before applying the EL1/EL2 offset...

Or am I getting it wrong?

Thanks,

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

2021-01-08 17:06:41

by Steven Price

[permalink] [raw]
Subject: Re: [PATCH] KVM: arm64: Compute TPIDR_EL2 ignoring MTE tag

On 08/01/2021 16:51, Marc Zyngier wrote:
> Hi Steven,
>
> On 2021-01-08 16:12, Steven Price wrote:
>> KASAN in HW_TAGS mode will store MTE tags in the top byte of the
>> pointer. When computing the offset for TPIDR_EL2 we don't want anything
>> in the top byte, so remove the tag to ensure the computation is correct
>> no matter what the tag.
>>
>> Fixes: 94ab5b61ee16 ("kasan, arm64: enable CONFIG_KASAN_HW_TAGS")
>> Signed-off-by: Steven Price <[email protected]>
>> ---
>> Without this fix I can't boot a config with KASAN_HW_TAGS and KVM on an
>> MTE enabled host. I'm unsure if this should really be in
>> this_cpu_ptr_nvhe_sym().
>
> this_cpu_ptr_nvhe_sym() should return something that is valid for
> the EL1 kernel, so I guess untagging in the helper may not be
> that useful.

Makes sense and was my suspicion.

> However, I'm more concerned by anything at requires us to follow
> pointers set up by EL1 at EL2. It looks to me that the only reason
> the whole thing works is because kern_hyp_va() *accidentally* drops
> tags before applying the EL1/EL2 offset...

In the case I'm fixing this is intended to be an offset calculation -
it's just messed up by the presence of an MTE tag in one of the pointers.

I agree I was somewhat surprised when everything 'just worked' with this
one change - and I think you're right it's because kern_hyp_va() 'just
happens' to lose the tags. Of course there may be other bugs lurking -
running MTE+KASAN on the model is slow so I didn't do much beyond boot it.

One of the 'fun' things about MTE is that you can no longer do pointer
subtraction to calculate the offset unless the pointers are actually
from the same allocation (and therefore have the same tag). I'm sure the
C language experts would point out that's "always been the case" but it
will probably break things elsewhere too.

Steve

> Or am I getting it wrong?
>
> Thanks,
>
>         M.