Re: [PATCH v7 1/7] KVM: guest_memfd: Make guest mem use guest mem inodes instead of anonymous inodes

[Shivank Garg <shivankg@amd.com>]

Hi Ackerley, Fuad,

On 3/28/2025 9:01 PM, Fuad Tabba wrote:
> From: Ackerley Tng <ackerleytng@google.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: Fuad Tabba <tabba@google.com>
> Signed-off-by: Ackerley Tng <ackerleytng@google.com>
> ---

<snip>

> +
> +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;
> +}
> +
...

>  void kvm_gmem_init(struct module *module)
>  {
>  	kvm_gmem_fops.owner = module;
> +
> +	kvm_gmem_init_mount();
>  }

Looks like we’re missing a kern_unmount() call to properly clean up when
the KVM module gets unloaded. How about adding this?

 void kvm_gmem_exit(void)
 {
-
+	kern_unmount(kvm_gmem_mnt);
 }

This hooks up the teardown for the guest_memfd code nicely.
Right now, kvm_gmem_exit() isn’t even there, so this needs to be added.

https://lore.kernel.org/linux-mm/20250408112402.181574-6-shivankg@amd.com
 
>  
>  static int kvm_gmem_migrate_folio(struct address_space *mapping,
> @@ -511,11 +549,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);

Also, I hit a build error with security_inode_init_security_anon() when
using this in the module.
Looks like it needs to be exported. Adding this fixed it for me:

+EXPORT_SYMBOL(security_inode_init_security_anon);

Can you guys check if this looks good?
If so, there’s a revised patch here that includes these changes:
https://lore.kernel.org/linux-mm/20250408112402.181574-1-shivankg@amd.com

Let me know what you think!

Thanks,
Shivank