All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Brian Foster <bfoster@redhat.com>
Cc: iustin@k1024.org, xfs@oss.sgi.com
Subject: Re: [PATCH 6/9] xfs: XFS_IOCTL_SETXATTR can run in user namespaces
Date: Fri, 30 Jan 2015 10:53:15 +1100	[thread overview]
Message-ID: <20150129235315.GG6282@dastard> (raw)
In-Reply-To: <20150129153515.GF17652@laptop.bfoster>

On Thu, Jan 29, 2015 at 10:35:15AM -0500, Brian Foster wrote:
> On Tue, Jan 27, 2015 at 02:14:43PM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@redhat.com>
> > 
> > Currently XFS_IOCTL_SETXATTR will fail if run in a user namespace as
> > it it not allowed to change project IDs. The current code, however,
> > also prevents any other change being made as well, so things like
> > extent size hints cannot be set in user namespaces. This is wrong,
> > so only disallow access to project IDs and related flags from inside
> > the init namespace.
> > 
> > Signed-off-by: Dave Chinner <dchinner@redhat.com>
> > ---
> >  fs/xfs/xfs_ioctl.c | 22 +++++++++++++---------
> >  1 file changed, 13 insertions(+), 9 deletions(-)
> > 
> > diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> > index 563d2b4..ae6e1e3 100644
> > --- a/fs/xfs/xfs_ioctl.c
> > +++ b/fs/xfs/xfs_ioctl.c
> > @@ -1120,6 +1120,19 @@ xfs_ioctl_setattr(
> >  		return -EINVAL;
> >  
> >  	/*
> > +	 * Project Quota ID state is only allowed to change from within the init
> > +	 * namespace. Enforce that restriction only if we are trying to change
> > +	 * the quota ID state. Everything else is allowed in user namespaces.
> > +	 */
> > +	if (current_user_ns() != &init_user_ns) {
> > +		if (xfs_get_projid(ip) != fa->fsx_projid)
> > +			return -EINVAL;
> > +		if ((fa->fsx_xflags & XFS_XFLAG_PROJINHERIT) ^
> > +		    (ip->i_d.di_flags & XFS_DIFLAG_PROJINHERIT))
> 
> Why not use != here? Looks fine, anyways:

Because ^ has an implicit cast of the variables to boolean (i.e flag
set or not), whereas != will only work if XFS_XFLAG_PROJINHERIT =
XFS_DIFLAG_PROJINHERIT. Given that the moment we add more DIFLAGs to
the xfs inode, the current "XFLAG value must match DIFLAG value"
rule is going to be broken, I think that logical evaluation is a
much safer practice for these types of comparisons.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

  reply	other threads:[~2015-01-29 23:54 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-27  3:14 [PATCH 0/9] xfs: xfs_ioctl_setxattr rework Dave Chinner
2015-01-27  3:14 ` [PATCH 1/9] xfs: FSX_NONBLOCK is not used Dave Chinner
2015-01-29 15:33   ` Brian Foster
2015-01-27  3:14 ` [PATCH 2/9] xfs: separate xflags from xfs_ioctl_setattr Dave Chinner
2015-01-29 15:34   ` Brian Foster
2015-01-27  3:14 ` [PATCH 3/9] xfs: factor out xfs_ioctl_setattr transaciton preamble Dave Chinner
2015-01-29 15:34   ` Brian Foster
2015-01-27  3:14 ` [PATCH 4/9] xfs: disaggregate xfs_ioctl_setattr Dave Chinner
2015-01-29 15:34   ` Brian Foster
2015-01-27  3:14 ` [PATCH 5/9] xfs: kill xfs_ioctl_setattr behaviour mask Dave Chinner
2015-01-29 15:35   ` Brian Foster
2015-01-27  3:14 ` [PATCH 6/9] xfs: XFS_IOCTL_SETXATTR can run in user namespaces Dave Chinner
2015-01-29 15:35   ` Brian Foster
2015-01-29 23:53     ` Dave Chinner [this message]
2015-01-30  3:04       ` Brian Foster
2015-01-30  7:44         ` Dave Chinner
2015-01-27  3:14 ` [PATCH 7/9] xfs; factor extsize hint checking out of xfs_ioctl_setattr Dave Chinner
2015-01-29 15:35   ` Brian Foster
2015-01-27  3:14 ` [PATCH 8/9] xfs; factor projid " Dave Chinner
2015-01-29 15:35   ` Brian Foster
2015-01-27  3:14 ` [PATCH 9/9] xfs: fix behaviour of XFS_IOC_FSSETXATTR on directories Dave Chinner
2015-01-29 15:35   ` Brian Foster
2015-01-29 15:38 ` [PATCH 0/9] xfs: xfs_ioctl_setxattr rework Brian Foster

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150129235315.GG6282@dastard \
    --to=david@fromorbit.com \
    --cc=bfoster@redhat.com \
    --cc=iustin@k1024.org \
    --cc=xfs@oss.sgi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.