From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932130AbcGBAza (ORCPT ); Fri, 1 Jul 2016 20:55:30 -0400 Received: from mail-pa0-f50.google.com ([209.85.220.50]:33588 "EHLO mail-pa0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752554AbcGBAz2 (ORCPT ); Fri, 1 Jul 2016 20:55:28 -0400 Date: Fri, 1 Jul 2016 17:55:25 -0700 From: Omar Sandoval To: Andrey Vagin Cc: linux-fsdevel@vger.kernel.org, Miklos Szeredi , "J. Bruce Fields" , Arnd Bergmann , NeilBrown , containers@lists.linux-foundation.org, linux-kernel@vger.kernel.org, Shuah Khan , criu@openvz.org, "Eric W. Biederman" , Alexander Viro Subject: Re: [PATCH 1/3] namei: add LOOKUP_DFD_ROOT to use dfd as root Message-ID: <20160702005525.GA28451@vader.dhcp.thefacebook.com> References: <1467135510-2564-1-git-send-email-avagin@openvz.org> <1467135510-2564-2-git-send-email-avagin@openvz.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1467135510-2564-2-git-send-email-avagin@openvz.org> User-Agent: Mutt/1.6.1 (2016-04-27) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Jun 28, 2016 at 10:38:28AM -0700, Andrey Vagin wrote: > The problem is that a pathname can contain absolute symlinks and now > they are resolved relative to the current root. > > If we want to open a file in another mount namespaces and we have a file > descriptor to its root directory, we probably want to resolve pathname > in the target mount namespace. For this we add this new flag. > > If LOOKUP_DFD_ROOT is set, path_init() initializes nd->root and nd->path > to the same value. > > Signed-off-by: Andrey Vagin Hi, Andrey, Seems like a useful feature. Make sure to cc linux-api@vger.kernel.org for new userspace interfaces. One comment on the implementation below. > --- > fs/namei.c | 12 +++++++++++- > include/linux/namei.h | 2 ++ > 2 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/fs/namei.c b/fs/namei.c > index 70580ab..5f08b69 100644 > --- a/fs/namei.c > +++ b/fs/namei.c > @@ -2148,7 +2148,7 @@ static const char *path_init(struct nameidata *nd, unsigned flags) > nd->path.dentry = NULL; > > nd->m_seq = read_seqbegin(&mount_lock); > - if (*s == '/') { > + if (*s == '/' && !(flags & LOOKUP_DFD_ROOT)) { > if (flags & LOOKUP_RCU) > rcu_read_lock(); > set_root(nd); > @@ -2174,6 +2174,11 @@ static const char *path_init(struct nameidata *nd, unsigned flags) > get_fs_pwd(current->fs, &nd->path); > nd->inode = nd->path.dentry->d_inode; > } > + if (flags & LOOKUP_DFD_ROOT) { > + nd->root = nd->path; > + if (!(flags & LOOKUP_RCU)) > + path_get(&nd->root); You're not initializing nd->root_seq here. That means that if we end up going through unlazy_walk(), we're going to call legitimize_path() (and thus read_seqcount_retry()) with stack garbage, get a spurious ECHILD, and do an unnecessary restart of the path lookup instead of dropping into ref-walk mode. > + } > return s; > } else { > /* Caller must check execute permissions on the starting path component */ > @@ -2202,6 +2207,11 @@ static const char *path_init(struct nameidata *nd, unsigned flags) > nd->inode = nd->path.dentry->d_inode; > } > fdput(f); > + if (flags & LOOKUP_DFD_ROOT) { > + nd->root = nd->path; > + if (!(flags & LOOKUP_RCU)) > + path_get(&nd->root); > + } Same here. The following should do the trick: diff --git a/fs/namei.c b/fs/namei.c index 9958b605e822..101d1fb8d3cb 100644 --- a/fs/namei.c +++ b/fs/namei.c @@ -2176,7 +2176,9 @@ static const char *path_init(struct nameidata *nd, unsigned flags) } if (flags & LOOKUP_DFD_ROOT) { nd->root = nd->path; - if (!(flags & LOOKUP_RCU)) + if (flags & LOOKUP_RCU) + nd->root_seq = nd->seq; + else path_get(&nd->root); } return s; @@ -2209,7 +2211,9 @@ static const char *path_init(struct nameidata *nd, unsigned flags) fdput(f); if (flags & LOOKUP_DFD_ROOT) { nd->root = nd->path; - if (!(flags & LOOKUP_RCU)) + if (flags & LOOKUP_RCU) + nd->root_seq = nd->seq; + else path_get(&nd->root); } return s; -- Omar