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


Groups > linux.kernel > #1592835

Re: fs: use-after-free in path_lookupat

From Al Viro <viro@ZenIV.linux.org.uk>
Newsgroups linux.kernel
Subject Re: fs: use-after-free in path_lookupat
Date 2017-03-05 21:10 +0100
Message-ID <thKDE-5px-15@gated-at.bofh.it> (permalink)
References (5 earlier) <thCPM-87W-11@gated-at.bofh.it> <thH34-2Ka-11@gated-at.bofh.it> <thH34-2Ka-9@gated-at.bofh.it> <thHmp-2Vk-3@gated-at.bofh.it> <thIit-3Dz-1@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


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;

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

fs: use-after-free in path_lookupat Dmitry Vyukov <dvyukov@google.com> - 2017-03-04 16:10 +0100
  Re: fs: use-after-free in path_lookupat Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-04 20:40 +0100
    Re: fs: use-after-free in path_lookupat Dmitry Vyukov <dvyukov@google.com> - 2017-03-05 12:20 +0100
      Re: fs: use-after-free in path_lookupat Dmitry Vyukov <dvyukov@google.com> - 2017-03-05 12:30 +0100
        Re: fs: use-after-free in path_lookupat Dmitry Vyukov <dvyukov@google.com> - 2017-03-05 12:50 +0100
          Re: fs: use-after-free in path_lookupat Dmitry Vyukov <dvyukov@google.com> - 2017-03-05 17:20 +0100
            Re: fs: use-after-free in path_lookupat Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-05 17:40 +0100
              Re: fs: use-after-free in path_lookupat Dmitry Vyukov <dvyukov@google.com> - 2017-03-05 18:40 +0100
                Re: fs: use-after-free in path_lookupat Dmitry Vyukov <dvyukov@google.com> - 2017-03-05 18:50 +0100
                Re: fs: use-after-free in path_lookupat Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-05 21:10 +0100
                Re: fs: use-after-free in path_lookupat Dmitry Vyukov <dvyukov@google.com> - 2017-03-06 10:50 +0100
          Re: fs: use-after-free in path_lookupat Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-05 19:10 +0100
      Re: fs: use-after-free in path_lookupat Dmitry Vyukov <dvyukov@google.com> - 2017-03-05 12:30 +0100

csiph-web