linux-kernel - Re: [PATCH 1/1] KVM: arm64: Make nVHE ASLR conditional on nokaslr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <86mskmv7ts.wl-maz@kernel.org>
Date: Thu, 05 Sep 2024 09:11:11 +0100
From: Marc Zyngier <maz@...nel.org>
To: qixiang.xu@...look.com
Cc: oliver.upton@...ux.dev,
	will@...nel.org,
	linux-arm-kernel@...ts.infradead.org,
	kvmarm@...ts.linux.dev,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/1] KVM: arm64: Make nVHE ASLR conditional on nokaslr

On Thu, 05 Sep 2024 07:30:26 +0100,
qixiang.xu@...look.com wrote:
> 
> From: Qxiang Xu <qixiang.xu@...look.com>
> 
> The random tag of hyp VA is determined by the `CONFIG_RANDOMIZE_BASE`
> option, so even if `nokaslr` is set in the cmdline, KASLR cannot be
> disabled for hyp VA. To align with kernel behavior, disable KASLR if
> the kernel cmdline includes `nokaslr`.
> 
> Link: https://lore.kernel.org/r/20240905061659.3410362-1-qixiang.xu@outlook.com

I get a 404.

> Signed-off-by: Qxiang Xu <qixiang.xu@...look.com>
> ---
>  arch/arm64/kvm/va_layout.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/va_layout.c b/arch/arm64/kvm/va_layout.c
> index 91b22a014610..bebb4b1ddc82 100644
> --- a/arch/arm64/kvm/va_layout.c
> +++ b/arch/arm64/kvm/va_layout.c
> @@ -72,7 +72,7 @@ __init void kvm_compute_layout(void)
>  	va_mask = GENMASK_ULL(tag_lsb - 1, 0);
>  	tag_val = hyp_va_msb;
>  
> -	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE) && tag_lsb != (vabits_actual - 1)) {
> +	if (kaslr_enabled() && tag_lsb != (vabits_actual - 1)) {
>  		/* We have some free bits to insert a random tag. */
>  		tag_val |= get_random_long() & GENMASK_ULL(vabits_actual - 2, tag_lsb);
>  	}

This is a change in behaviour that would leave the 2 implementations
affected by Spectre-v3a unmitigated and leaking information to
*guests*, while they would have been safe until this change. Is this
what we really want to do?

This is also not disabling the whole thing, since we still do the
indirect vector dance.

So while I'm not opposed to having an option that disables the
randomisation, it has to match two requirements:

- it has to be a *new* option -- changing an existing behaviour is not
  acceptable,

- it has to disable both the VA alteration and the vector indirection.

Thanks,

	M.

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