From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1423938AbdD1GTo (ORCPT <rfc822;w@1wt.eu>);
        Fri, 28 Apr 2017 02:19:44 -0400
Received: from mail-vk0-f44.google.com ([209.85.213.44]:33811 "EHLO
        mail-vk0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1423906AbdD1GTe (ORCPT
        <rfc822;linux-kernel@vger.kernel.org>);
        Fri, 28 Apr 2017 02:19:34 -0400
MIME-Version: 1.0
In-Reply-To: <CACT4Y+byghFT+RVGoP9-osQYQfgujLmLGiFfBiR1oQ2Gfqf9TQ@mail.gmail.com>
References: <CACT4Y+bWe1bhVKmH4v1RrhQYUKzwu9BG1COaffx-4J2nnzPO-A@mail.gmail.com>
 <20170304193910.GG29622@ZenIV.linux.org.uk> <CACT4Y+Y23b8QhB8Hmasr5jXtwAwvvY=wVrorgRHzUWcdspURMg@mail.gmail.com>
 <CACT4Y+YNak+YLMpEGrTP5iwtrKB=Csd25fWKAT9hLYRYY6YzGQ@mail.gmail.com>
 <CACT4Y+ZNiki1Xdpe_m-=Tk+7TpeZNnvV-tq=cNai=Xs7dr0rbw@mail.gmail.com>
 <CACT4Y+ZHgE2r9fGsDS_7CHsxa53yrsx0Zp0TBvH4oeqg24w5Yg@mail.gmail.com>
 <20170305155707.GI29622@ZenIV.linux.org.uk> <CACT4Y+b2mfMmm1evdWn7PHkNOe1xhwtxqtUB83oawNRQ1ZzbKA@mail.gmail.com>
 <20170305163301.GJ29622@ZenIV.linux.org.uk> <CACT4Y+bZC8joKP=_TWbf1QT0SS7HBXe=b-9KNgHxxPPLFZ+FYA@mail.gmail.com>
 <20170305191802.GK29622@ZenIV.linux.org.uk> <CACT4Y+byghFT+RVGoP9-osQYQfgujLmLGiFfBiR1oQ2Gfqf9TQ@mail.gmail.com>
From: Dmitry Vyukov <dvyukov@google.com>
Date: Fri, 28 Apr 2017 08:19:12 +0200
Message-ID: <CACT4Y+asas_4rnUTZSXA4VKvgoUcjx=aHUuDn6SKX_t4MreogQ@mail.gmail.com>
Subject: Re: fs: use-after-free in path_lookupat
To: Al Viro <viro@zeniv.linux.org.uk>
Cc: "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        syzkaller <syzkaller@googlegroups.com>
Content-Type: text/plain; charset=UTF-8
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Mar 23, 2017 at 3:17 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> On Sun, Mar 5, 2017 at 8:18 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
>> On Sun, Mar 05, 2017 at 06:33:18PM +0100, Dmitry Vyukov wrote:
>>
>>> Added more debug output.
>>>
>>> name_to_handle_at(r4, &(0x7f0000003000-0x6)="2e2f62757300",
>>> &(0x7f0000003000-0xd)={0xc, 0x0, "cd21"}, &(0x7f0000002000)=0x0,
>>> 0x1000)
>>>
>>> actually passes name="" because of the overlapping addresses. Flags
>>> contain AT_EMPTY_PATH.
>>
>> Bloody hell...  So you end up with name == (char *)&handle->handle_type + 3?
>> Looks like it would be a lot more useful to dump the actual contents of
>> those suckers right before the syscall...
>>
>> Anyway, that explains WTF is going on.  The bug is in path_init() and
>> it triggers when you pass something with dentry allocated by d_alloc_pseudo()
>> as dfd, combined with empty pathname.  You need to have the file closed
>> by another thread, and have that another thread get out of closing syscall
>> (close(), dup2(), etc.) before the caller of path_init() gets to
>> complete_walk().  We need to make sure that this sucker gets DCACHE_RCUPDATE
>> while it's still guaranteed to be pinned down.  Could you try to reproduce
>> with the patch below applied?
>>
>> diff --git a/fs/namei.c b/fs/namei.c
>> index 6f7d96368734..70840281a41c 100644
>> --- a/fs/namei.c
>> +++ b/fs/namei.c
>> @@ -2226,11 +2226,16 @@ static const char *path_init(struct nameidata *nd, unsigned flags)
>>                 nd->path = f.file->f_path;
>>                 if (flags & LOOKUP_RCU) {
>>                         rcu_read_lock();
>> -                       nd->inode = nd->path.dentry->d_inode;
>> -                       nd->seq = read_seqcount_begin(&nd->path.dentry->d_seq);
>> +                       if (unlikely(!(dentry->d_flags & DCACHE_RCUACCESS))) {
>> +                               spin_lock(&dentry->d_lock);
>> +                               dentry->d_flags |= DCACHE_RCUACCESS;
>> +                               spin_unlock(&dentry->d_lock);
>> +                       }
>> +                       nd->inode = dentry->d_inode;
>> +                       nd->seq = read_seqcount_begin(&dentry->d_seq);
>>                 } else {
>>                         path_get(&nd->path);
>> -                       nd->inode = nd->path.dentry->d_inode;
>> +                       nd->inode = dentry->d_inode;
>>                 }
>>                 fdput(f);
>>                 return s;
>
>
> Al, please send this patch officially. I am running with it since then
> and have not seen the crashes, nor any other issues that look related.
>
> Thanks!


Al, ping. Please send this patch.