Yan Zhao yan.y.zhao@intel.com writes:
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.
No worries, thank you so much for your reviews!
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.
Thanks, Fuad will be addressing this in a revision of [1].
BTW, there're lots of symbols not exported under mm.
Thanks again, is there a good way to do a build test for symbols not being exported? What CONFIG flags do you use?
+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);}
Thanks, Fuad will be addressing this in a revision of [1].
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
[1] https://lore.kernel.org/all/20250328153133.3504118-2-tabba@google.com/