Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 18 Jan 2021 08:06:21 +1100
From:   Dave Chinner <david@fromorbit.com>
To:     Christian Brauner <christian.brauner@ubuntu.com>
Cc:     Alexander Viro <viro@zeniv.linux.org.uk>,
        Christoph Hellwig <hch@infradead.org>,
        linux-fsdevel@vger.kernel.org,
        John Johansen <john.johansen@canonical.com>,
        James Morris <jmorris@namei.org>,
        Mimi Zohar <zohar@linux.ibm.com>,
        Dmitry Kasatkin <dmitry.kasatkin@gmail.com>,
        Stephen Smalley <stephen.smalley.work@gmail.com>,
        Casey Schaufler <casey@schaufler-ca.com>,
        Arnd Bergmann <arnd@arndb.de>,
        Andreas Dilger <adilger.kernel@dilger.ca>,
        OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>,
        Geoffrey Thomas <geofft@ldpreload.com>,
        Mrunal Patel <mpatel@redhat.com>,
        Josh Triplett <josh@joshtriplett.org>,
        Andy Lutomirski <luto@kernel.org>,
        Theodore Tso <tytso@mit.edu>, Alban Crequy <alban@kinvolk.io>,
        Tycho Andersen <tycho@tycho.ws>,
        David Howells <dhowells@redhat.com>,
        James Bottomley <James.Bottomley@hansenpartnership.com>,
        Seth Forshee <seth.forshee@canonical.com>,
        =?iso-8859-1?Q?St=E9phane?= Graber <stgraber@ubuntu.com>,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Aleksa Sarai <cyphar@cyphar.com>,
        Lennart Poettering <lennart@poettering.net>,
        "Eric W. Biederman" <ebiederm@xmission.com>, smbarber@chromium.org,
        Phil Estes <estesp@gmail.com>, Serge Hallyn <serge@hallyn.com>,
        Kees Cook <keescook@chromium.org>,
        Todd Kjos <tkjos@google.com>, Paul Moore <paul@paul-moore.com>,
        Jonathan Corbet <corbet@lwn.net>,
        containers@lists.linux-foundation.org,
        linux-security-module@vger.kernel.org, linux-api@vger.kernel.org,
        linux-ext4@vger.kernel.org, linux-xfs@vger.kernel.org,
        linux-integrity@vger.kernel.org, selinux@vger.kernel.org,
        Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH v5 37/42] xfs: support idmapped mounts
Message-ID: <20210117210621.GA78941@dread.disaster.area>
References: <20210112220124.837960-1-christian.brauner@ubuntu.com>
 <20210112220124.837960-38-christian.brauner@ubuntu.com>
 <20210114205154.GL331610@dread.disaster.area>
 <20210114221048.ppf2pfuxrjak4kvm@wittgenstein>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210114221048.ppf2pfuxrjak4kvm@wittgenstein>
Precedence: bulk

On Thu, Jan 14, 2021 at 11:10:48PM +0100, Christian Brauner wrote:
> On Fri, Jan 15, 2021 at 07:51:54AM +1100, Dave Chinner wrote:
> > On Tue, Jan 12, 2021 at 11:01:19PM +0100, Christian Brauner wrote:
> > > From: Christoph Hellwig <hch@lst.de>
> > > 
> > > Enable idmapped mounts for xfs. This basically just means passing down
> > > the user_namespace argument from the VFS methods down to where it is
> > > passed to helper.
> > > 
> > > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ....
> > > @@ -654,6 +658,7 @@ xfs_vn_change_ok(
> > >   */
> > >  static int
> > >  xfs_setattr_nonsize(
> > > +	struct user_namespace	*mnt_userns,
> > >  	struct xfs_inode	*ip,
> > >  	struct iattr		*iattr)
> > >  {
> > > @@ -813,7 +818,7 @@ xfs_setattr_nonsize(
> > >  	 * 	     Posix ACL code seems to care about this issue either.
> > >  	 */
> > >  	if (mask & ATTR_MODE) {
> > > -		error = posix_acl_chmod(&init_user_ns, inode, inode->i_mode);
> > > +		error = posix_acl_chmod(mnt_userns, inode, inode->i_mode);
> > >  		if (error)
> > >  			return error;
> > >  	}
> > > @@ -868,7 +873,7 @@ xfs_setattr_size(
> > >  		 * Use the regular setattr path to update the timestamps.
> > >  		 */
> > >  		iattr->ia_valid &= ~ATTR_SIZE;
> > > -		return xfs_setattr_nonsize(ip, iattr);
> > > +		return xfs_setattr_nonsize(&init_user_ns, ip, iattr);
> > 
> > Shouldn't that be passing mnt_userns?
> 
> Hey Dave,
> 
> Thanks for taking a look.
> 
> This is the time updating codepath.

Yes, I understand the code path, that's why I asked the question and
commented that it's a landmine. That is, if in future we ever need
to do anything that is is in any way namespace related in the
truncate path, the wrong thing will happen because we are passing
the wrong namespace into that function.

Please just pass down the correct namespace for the operation even
though we don't currently require it for the operations being
performed in that path.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com