Re: general protection fault in kernfs_kill_sb (2)

From: Al Viro
Date: Mon May 14 2018 - 00:32:48 EST


On Mon, May 14, 2018 at 05:04:15AM +0100, Al Viro wrote:
> diff --git a/fs/sysfs/mount.c b/fs/sysfs/mount.c
> index b428d317ae92..92682fcc41f6 100644
> --- a/fs/sysfs/mount.c
> +++ b/fs/sysfs/mount.c
> @@ -25,7 +25,7 @@ static struct dentry *sysfs_mount(struct file_system_type *fs_type,
> {
> struct dentry *root;
> void *ns;
> - bool new_sb;
> + bool new_sb = false;
>
> if (!(flags & SB_KERNMOUNT)) {
> if (!kobj_ns_current_may_mount(KOBJ_NS_TYPE_NET))
> @@ -35,9 +35,9 @@ static struct dentry *sysfs_mount(struct file_system_type *fs_type,
> ns = kobj_ns_grab_current(KOBJ_NS_TYPE_NET);
> root = kernfs_mount_ns(fs_type, flags, sysfs_root,
> SYSFS_MAGIC, &new_sb, ns);
> - if (IS_ERR(root) || !new_sb)
> + if (!new_sb)
> kobj_ns_drop(KOBJ_NS_TYPE_NET, ns);
> - else if (new_sb)
> + else if (!IS_ERR(root))
> root->d_sb->s_iflags |= SB_I_USERNS_VISIBLE;
>
> return root;

What we want for that kobj_ns_drop() is "no fs instances created" (== no
->kill_sb(), be it now or later, to drop that kobj reference); for setting
->s_iflags - "new instance successfully set up".

That's it; all we need is new_sb that would be accurate on its own.
The problem is with kludging over the cases when it's left uninitialized
(early exits from kernfs_mount_ns()) with IS_ERR(root), which happens to
grab the cases when new_sb *was* set to true. So the fix is to initialize
new_sb properly and get rid of that kludge. Which turns the whole thing
into
if (!new_sb)
...
if (!IS_ERR(root) && new_sb)
...
i.e.
if (!new_sb)
...
else if (!IS_ERR(root))
...