linux-kernel - Re: [RFC PATCH v2 01/10] kvm: gmem: Add option to remove gmem from direct map

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: <Zuppxn_uW5JhDBjR@kernel.org>
Date: Wed, 18 Sep 2024 07:48:54 +0200
From: Mike Rapoport <rppt@...nel.org>
To: Patrick Roy <roypat@...zon.co.uk>
Cc: seanjc@...gle.com, pbonzini@...hat.com, tglx@...utronix.de,
	mingo@...hat.com, bp@...en8.de, dave.hansen@...ux.intel.com,
	x86@...nel.org, hpa@...or.com, rostedt@...dmis.org,
	mhiramat@...nel.org, mathieu.desnoyers@...icios.com,
	kvm@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-trace-kernel@...r.kernel.org, quic_eberman@...cinc.com,
	dwmw@...zon.com, david@...hat.com, tabba@...gle.com,
	linux-mm@...ck.org, dmatlack@...gle.com, graf@...zon.com,
	jgowans@...zon.com, derekmn@...zon.com, kalyazin@...zon.com,
	xmarcalx@...zon.com
Subject: Re: [RFC PATCH v2 01/10] kvm: gmem: Add option to remove gmem from
 direct map

On Tue, Sep 10, 2024 at 05:30:27PM +0100, Patrick Roy wrote:
> Add a flag to the KVM_CREATE_GUEST_MEMFD ioctl that causes gmem pfns
> to be removed from the host kernel's direct map. Memory is removed
> immediately after allocation and preparation of gmem folios (after
> preparation, as the prepare callback might expect the direct map entry
> to be present). Direct map entries are restored before
> kvm_arch_gmem_invalidate is called (as ->invalidate_folio is called
> before ->free_folio), for the same reason.
> 
> Use the PG_private flag to indicate that a folio is part of gmem with
> direct map removal enabled. While in this patch, PG_private does have a
> meaning of "folio not in direct map", this will no longer be true in
> follow up patches. Gmem folios might get temporarily reinserted into the
> direct map, but the PG_private flag needs to remain set, as the folios
> will have private data that needs to be freed independently of direct
> map status. This is why kvm_gmem_folio_clear_private does not call
> folio_clear_private.
> 
> kvm_gmem_{set,clear}_folio_private must be called with the folio lock
> held.
> 
> To ensure that failures in kvm_gmem_{clear,set}_private do not cause
> system instability due to leaving holes in the direct map, try to always
> restore direct map entries on failure. Pages for which restoration of
> direct map entries fails are marked as HWPOISON, to prevent the
> kernel from ever touching them again.
> 
> Signed-off-by: Patrick Roy <roypat@...zon.co.uk>
> ---
>  include/uapi/linux/kvm.h |  2 +
>  virt/kvm/guest_memfd.c   | 96 +++++++++++++++++++++++++++++++++++++---
>  2 files changed, 91 insertions(+), 7 deletions(-)
> 
> diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
> index 637efc0551453..81b0f4a236b8c 100644
> --- a/include/uapi/linux/kvm.h
> +++ b/include/uapi/linux/kvm.h
> @@ -1564,6 +1564,8 @@ struct kvm_create_guest_memfd {
>  	__u64 reserved[6];
>  };
>  
> +#define KVM_GMEM_NO_DIRECT_MAP			(1ULL << 0)
> +
>  #define KVM_PRE_FAULT_MEMORY	_IOWR(KVMIO, 0xd5, struct kvm_pre_fault_memory)
>  
>  struct kvm_pre_fault_memory {
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 1c509c3512614..2ed27992206f3 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -4,6 +4,7 @@
>  #include <linux/kvm_host.h>
>  #include <linux/pagemap.h>
>  #include <linux/anon_inodes.h>
> +#include <linux/set_memory.h>
>  
>  #include "kvm_mm.h"
>  
> @@ -49,8 +50,69 @@ static int kvm_gmem_prepare_folio(struct inode *inode, pgoff_t index, struct fol
>  	return 0;
>  }
>  
> +static bool kvm_gmem_test_no_direct_map(struct inode *inode)
> +{
> +	return ((unsigned long)inode->i_private & KVM_GMEM_NO_DIRECT_MAP) == KVM_GMEM_NO_DIRECT_MAP;
> +}
> +
> +static int kvm_gmem_folio_set_private(struct folio *folio)
> +{
> +	unsigned long start, npages, i;
> +	int r;
> +
> +	start = (unsigned long) folio_address(folio);
> +	npages = folio_nr_pages(folio);
> +
> +	for (i = 0; i < npages; ++i) {
> +		r = set_direct_map_invalid_noflush(folio_page(folio, i));
> +		if (r)
> +			goto out_remap;
> +	}

I feels like we need a new helper that takes care of contiguous pages.
arm64 already has set_memory_valid(), so it may be something like

	int set_direct_map_valid_noflush(struct page *p, unsigned nr, bool valid);

> +	flush_tlb_kernel_range(start, start + folio_size(folio));
> +	folio_set_private(folio);
> +	return 0;
> +out_remap:
> +	for (; i > 0; i--) {
> +		struct page *page = folio_page(folio, i - 1);
> +
> +		if (WARN_ON_ONCE(set_direct_map_default_noflush(page))) {
> +			/*
> +			 * Random holes in the direct map are bad, let's mark
> +			 * these pages as corrupted memory so that the kernel
> +			 * avoids ever touching them again.
> +			 */
> +			folio_set_hwpoison(folio);
> +			r = -EHWPOISON;
> +		}
> +	}
> +	return r;
> +}
> +

-- 
Sincerely yours,
Mike.