Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1652569 > unrolled thread

Re: fs: use-after-free in path_lookupat

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: fs: use-after-free in path_lookupat Dmitry Vyukov <dvyukov@google.com> - 2017-05-29 16:50 +0200
    Re: fs: use-after-free in path_lookupat Al Viro <viro@ZenIV.linux.org.uk> - 2017-05-30 08:30 +0200
      Re: fs: use-after-free in path_lookupat Dmitry Vyukov <dvyukov@google.com> - 2017-05-30 10:30 +0200

#1652569 — Re: fs: use-after-free in path_lookupat

On Fri, Apr 28, 2017 at 8:19 AM, Dmitry Vyukov <dvyukov@google.com> wrote:
> 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.


Al, do you want me to mail the patch?
I won't be able to write a super detailed description, but I can do
some format patch.

[toc] | [next] | [standalone]

#1652861

On Mon, May 29, 2017 at 04:48:17PM +0200, Dmitry Vyukov wrote:

> Al, do you want me to mail the patch?
> I won't be able to write a super detailed description, but I can do
> some format patch.

It's been fixed by commit c0eb027e5aef7; if you are still able to
trigger it on the current mainline, please yell - that would have
to be something different.

[toc] | [prev] | [next] | [standalone]

#1652940

On Tue, May 30, 2017 at 8:24 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Mon, May 29, 2017 at 04:48:17PM +0200, Dmitry Vyukov wrote:
>
>> Al, do you want me to mail the patch?
>> I won't be able to write a super detailed description, but I can do
>> some format patch.
>
> It's been fixed by commit c0eb027e5aef7; if you are still able to
> trigger it on the current mainline, please yell - that would have
> to be something different.


Thanks for the update!

No, I can't say that I still see this. It happened very infrequently,
so I wanted to make sure that we don't lost it regardless of whether I
see it or not.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web