From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751148AbdE2Osl (ORCPT ); Mon, 29 May 2017 10:48:41 -0400 Received: from mail-ua0-f182.google.com ([209.85.217.182]:36445 "EHLO mail-ua0-f182.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751042AbdE2Osj (ORCPT ); Mon, 29 May 2017 10:48:39 -0400 MIME-Version: 1.0 In-Reply-To: References: <20170304193910.GG29622@ZenIV.linux.org.uk> <20170305155707.GI29622@ZenIV.linux.org.uk> <20170305163301.GJ29622@ZenIV.linux.org.uk> <20170305191802.GK29622@ZenIV.linux.org.uk> From: Dmitry Vyukov Date: Mon, 29 May 2017 16:48:17 +0200 Message-ID: Subject: Re: fs: use-after-free in path_lookupat To: Al Viro Cc: "linux-fsdevel@vger.kernel.org" , LKML , syzkaller , Cong Wang Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Apr 28, 2017 at 8:19 AM, Dmitry Vyukov wrote: > On Thu, Mar 23, 2017 at 3:17 PM, Dmitry Vyukov wrote: >> On Sun, Mar 5, 2017 at 8:18 PM, Al Viro 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.