linux-kernel - Re: [PATCH v2 07/15] KVM: arm64: Introduce kvm_share

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] [day] [month] [year] [list]

Date:   Thu, 21 Oct 2021 10:07:47 +0000
From:   David Brazdil <dbrazdil@...gle.com>
To:     Quentin Perret <qperret@...gle.com>
Cc:     Marc Zyngier <maz@...nel.org>, James Morse <james.morse@....com>,
        Alexandru Elisei <alexandru.elisei@....com>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>, Fuad Tabba <tabba@...gle.com>,
        Andrew Walbran <qwandor@...gle.com>,
        linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.cs.columbia.edu,
        linux-kernel@...r.kernel.org, kernel-team@...roid.com
Subject: Re: [PATCH v2 07/15] KVM: arm64: Introduce kvm_share_hyp()

Hi Quentin,

> diff --git a/arch/arm64/kvm/mmu.c b/arch/arm64/kvm/mmu.c
> index 0019b2309f70..0cc4b295e525 100644
> --- a/arch/arm64/kvm/mmu.c
> +++ b/arch/arm64/kvm/mmu.c
> @@ -299,6 +299,17 @@ static int pkvm_share_hyp(phys_addr_t start, phys_addr_t end)
>  	return 0;
>  }
>  
> +int kvm_share_hyp(void *from, void *to)
> +{
> +	if (is_kernel_in_hyp_mode())
> +		return 0;
> +
> +	if (kvm_host_owns_hyp_mappings())
> +		return create_hyp_mappings(from, to, PAGE_HYP);
> +
> +	return pkvm_share_hyp(kvm_kaddr_to_phys(from), kvm_kaddr_to_phys(to));

We should be careful about vmalloc memory here. kvm_kaddr_to_phys will
happily return the physical address but the range is not guaranteed to
be physically contiguous. It doesn't look like this series ever shares
vmalloc memory, but we should make it harder for users of this function
to shoot themselves in the foot.

One option would be to turn this into a loop and call pkvm_share_hyp on
each physical page. But since the hypervisor has no means of making
those pages virtually contigous anyway, probably not the right approach.

We could make it possible to share vmalloc buffer that fit into a page
and reject others. There we still need to be careful about the upper
bound because the way it's written now, 'to' can be treated as
exclusive and 'kvm_kaddr_to_phys(to)' would not always return the page
after 'kvm_kaddr_to_phys(from)'.

-David