All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: cem@kernel.org
Cc: linux-xfs@vger.kernel.org, stable@vger.kernel.org,
	Eric Sandeen <sandeen@redhat.com>, Christoph Hellwig <hch@lst.de>,
	Jan Kara <jack@suse.cz>, Dave Chinner <david@fromorbit.com>,
	"Dr. Thomas Orgis" <thomas.orgis@uni-hamburg.de>
Subject: Re: [PATCH 1/2] xfs: fix capabily check in xfs
Date: Fri, 26 Jun 2026 07:49:34 -0700	[thread overview]
Message-ID: <20260626144934.GR6078@frogsfrogsfrogs> (raw)
In-Reply-To: <20260626102934.57834-2-cem@kernel.org>

Note: s/capabily/capability/ in the subject line

On Fri, Jun 26, 2026 at 12:29:24PM +0200, cem@kernel.org wrote:
> From: Carlos Maiolino <cem@kernel.org>
> 
> An user reported a bug where he managed to evade group's quota
> by changing a file's gid to a different group id the same user
> belonged to, even though quotas were enforced on both gids and the
> file's size was big enough to exceed the quota's hardlimit.
> 
> Commit eba0549bc7d1 replaced a capable() call by a
> has_capability_noaudit() to prevent unnecessary selinux audit messages.
> Turns out that both calls have slightly different semantics even though
> their documentation seems similar. Where in a nutshell:
> 
> capable() - Tests the task's effective credentials
> has_ns_capability_noaudit() - Tests the task's real credentials
> 
> This most of the time has no practical difference but in some cases like
> changing attrs (specifically group id in this case) through a NFS client
> this will allow the quota code to use XFS_QMOPT_FORCE_RES, effectively
> bypassing quota accounting checks.
> 
> Using instead ns_capable_noaudit() should fix this issue and prevent
> selinux audit messages.
> 
> This also fix the remaining calls to has_capability_noaudit()
> 
> Fixes: eba0549bc7d1 ("xfs: don't generate selinux audit messages for capability testing")
> Cc: <stable@vger.kernel.org> # v5.18
> Cc: Darrick J. Wong <djwong@kernel.org>
> Cc: Eric Sandeen <sandeen@redhat.com>
> Cc: Christoph Hellwig <hch@lst.de>
> Cc: Jan Kara <jack@suse.cz>
> Cc: Dave Chinner <david@fromorbit.com>
> Reported-by: Dr. Thomas Orgis <thomas.orgis@uni-hamburg.de>
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---
>  fs/xfs/xfs_fsmap.c | 2 +-
>  fs/xfs/xfs_ioctl.c | 2 +-
>  fs/xfs/xfs_iops.c  | 3 ++-
>  3 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/xfs/xfs_fsmap.c b/fs/xfs/xfs_fsmap.c
> index b6a3bc9f143c..7c79fbe0a74c 100644
> --- a/fs/xfs/xfs_fsmap.c
> +++ b/fs/xfs/xfs_fsmap.c
> @@ -1175,7 +1175,7 @@ xfs_getfsmap(
>  		return -EINVAL;
>  
>  	use_rmap = xfs_has_rmapbt(mp) &&
> -		   has_capability_noaudit(current, CAP_SYS_ADMIN);
> +		   ns_capable_noaudit(&init_user_ns, CAP_SYS_ADMIN);
>  	head->fmh_entries = 0;
>  
>  	/* Set up our device handlers. */
> diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
> index 96af6b62ce39..852ff2ab4531 100644
> --- a/fs/xfs/xfs_ioctl.c
> +++ b/fs/xfs/xfs_ioctl.c
> @@ -647,7 +647,7 @@ xfs_ioctl_setattr_get_trans(
>  		goto out_error;
>  
>  	error = xfs_trans_alloc_ichange(ip, NULL, NULL, pdqp,
> -			has_capability_noaudit(current, CAP_FOWNER), &tp);
> +			ns_capable_noaudit(&init_user_ns, CAP_FOWNER), &tp);
>  	if (error)
>  		goto out_error;
>  
> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
> index 325c2200c501..9db9ef1d8c3a 100644
> --- a/fs/xfs/xfs_iops.c
> +++ b/fs/xfs/xfs_iops.c
> @@ -835,7 +835,8 @@ xfs_setattr_nonsize(
>  	}
>  
>  	error = xfs_trans_alloc_ichange(ip, udqp, gdqp, NULL,
> -			has_capability_noaudit(current, CAP_FOWNER), &tp);
> +				ns_capable_noaudit(&init_user_ns, CAP_FOWNER),
> +				&tp);

Extra indenting of the second and third lines, but otherwise this looks
good to me.  With the indent fixed,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D


>  	if (error)
>  		goto out_dqrele;
>  
> -- 
> 2.54.0
> 
> 

  reply	other threads:[~2026-06-26 14:49 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-26 10:29 [PATCH 0/2] Fix capabilities check cem
2026-06-26 10:29 ` [PATCH 1/2] xfs: fix capabily check in xfs cem
2026-06-26 14:49   ` Darrick J. Wong [this message]
2026-06-26 10:29 ` [PATCH 2/2] xfs: lift quota capability check to xfs_trans_dqresv cem
2026-06-26 15:14   ` Darrick J. Wong

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=20260626144934.GR6078@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=cem@kernel.org \
    --cc=david@fromorbit.com \
    --cc=hch@lst.de \
    --cc=jack@suse.cz \
    --cc=linux-xfs@vger.kernel.org \
    --cc=sandeen@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=thomas.orgis@uni-hamburg.de \
    /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.