linux-kernel - Re: [PATCH RFC v7 5/8] KVM: guest_memfd: Make guest mem use guest mem inodes instead of anonymous inodes

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: <Z_eEfjrkspAt4ACP@infradead.org>
Date: Thu, 10 Apr 2025 01:42:38 -0700
From: Christoph Hellwig <hch@...radead.org>
To: Shivank Garg <shivankg@....com>
Cc: seanjc@...gle.com, david@...hat.com, vbabka@...e.cz,
	willy@...radead.org, akpm@...ux-foundation.org, shuah@...nel.org,
	pbonzini@...hat.com, ackerleytng@...gle.com, paul@...l-moore.com,
	jmorris@...ei.org, serge@...lyn.com, pvorel@...e.cz,
	bfoster@...hat.com, tabba@...gle.com, vannapurve@...gle.com,
	chao.gao@...el.com, bharata@....com, nikunj@....com,
	michael.day@....com, yan.y.zhao@...el.com, Neeraj.Upadhyay@....com,
	thomas.lendacky@....com, michael.roth@....com, aik@....com,
	jgg@...dia.com, kalyazin@...zon.com, peterx@...hat.com,
	linux-fsdevel@...r.kernel.org, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org, linux-security-module@...r.kernel.org,
	kvm@...r.kernel.org, linux-kselftest@...r.kernel.org,
	linux-coco@...ts.linux.dev
Subject: Re: [PATCH RFC v7 5/8] KVM: guest_memfd: Make guest mem use guest
 mem inodes instead of anonymous inodes

On Tue, Apr 08, 2025 at 11:23:59AM +0000, Shivank Garg wrote:
> From: Ackerley Tng <ackerleytng@...gle.com>
> 
> Using guest mem inodes allows us to store metadata for the backing
> memory on the inode. Metadata will be added in a later patch to support
> HugeTLB pages.
> 
> Metadata about backing memory should not be stored on the file, since
> the file represents a guest_memfd's binding with a struct kvm, and
> metadata about backing memory is not unique to a specific binding and
> struct kvm.
> 
> Signed-off-by: Ackerley Tng <ackerleytng@...gle.com>
> Signed-off-by: Fuad Tabba <tabba@...gle.com>
> Signed-off-by: Shivank Garg <shivankg@....com>
> ---
>  include/uapi/linux/magic.h |   1 +
>  virt/kvm/guest_memfd.c     | 133 +++++++++++++++++++++++++++++++------
>  2 files changed, 113 insertions(+), 21 deletions(-)
> 
> diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h
> index bb575f3ab45e..169dba2a6920 100644
> --- a/include/uapi/linux/magic.h
> +++ b/include/uapi/linux/magic.h
> @@ -103,5 +103,6 @@
>  #define DEVMEM_MAGIC		0x454d444d	/* "DMEM" */
>  #define SECRETMEM_MAGIC		0x5345434d	/* "SECM" */
>  #define PID_FS_MAGIC		0x50494446	/* "PIDF" */
> +#define GUEST_MEMORY_MAGIC	0x474d454d	/* "GMEM" */
>  
>  #endif /* __LINUX_MAGIC_H__ */
> diff --git a/virt/kvm/guest_memfd.c b/virt/kvm/guest_memfd.c
> index 88453b040926..002328569c9e 100644
> --- a/virt/kvm/guest_memfd.c
> +++ b/virt/kvm/guest_memfd.c
> @@ -1,12 +1,17 @@
>  // SPDX-License-Identifier: GPL-2.0
> +#include <linux/fs.h>
> +#include <linux/mount.h>
>  #include <linux/backing-dev.h>
>  #include <linux/falloc.h>
>  #include <linux/kvm_host.h>
> +#include <linux/pseudo_fs.h>
>  #include <linux/pagemap.h>
>  #include <linux/anon_inodes.h>
>  
>  #include "kvm_mm.h"
>  
> +static struct vfsmount *kvm_gmem_mnt;
> +
>  struct kvm_gmem {
>  	struct kvm *kvm;
>  	struct xarray bindings;
> @@ -312,6 +317,38 @@ static pgoff_t kvm_gmem_get_index(struct kvm_memory_slot *slot, gfn_t gfn)
>  	return gfn - slot->base_gfn + slot->gmem.pgoff;
>  }
>  
> +static const struct super_operations kvm_gmem_super_operations = {
> +	.statfs		= simple_statfs,
> +};
> +
> +static int kvm_gmem_init_fs_context(struct fs_context *fc)
> +{
> +	struct pseudo_fs_context *ctx;
> +
> +	if (!init_pseudo(fc, GUEST_MEMORY_MAGIC))
> +		return -ENOMEM;
> +
> +	ctx = fc->fs_private;
> +	ctx->ops = &kvm_gmem_super_operations;
> +
> +	return 0;
> +}
> +
> +static struct file_system_type kvm_gmem_fs = {
> +	.name		 = "kvm_guest_memory",
> +	.init_fs_context = kvm_gmem_init_fs_context,
> +	.kill_sb	 = kill_anon_super,
> +};
> +
> +static void kvm_gmem_init_mount(void)
> +{
> +	kvm_gmem_mnt = kern_mount(&kvm_gmem_fs);
> +	BUG_ON(IS_ERR(kvm_gmem_mnt));
> +
> +	/* For giggles. Userspace can never map this anyways. */
> +	kvm_gmem_mnt->mnt_flags |= MNT_NOEXEC;
> +}
> +
>  static struct file_operations kvm_gmem_fops = {
>  	.open		= generic_file_open,
>  	.release	= kvm_gmem_release,
> @@ -321,11 +358,13 @@ static struct file_operations kvm_gmem_fops = {
>  void kvm_gmem_init(struct module *module)
>  {
>  	kvm_gmem_fops.owner = module;
> +
> +	kvm_gmem_init_mount();
>  }
>  
>  void kvm_gmem_exit(void)
>  {
> -
> +	kern_unmount(kvm_gmem_mnt);
>  }
>  
>  static int kvm_gmem_migrate_folio(struct address_space *mapping,
> @@ -407,11 +446,79 @@ static const struct inode_operations kvm_gmem_iops = {
>  	.setattr	= kvm_gmem_setattr,
>  };
>  
> +static struct inode *kvm_gmem_inode_make_secure_inode(const char *name,
> +						      loff_t size, u64 flags)
> +{
> +	const struct qstr qname = QSTR_INIT(name, strlen(name));
> +	struct inode *inode;
> +	int err;
> +
> +	inode = alloc_anon_inode(kvm_gmem_mnt->mnt_sb);
> +	if (IS_ERR(inode))
> +		return inode;
> +
> +	err = security_inode_init_security_anon(inode, &qname, NULL);
> +	if (err) {
> +		iput(inode);
> +		return ERR_PTR(err);
> +	}

So why do other alloc_anon_inode callers not need
security_inode_init_security_anon?