* [PATCH v3 1/5] xfs: fix capability check in xfs [not found] <20260702093324.127450-1-cem@kernel.org> @ 2026-07-02 9:33 ` cem 2026-07-02 10:30 ` Christoph Hellwig 0 siblings, 1 reply; 6+ messages in thread From: cem @ 2026-07-02 9:33 UTC (permalink / raw) To: cem Cc: stable, Jan Kara, Christoph Hellwig, Serge E. Hallyn, Darrick J. Wong, Dave Chinner, Eric Sandeen, linux-xfs, linux-fsdevel, linux-security-module, linux-kernel, Dr. Thomas Orgis 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: Jan Kara <jack@suse.cz> Cc: Christoph Hellwig <hch@lst.de> Cc: Serge E. Hallyn <serge@hallyn.com> Cc: Darrick J. Wong <djwong@kernel.org> Cc: Dave Chinner <david@fromorbit.com> Cc: Eric Sandeen <sandeen@redhat.com> Cc: Dr. Thomas Orgis" <thomas.orgis@uni-hamburg.de> Cc: linux-xfs@vger.kernel.org Cc: linux-fsdevel@vger.kernel.org Cc: linux-security-module@vger.kernel.org Cc: linux-kernel@vger.kernel.org Reported-by: Dr. Thomas Orgis <thomas.orgis@uni-hamburg.de> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org> --- 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 1b53701bebea..1a8af827dde1 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 6339f4956ecb..205fe2dae732 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); if (error) goto out_dqrele; -- 2.54.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/5] xfs: fix capability check in xfs 2026-07-02 9:33 ` [PATCH v3 1/5] xfs: fix capability check in xfs cem @ 2026-07-02 10:30 ` Christoph Hellwig 2026-07-02 11:17 ` Carlos Maiolino 0 siblings, 1 reply; 6+ messages in thread From: Christoph Hellwig @ 2026-07-02 10:30 UTC (permalink / raw) To: cem Cc: stable, Jan Kara, Christoph Hellwig, Serge E. Hallyn, Darrick J. Wong, Dave Chinner, Eric Sandeen, linux-xfs, linux-fsdevel, linux-security-module, linux-kernel, Dr. Thomas Orgis On Thu, Jul 02, 2026 at 11:33:17AM +0200, cem@kernel.org wrote: > index 6339f4956ecb..205fe2dae732 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), Extra indentation and an overly long line caused by that here. Otherwise looks good. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/5] xfs: fix capability check in xfs 2026-07-02 10:30 ` Christoph Hellwig @ 2026-07-02 11:17 ` Carlos Maiolino 2026-07-02 11:24 ` Christoph Hellwig 0 siblings, 1 reply; 6+ messages in thread From: Carlos Maiolino @ 2026-07-02 11:17 UTC (permalink / raw) To: Christoph Hellwig Cc: stable, Jan Kara, Serge E. Hallyn, Darrick J. Wong, Dave Chinner, Eric Sandeen, linux-xfs, linux-fsdevel, linux-security-module, linux-kernel, Dr. Thomas Orgis On Thu, Jul 02, 2026 at 12:30:52PM +0200, Christoph Hellwig wrote: > On Thu, Jul 02, 2026 at 11:33:17AM +0200, cem@kernel.org wrote: > > index 6339f4956ecb..205fe2dae732 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), > Thanks, I tried to keep the parameters aligned, but I can bring it one tab back. Do you mind if I fix it at commit time if -unlikely- no other change is required? This is what it will look like: 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 indentation and an overly long line caused by that here. > > Otherwise looks good. > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/5] xfs: fix capability check in xfs 2026-07-02 11:17 ` Carlos Maiolino @ 2026-07-02 11:24 ` Christoph Hellwig 2026-07-02 12:11 ` Carlos Maiolino 2026-07-02 12:24 ` Carlos Maiolino 0 siblings, 2 replies; 6+ messages in thread From: Christoph Hellwig @ 2026-07-02 11:24 UTC (permalink / raw) To: Carlos Maiolino Cc: Christoph Hellwig, stable, Jan Kara, Serge E. Hallyn, Darrick J. Wong, Dave Chinner, Eric Sandeen, linux-xfs, linux-fsdevel, linux-security-module, linux-kernel, Dr. Thomas Orgis On Thu, Jul 02, 2026 at 01:17:29PM +0200, Carlos Maiolino wrote: > On Thu, Jul 02, 2026 at 12:30:52PM +0200, Christoph Hellwig wrote: > > On Thu, Jul 02, 2026 at 11:33:17AM +0200, cem@kernel.org wrote: > > > index 6339f4956ecb..205fe2dae732 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), > > > > Thanks, I tried to keep the parameters aligned, but I can bring it one > tab back. Do you mind if I fix it at commit time if -unlikely- no other > change is required? > > This is what it will look like: > > 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); This still adds an extra tab. Like much (but not all) of the kernel we use two-tabs by default, which is also in the other two hinks. This now adds a third. Just keep it as it was: error = xfs_trans_alloc_ichange(ip, udqp, gdqp, NULL, ns_capable_noaudit(&init_user_ns, CAP_FOWNER), &tp); ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/5] xfs: fix capability check in xfs 2026-07-02 11:24 ` Christoph Hellwig @ 2026-07-02 12:11 ` Carlos Maiolino 2026-07-02 12:24 ` Carlos Maiolino 1 sibling, 0 replies; 6+ messages in thread From: Carlos Maiolino @ 2026-07-02 12:11 UTC (permalink / raw) To: Christoph Hellwig Cc: stable, Jan Kara, Serge E. Hallyn, Darrick J. Wong, Dave Chinner, Eric Sandeen, linux-xfs, linux-fsdevel, linux-security-module, linux-kernel, Dr. Thomas Orgis On Thu, Jul 02, 2026 at 01:24:38PM +0200, Christoph Hellwig wrote: > On Thu, Jul 02, 2026 at 01:17:29PM +0200, Carlos Maiolino wrote: > > On Thu, Jul 02, 2026 at 12:30:52PM +0200, Christoph Hellwig wrote: > > > On Thu, Jul 02, 2026 at 11:33:17AM +0200, cem@kernel.org wrote: > > > > index 6339f4956ecb..205fe2dae732 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), > > > > > > > Thanks, I tried to keep the parameters aligned, but I can bring it one > > tab back. Do you mind if I fix it at commit time if -unlikely- no other > > change is required? > > > > This is what it will look like: > > > > 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); > > This still adds an extra tab. Like much (but not all) of the kernel > we use two-tabs by default, which is also in the other two hinks. This > now adds a third. Just keep it as it was: > > error = xfs_trans_alloc_ichange(ip, udqp, gdqp, NULL, > ns_capable_noaudit(&init_user_ns, CAP_FOWNER), &tp); > > Ok, will do! ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/5] xfs: fix capability check in xfs 2026-07-02 11:24 ` Christoph Hellwig 2026-07-02 12:11 ` Carlos Maiolino @ 2026-07-02 12:24 ` Carlos Maiolino 1 sibling, 0 replies; 6+ messages in thread From: Carlos Maiolino @ 2026-07-02 12:24 UTC (permalink / raw) To: Christoph Hellwig Cc: stable, Jan Kara, Serge E. Hallyn, Darrick J. Wong, Dave Chinner, Eric Sandeen, linux-xfs, linux-fsdevel, linux-security-module, linux-kernel, Dr. Thomas Orgis > > + ns_capable_noaudit(&init_user_ns, CAP_FOWNER), > > + &tp); > > This still adds an extra tab. Like much (but not all) of the kernel > we use two-tabs by default, which is also in the other two hinks. This > now adds a third. Just keep it as it was: > > error = xfs_trans_alloc_ichange(ip, udqp, gdqp, NULL, > ns_capable_noaudit(&init_user_ns, CAP_FOWNER), &tp); > > FWIW, I also fixed these in the patch 4 which I had screwed up too :) ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-02 12:24 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20260702093324.127450-1-cem@kernel.org>
2026-07-02 9:33 ` [PATCH v3 1/5] xfs: fix capability check in xfs cem
2026-07-02 10:30 ` Christoph Hellwig
2026-07-02 11:17 ` Carlos Maiolino
2026-07-02 11:24 ` Christoph Hellwig
2026-07-02 12:11 ` Carlos Maiolino
2026-07-02 12:24 ` Carlos Maiolino
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox