linux-kernel - Re: [PATCH linux-next] mqueue: fix IPC namespace use-after-free

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20171219154400.GS21978@ZenIV.linux.org.uk>
Date:   Tue, 19 Dec 2017 15:44:01 +0000
From:   Al Viro <viro@...IV.linux.org.uk>
To:     Giuseppe Scrivano <gscrivan@...hat.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        LKML <linux-kernel@...r.kernel.org>, alexander.deucher@....com,
        broonie@...nel.org, chris@...is-wilson.co.uk,
        David Miller <davem@...emloft.net>, deepa.kernel@...il.com,
        Greg KH <gregkh@...uxfoundation.org>,
        luc.vanoostenryck@...il.com, lucien xin <lucien.xin@...il.com>,
        Ingo Molnar <mingo@...nel.org>,
        Neil Horman <nhorman@...driver.com>,
        syzkaller-bugs@...glegroups.com,
        Vladislav Yasevich <vyasevich@...il.com>
Subject: Re: [PATCH linux-next] mqueue: fix IPC namespace use-after-free

On Tue, Dec 19, 2017 at 03:32:25PM +0000, Al Viro wrote:
> +	m = mq_internal_mount();
> +	if (IS_ERR(m))
> +		return ERR_CAST(m);
> +	atomic_inc(&m->mnt_sb->s_active);
> +	down_write(&m->mnt_sb->s_umount);
> +	return dget(m->mnt_root);

Note: this is stripped down mount_subtree(m, ""), of course;
it might make sense to recognize that case and bypass the
create_mnt_ns/vfs_path_lookup/put_mnt_ns business in
mount_subtree() when the relative pathname is empty, replacing
it with path.mnt = mntget(mnt); path.dentry = dget(mnt->mnt_root);
in such case.  That'd allow to simply call mount_subtree() here.
It would work as-is, but it's ridiculously heavy for such use
right now.

>  static int __init init_mqueue_fs(void)
>  {
> +	struct vfsmount *m;
>  	int error;
>  
>  	mqueue_inode_cachep = kmem_cache_create("mqueue_inode_cache",
> @@ -1577,6 +1606,10 @@ static int __init init_mqueue_fs(void)
>  	if (error)
>  		goto out_filesystem;
>  
> +	m = kern_mount_data(&mqueue_fs_type, &init_ipc_ns);
> +	if (IS_ERR(m))
> +		goto out_filesystem;
> +	init_ipc_ns.mq_mnt = m;
>  	return 0;
>  
>  out_filesystem:

Unrelated issue, but register_filesystem() should be the last thing
module_init() of a filesystem driver does.  It's a separate story,
in any case...