linux-kernel - Re: [PATCH v3 3/7] virt: geniezone: Introduce GenieZone hypervisor support

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: <86h6sakprk.wl-maz@kernel.org>
Date:   Thu, 18 May 2023 09:27:59 +0100
From:   Marc Zyngier <maz@...nel.org>
To:     Yi-De Wu <yi-de.wu@...iatek.com>
Cc:     Yingshiuan Pan <yingshiuan.pan@...iatek.com>,
        Ze-Yu Wang <ze-yu.wang@...iatek.com>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>,
        Jonathan Corbet <corbet@....net>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>, Arnd Bergmann <arnd@...db.de>,
        "Matthias Brugger" <matthias.bgg@...il.com>,
        AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>,
        <devicetree@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
        <linux-doc@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-arch@...r.kernel.org>, <linux-mediatek@...ts.infradead.org>,
        "Trilok Soni" <quic_tsoni@...cinc.com>,
        David Bradil <dbrazdil@...gle.com>,
        Jade Shih <jades.shih@...iatek.com>,
        Miles Chen <miles.chen@...iatek.com>,
        Ivan Tseng <ivan.tseng@...iatek.com>,
        My Chuang <my.chuang@...iatek.com>,
        Shawn Hsiao <shawn.hsiao@...iatek.com>,
        PeiLun Suei <peilun.suei@...iatek.com>,
        Liju Chen <liju-clr.chen@...iatek.com>
Subject: Re: [PATCH v3 3/7] virt: geniezone: Introduce GenieZone hypervisor support

On Fri, 12 May 2023 09:04:01 +0100,
Yi-De Wu <yi-de.wu@...iatek.com> wrote:
> 
> From: "Yingshiuan Pan" <yingshiuan.pan@...iatek.com>
> 
> GenieZone is MediaTek hypervisor solution, and it is running in EL2
> stand alone as a type-I hypervisor. This patch exports a set of ioctl
> interfaces for userspace VMM (e.g., crosvm) to operate guest VMs
> lifecycle (creation and destroy) on GenieZone.
> 
> Signed-off-by: Yingshiuan Pan <yingshiuan.pan@...iatek.com>
> Signed-off-by: Yi-De Wu <yi-de.wu@...iatek.com>

[...]

> +/**
> + * gzvm_gfn_to_pfn_memslot() - Translate gfn (guest ipa) to pfn (host pa),
> + *			       result is in @pfn
> + *
> + * Leverage KVM's gfn_to_pfn_memslot(). Because gfn_to_pfn_memslot() needs
> + * kvm_memory_slot as parameter, this function populates necessary fileds
> + * for calling gfn_to_pfn_memslot().
> + *
> + * Return:
> + * * 0			- Succeed
> + * * -EFAULT		- Failed to convert
> + */
> +static int gzvm_gfn_to_pfn_memslot(struct gzvm_memslot *memslot, u64 gfn, u64 *pfn)
> +{
> +	hfn_t __pfn;
> +	struct kvm_memory_slot kvm_slot = {0};
> +
> +	kvm_slot.base_gfn = memslot->base_gfn;
> +	kvm_slot.npages = memslot->npages;
> +	kvm_slot.dirty_bitmap = NULL;
> +	kvm_slot.userspace_addr = memslot->userspace_addr;
> +	kvm_slot.flags = memslot->flags;
> +	kvm_slot.id = memslot->slot_id;
> +	kvm_slot.as_id = 0;
> +
> +	__pfn = gfn_to_pfn_memslot(&kvm_slot, gfn);
> +	if (is_error_noslot_pfn(__pfn)) {
> +		*pfn = 0;
> +		return -EFAULT;
> +	}

I have commented on this before: there is absolutely *no way* that you
can use KVM as the unwilling helper for your stuff. You are passing
uninitialised data to the core KVM, completely ignoring the semantics
of all the other fields.

More importantly, you are now holding us responsible for any breakage
that would be caused to your code if we change the internals of this
*PRIVATE FUNCTION*.

Do you see Xen or Hyper-V using KVM's internals as some sort of
backend to make their life easier? No, because they understand that
this is off-limits, and creates an unhealthy dependency for both
hypervisors.

So this is a strong NAK. And you can trust me to keep voicing my
opposition to this sort of horror, wherever I will see these patches.

	M.

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