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