MIME-Version: 1.0
In-Reply-To: <20171219154400.GS21978@ZenIV.linux.org.uk>
References: <20171219101440.19736-1-gscrivan@redhat.com> <20171219114819.GQ21978@ZenIV.linux.org.uk>
 <20171219153225.GA14771@ZenIV.linux.org.uk> <20171219154400.GS21978@ZenIV.linux.org.uk>
From: Dmitry Vyukov <dvyukov@google.com>
Date: Tue, 19 Dec 2017 17:31:48 +0100
Message-ID: <CACT4Y+b6=5zJX-N6byM-xBr75Pn8wr556jtBjtMsi88NQ8pFiQ@mail.gmail.com>
Subject: Re: [PATCH linux-next] mqueue: fix IPC namespace use-after-free
To: Al Viro <viro@zeniv.linux.org.uk>
Cc: Giuseppe Scrivano <gscrivan@redhat.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        LKML <linux-kernel@vger.kernel.org>, alexander.deucher@amd.com,
        Mark Brown <broonie@kernel.org>,
        Chris Wilson <chris@chris-wilson.co.uk>,
        David Miller <davem@davemloft.net>, deepa.kernel@gmail.com,
        Greg KH <gregkh@linuxfoundation.org>, luc.vanoostenryck@gmail.com,
        lucien xin <lucien.xin@gmail.com>, Ingo Molnar <mingo@kernel.org>,
        Neil Horman <nhorman@tuxdriver.com>, syzkaller-bugs@googlegroups.com,
        Vladislav Yasevich <vyasevich@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1557
Lines: 42

On Tue, Dec 19, 2017 at 4:44 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> 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...

Giuseppe, what report is this?
If there is a reproducer, you can ask syzbot to test a patch.