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
