Hi Ackerley, Fuad,
On 3/28/2025 9:01 PM, Fuad Tabba wrote:
> From: Ackerley Tng <ackerleytng@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> Signed-off-by: Ackerley Tng <ackerleytng@xxxxxxxxxx>
> ---
<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@xxxxxxx
>
> 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@xxxxxxx
Let me know what you think!
Thanks,
Shivank
