Re: [PATCH v4 07/25] KVM: arm64: Prevent the donation of no-map pages

Philippe Mathieu-Daudé
2022-10-18T13:43:24Z

Hi Will & Quentin,

On 17/10/22 13:51, Will Deacon wrote:
> From: Quentin Perret <qperret@google.com>
>
> Memory regions marked as "no-map" in the host device-tree routinely
> include TrustZone carevouts and DMA pools. Although donating such pages

Typo "carve-outs"?

> to the hypervisor may not breach confidentiality, it could be used to
> corrupt its state in uncontrollable ways. To prevent this, let's block
> host-initiated memory transitions targeting "no-map" pages altogether in
> nVHE protected mode as there should be no valid reason to do this in
> current operation.
>
> Thankfully, the pKVM EL2 hypervisor has a full copy of the host's list
> of memblock regions, so we can easily check for the presence of the
> MEMBLOCK_NOMAP flag on a region containing pages being donated from the
> host.
>
> Tested-by: Vincent Donnefort <vdonnefort@google.com>
> Signed-off-by: Quentin Perret <qperret@google.com>
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
> arch/arm64/kvm/hyp/nvhe/mem_protect.c | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> index c30402737548..a7156fd13bc8 100644
> --- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> +++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
> @@ -193,7 +193,7 @@ struct kvm_mem_range {
> u64 end;
> };

> bool addr_is_memory(phys_addr_t phys)
> {
> struct kvm_mem_range range;
>
> - return find_mem_range(phys, &range);
> + return !!find_mem_range(phys, &range);
> +}
> +
> +static bool addr_is_allowed_memory(phys_addr_t phys)
> +{
> + struct memblock_region *reg;
> + struct kvm_mem_range range;
> +
> + reg = find_mem_range(phys, &range);
> +
> + return reg && !(reg->flags & MEMBLOCK_NOMAP);
> }
>
> static bool is_in_mem_range(u64 addr, struct kvm_mem_range *range)
> @@ -346,7 +356,7 @@ static bool host_stage2_force_pte_cb(u64 addr, u64 end, enum kvm_pgtable_prot pr
> static int host_stage2_idmap(u64 addr)
> {
> struct kvm_mem_range range;
> - bool is_memory = find_mem_range(addr, &range);
> + bool is_memory = !!find_mem_range(addr, &range);

We don't replace by addr_is_allowed_memory() because we still use
&range, OK.

> enum kvm_pgtable_prot prot;
> int ret;
>
> @@ -424,7 +434,7 @@ static int __check_page_state_visitor(u64 addr, u64 end, u32 level,
> struct check_walk_data *d = arg;
> kvm_pte_t pte = *ptep;
>
> - if (kvm_pte_valid(pte) && !addr_is_memory(kvm_pte_to_phys(pte)))
> + if (kvm_pte_valid(pte) && !addr_is_allowed_memory(kvm_pte_to_phys(pte)))
> return -EINVAL;

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>