lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Mon, 18 Jan 2021 08:06:21 +1100
From:   Dave Chinner <david@...morbit.com>
To:     Christian Brauner <christian.brauner@...ntu.com>
Cc:     Alexander Viro <viro@...iv.linux.org.uk>,
        Christoph Hellwig <hch@...radead.org>,
        linux-fsdevel@...r.kernel.org,
        John Johansen <john.johansen@...onical.com>,
        James Morris <jmorris@...ei.org>,
        Mimi Zohar <zohar@...ux.ibm.com>,
        Dmitry Kasatkin <dmitry.kasatkin@...il.com>,
        Stephen Smalley <stephen.smalley.work@...il.com>,
        Casey Schaufler <casey@...aufler-ca.com>,
        Arnd Bergmann <arnd@...db.de>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>,
        Geoffrey Thomas <geofft@...reload.com>,
        Mrunal Patel <mpatel@...hat.com>,
        Josh Triplett <josh@...htriplett.org>,
        Andy Lutomirski <luto@...nel.org>,
        Theodore Tso <tytso@....edu>, Alban Crequy <alban@...volk.io>,
        Tycho Andersen <tycho@...ho.ws>,
        David Howells <dhowells@...hat.com>,
        James Bottomley <James.Bottomley@...senpartnership.com>,
        Seth Forshee <seth.forshee@...onical.com>,
        St├ęphane Graber <stgraber@...ntu.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Aleksa Sarai <cyphar@...har.com>,
        Lennart Poettering <lennart@...ttering.net>,
        "Eric W. Biederman" <ebiederm@...ssion.com>, smbarber@...omium.org,
        Phil Estes <estesp@...il.com>, Serge Hallyn <serge@...lyn.com>,
        Kees Cook <keescook@...omium.org>,
        Todd Kjos <tkjos@...gle.com>, Paul Moore <paul@...l-moore.com>,
        Jonathan Corbet <corbet@....net>,
        containers@...ts.linux-foundation.org,
        linux-security-module@...r.kernel.org, linux-api@...r.kernel.org,
        linux-ext4@...r.kernel.org, linux-xfs@...r.kernel.org,
        linux-integrity@...r.kernel.org, selinux@...r.kernel.org,
        Christoph Hellwig <hch@....de>
Subject: Re: [PATCH v5 37/42] xfs: support idmapped mounts

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@....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@....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@...morbit.com

Powered by blists - more mailing lists