Re: [RFC PATCH 13/39] KVM: guest_memfd: Make guest mem use guest mem inodes instead of anonymous inodes

Yan Zhao
2025-04-02T04:03:43Z

Hi Ackerley,

Not sure if below nits have been resolved in your latest code.
I came across them and felt it's better to report them anyway.

Apologies for any redundancy if you've already addressed them.

On Tue, Sep 10, 2024 at 11:43:44PM +0000, Ackerley Tng wrote:
> +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,
> @@ -311,6 +348,8 @@ static struct file_operations kvm_gmem_fops = {
> void kvm_gmem_init(struct module *module)
> {
> kvm_gmem_fops.owner = module;
> +
> + kvm_gmem_init_mount();
> }
When KVM is compiled as a module, looks "kern_unmount(kvm_gmem_mnt)" is
missing in the kvm_exit() path.

This may lead to kernel oops when executing "sync" after KVM is unloaded or
reloaded.

BTW, there're lots of symbols not exported under mm.

> +static struct file *kvm_gmem_inode_create_getfile(void *priv, loff_t size,
> + u64 flags)
> +{
> + static const char *name = "[kvm-gmem]";
> + struct inode *inode;
> + struct file *file;
> +
> + if (kvm_gmem_fops.owner && !try_module_get(kvm_gmem_fops.owner))
> + return ERR_PTR(-ENOENT);
> +
> + inode = kvm_gmem_inode_make_secure_inode(name, size, flags);
> + if (IS_ERR(inode))
Missing module_put() here. i.e.,

- if (IS_ERR(inode))
+ if (IS_ERR(inode)) {
+ if (kvm_gmem_fops.owner)
+ module_put(kvm_gmem_fops.owner);
+
return ERR_CAST(inode);
+ }

> + return ERR_CAST(inode);
> +
> + file = alloc_file_pseudo(inode, kvm_gmem_mnt, name, O_RDWR,
> + &kvm_gmem_fops);
> + if (IS_ERR(file)) {
> + iput(inode);
> + return file;
> + }
> +
> + file->f_mapping = inode->i_mapping;
> + file->f_flags |= O_LARGEFILE;
> + file->private_data = priv;
> +
> + return file;
> +}
> +

Thanks
Yan