Fix kernel NULL function pointer dereference in vfs_kern_mount()
PATCH 1/1 Fix kernel NULL function pointer dereference in vfs_kern_mount()
This patch is try to fix a potential NULL function pointer dereference in the exported vfs_kern_mount() function which can be triggered by following kernel module code piece:
static struct vfsmount *npfs_mnt;
static struct file_system_type np_fs_type = {
.name = "npfs",
.kill_sb = npfs_kill_sb,
.fs_flags = FS_USERNS_MOUNT,
};
static int fsmod_init(void)
{
int err = -ENOMEM;
err = register_filesystem(&np_fs_type);
if (!err) {
npfs_mnt = kern_mount(&np_fs_type);
if (IS_ERR(npfs_mnt)) {
printk(KERN_ERR "npfs: could not mount!\n");
unregister_filesystem(&np_fs_type);
return PTR_ERR(npfs_mnt);
}
}
return err;
}I happened to forget to implement a (*mount)() function in the np_fs_type instance which is buggy definitely, but as an exported function to the outside, kernel should not suppose the caller will always take the right action. In this scenario, I have to reboot my machine to clean the loaded kernel module.
---
fs/filesystems.c | 5 +++++ fs/namespace.c | 2 ++ fs/super.c | 10 ++++++++++ 3 files changed, 17 insertions(+)
fs/filesystems.c +5 -0
@@ -71,6 +71,11 @@ int register_filesystem(struct file_system_type * fs)int res =0 ;struct file_system_type ** p; +if (!fs) +return -ENODEV ; +if (!fs->name) +return -EINVAL ; +BUG_ON (strchr (fs->name,'.' ));if (fs->next)return -EBUSY ;
fs/namespace.c +2 -0
@@ -2758,6 +2758,8 @@ void put_mnt_ns(struct mnt_namespace *ns)struct vfsmount *kern_mount_data (struct file_system_type *type,void *data) {struct vfsmount *mnt; +if (!type) +return ERR_PTR (-ENODEV ); mnt =vfs_kern_mount (type,MS_KERNMOUNT , type->name, data);if (!IS_ERR (mnt)) {/*
fs/super.c +10 -0
@@ -1101,11 +1101,21 @@ mount_fs(struct file_system_type *type, int flags, const char *name, void *data) goto out_free_secdata; } +if (!type->mount) { + error = -EINVAL ; +printk (KERN_ERR "No 'mount' method defined in %s\n" , type->name); + goto out_free_secdata; + } root = type->mount (type, flags, name, data);if (IS_ERR (root)) { error =PTR_ERR (root); goto out_free_secdata; } +if (!root) { + error = -EINVAL ; + goto out_free_secdata; + } + sb = root->d_sb;BUG_ON (!sb);WARN_ON (!sb->s_bdi);
Quoted 79 lines · Dennis Chen
This patch is try to fix a potential NULL function pointer dereference in the exported
vfs_kern_mount()function which can be triggered by following kernel module code piece:
static struct vfsmount*npfs_mnt;
static structfile_system_typenp_fs_type= {
.name = "npfs",
.kill_sb =npfs_kill_sb,
.fs_flags =FS_USERNS_MOUNT,
};
static intfsmod_init(void){ int err = -ENOMEM; err = register_filesystem(&np_fs_type); if (!err) { npfs_mnt = kern_mount(&np_fs_type); if (IS_ERR(npfs_mnt)) { printk(KERN_ERR "npfs: could not mount!\n"); unregister_filesystem(&np_fs_type); return PTR_ERR(npfs_mnt); } } return err; }I happened to forget to implement a (*mount)() function in the
np_fs_typeinstance which is buggy definitely, but as an exported function to the outside, kernel should not suppose the caller will always take the right action. In this scenario, I have to reboot my machine to clean the loaded kernel module.Signed-off-by Dennis Chen <[email protected]>Cc Alexander Viro <[email protected]>---
fs/filesystems.c | 5 +++++ fs/namespace.c | 2 ++ fs/super.c | 10 ++++++++++ 3 files changed, 17 insertions(+)fs/filesystems.c +5
-0@@ -71,6 +71,11 @@ int register_filesystem(struct file_system_type * fs)int res =0 ;struct file_system_type ** p; +if (!fs) +return -ENODEV ; +if (!fs->name) +return -EINVAL ; +BUG_ON (strchr (fs->name,'.' ));if (fs->next)return -EBUSY ;fs/namespace.c +2
-0@@ -2758,6 +2758,8 @@ void put_mnt_ns(struct mnt_namespace *ns)struct vfsmount *kern_mount_data (struct file_system_type *type,void *data) {struct vfsmount *mnt; +if (!type) +return ERR_PTR (-ENODEV ); mnt =vfs_kern_mount (type,MS_KERNMOUNT , type->name, data);if (!IS_ERR (mnt)) {/* fs/super.c +5
-0@@ -1101,11 +1101,21 @@ mount_fs(struct file_system_type *type, int flags, const char *name, void *data) goto out_free_secdata; } +if (!type->mount) { + error = -EINVAL ; +printk (KERN_ERR "No 'mount' method defined in %s\n" , type->name); + goto out_free_secdata; + } root = type->mount (type, flags, name, data);if (IS_ERR (root)) { error =PTR_ERR (root);
fs/super.c mount_fs(struct file_system_type *type, int flags, const char *name, void *data) · line 1121
Quoted 10 lines
fs/super.c +5
-0goto out_free_secdata; } +if (!root) { + error = -EINVAL ; + goto out_free_secdata; + } + sb = root->d_sb;BUG_ON (!sb);WARN_ON (!sb->s_bdi);
Any feedback for this patch? If we don't need it meaning the kernel module developer should take the responsibility to make sure his/her codes will not crash something, but in my opinion, as exported kernel function, it should do its best to avoid put the kernel in a unstable state, while not transfer the responsibility to the user, right?
den