[PATCHSET 1/2] xfsprogs: various bug fixes for 7.3

[PATCHSET 1/2] xfsprogs: various bug fixes for 7.3

[Darrick J. Wong]

Hi all,

This series contains miscellaneous bugfixes.

If you're going to start using this code, I strongly recommend pulling
from my git trees, which are linked below.

With a bit of luck, this should all go splendidly.
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=random-fixes

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=random-fixes

fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=random-fixes
---
Commits in this patchset:
 * xfs_scrub: terminate systemd services if the sysadmin unmounts
 * mkfs: allow disabling quota flags
 * xfs_quota: fix XFS_GETQSTAT parameter pointer
---
 mkfs/xfs_mkfs.c                   |   42 ++++++++++++++++++++++++++-----------
 quota/state.c                     |    2 +-
 scrub/xfs_scrub@.service.in       |    3 +++
 scrub/xfs_scrub_media@.service.in |    3 +++
 4 files changed, 37 insertions(+), 13 deletions(-)

[PATCH 1/3] xfs_scrub: terminate systemd services if the sysadmin unmounts

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

We should make sure that any systemd services started on behalf of an
XFS mount are torn down when the mount goes away.  On a modern system,
calling umount(8) while there's an xfs_scrub service running on that
mount point will successfully remove the mount from the caller's mount
namespace, but the filesystem remains mounted in the service's private
mount namespace, with the effect that the filesystem stays mounted.
This is probably not what the user wants.

Per cks' blog, the way to fix this is through a BindTo= clause.

Note: This isn't needed for xfs_healer because it only maintains a soft
reference to the mounted filesystem and can reconnect to a filesystem if
its mounts move.

Link: https://utcc.utoronto.ca/~cks/space/blog/linux/SystemdStopOnFSUnmount
Cc: <linux-xfs@vger.kernel.org> # v6.10.0
Fixes: 0109639eede4dc ("xfs_scrub: tighten up the security on the background systemd service")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Andrey Albershteyn <aalbersh@kernel.org>
---
 scrub/xfs_scrub@.service.in       |    3 +++
 scrub/xfs_scrub_media@.service.in |    3 +++
 2 files changed, 6 insertions(+)


diff --git a/scrub/xfs_scrub@.service.in b/scrub/xfs_scrub@.service.in
index fb38319e95c148..3e4e79fe6a1913 100644
--- a/scrub/xfs_scrub@.service.in
+++ b/scrub/xfs_scrub@.service.in
@@ -19,6 +19,9 @@ ConditionCapability=CAP_SYS_RAWIO
 ConditionPathIsMountPoint=%f
 RequiresMountsFor=%f
 
+# Terminate service if someone unmounts the filesystem
+BindsTo=%i.mount
+
 [Service]
 Type=oneshot
 Environment=SERVICE_MODE=1
diff --git a/scrub/xfs_scrub_media@.service.in b/scrub/xfs_scrub_media@.service.in
index 98cd1ac44fbebd..954b8e92a1ce53 100644
--- a/scrub/xfs_scrub_media@.service.in
+++ b/scrub/xfs_scrub_media@.service.in
@@ -19,6 +19,9 @@ ConditionCapability=CAP_SYS_RAWIO
 ConditionPathIsMountPoint=%f
 RequiresMountsFor=%f
 
+# Terminate service if someone unmounts the filesystem
+BindsTo=%i.mount
+
 [Service]
 Type=oneshot
 Environment=SERVICE_MODE=1

Re: [PATCH 1/3] xfs_scrub: terminate systemd services if the sysadmin unmounts

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 2/3] mkfs: allow disabling quota flags

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

LOLLM noticed that with the new defaults= option, it's possible for the
mkfs input validation code to allow the defaults file to turn on quotas
and a cli option to turn off quotas.  However, the qflags modification
code doesn't clear flags correctly because previously it didn't have to;
qflags is zero by default, so uquota=0 wouldn't change anything.

Fix this by introducing a little more logic to clear a mask of quota
flags and (if the value is 1) OR'ing a flag back in.

Cc: <linux-xfs@vger.kernel.org> # v6.13.0
Fixes: 525f826429a868 ("mkfs: add quota flags when setting up filesystem")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # finding obvious bugs
---
 mkfs/xfs_mkfs.c |   42 ++++++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 12 deletions(-)


diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
index 334367b81555ec..aa87951db0d041 100644
--- a/mkfs/xfs_mkfs.c
+++ b/mkfs/xfs_mkfs.c
@@ -2055,6 +2055,21 @@ log_opts_parser(
 	return 0;
 }
 
+static void
+toggle_qflag(
+	struct opt_params	*opts,
+	int			subopt,
+	const char		*value,
+	struct cli_params	*cli,
+	uint16_t		flag,
+	uint16_t		mask)
+{
+	cli->sb_feat.qflags &= ~mask;
+
+	if (getnum(value, opts, subopt))
+		cli->sb_feat.qflags |= flag;
+}
+
 static int
 meta_opts_parser(
 	struct opt_params	*opts,
@@ -2107,28 +2122,31 @@ meta_opts_parser(
 		cli->sb_feat.metadir = getnum(value, opts, subopt);
 		break;
 	case M_UQUOTA:
-		if (getnum(value, opts, subopt))
-			cli->sb_feat.qflags |= XFS_UQUOTA_ACCT | XFS_UQUOTA_ENFD;
+		toggle_qflag(opts, subopt, value, cli,
+				XFS_UQUOTA_ACCT | XFS_UQUOTA_ENFD,
+				XFS_UQUOTA_ACCT | XFS_UQUOTA_ENFD);
 		break;
 	case M_GQUOTA:
-		if (getnum(value, opts, subopt))
-			cli->sb_feat.qflags |= XFS_GQUOTA_ACCT | XFS_GQUOTA_ENFD;
+		toggle_qflag(opts, subopt, value, cli,
+				XFS_GQUOTA_ACCT | XFS_GQUOTA_ENFD,
+				XFS_GQUOTA_ACCT | XFS_GQUOTA_ENFD);
 		break;
 	case M_PQUOTA:
-		if (getnum(value, opts, subopt))
-			cli->sb_feat.qflags |= XFS_PQUOTA_ACCT | XFS_PQUOTA_ENFD;
+		toggle_qflag(opts, subopt, value, cli,
+				XFS_PQUOTA_ACCT | XFS_PQUOTA_ENFD,
+				XFS_PQUOTA_ACCT | XFS_PQUOTA_ENFD);
 		break;
 	case M_UQNOENFORCE:
-		if (getnum(value, opts, subopt))
-			cli->sb_feat.qflags |= XFS_UQUOTA_ACCT;
+		toggle_qflag(opts, subopt, value, cli, XFS_UQUOTA_ACCT,
+				XFS_UQUOTA_ACCT | XFS_UQUOTA_ENFD);
 		break;
 	case M_GQNOENFORCE:
-		if (getnum(value, opts, subopt))
-			cli->sb_feat.qflags |= XFS_GQUOTA_ACCT;
+		toggle_qflag(opts, subopt, value, cli, XFS_GQUOTA_ACCT,
+				XFS_GQUOTA_ACCT | XFS_GQUOTA_ENFD);
 		break;
 	case M_PQNOENFORCE:
-		if (getnum(value, opts, subopt))
-			cli->sb_feat.qflags |= XFS_PQUOTA_ACCT;
+		toggle_qflag(opts, subopt, value, cli, XFS_PQUOTA_ACCT,
+				XFS_PQUOTA_ACCT | XFS_PQUOTA_ENFD);
 		break;
 	default:
 		return -EINVAL;

Re: [PATCH 2/3] mkfs: allow disabling quota flags

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 3/3] xfs_quota: fix XFS_GETQSTAT parameter pointer

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

LOLLM noticed that XFS_GETQSTAT is supposed to take a pointer to a
struct fs_quota_stat, not a struct fs_quota_statv.  However, quotactl
call throws away all such nuance by accepting any void pointer, so this
got broken by the recent refactoring.  Fix all that.

Cc: <linux-xfs@vger.kernel.org> # v7.2.0
Fixes: 500da05860c792 ("libfrog: try to pass struct fs_path objects to quotactl wrapper")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
Assisted-by: LOLLM # Finding stupid bugs
---
 quota/state.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/quota/state.c b/quota/state.c
index 3679c247ea6204..239b513125518d 100644
--- a/quota/state.c
+++ b/quota/state.c
@@ -206,7 +206,7 @@ state_quotafile_stat(
 
 	ret = xfrog_quotactl(mount, XFS_GETQSTATV, type, 0, sv);
 	if (ret < 0) {
-		ret = xfrog_quotactl(mount, XFS_GETQSTAT, type, 0, sv);
+		ret = xfrog_quotactl(mount, XFS_GETQSTAT, type, 0, s);
 		if (ret < 0) {
 			if (flags & VERBOSE_FLAG)
 				fprintf(fp,

Re: [PATCH 3/3] xfs_quota: fix XFS_GETQSTAT parameter pointer

[Christoph Hellwig]

On Sun, Sep 20, 2026 at 11:14:27PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> LOLLM noticed that XFS_GETQSTAT is supposed to take a pointer to a
> struct fs_quota_stat, not a struct fs_quota_statv.  However, quotactl
> call throws away all such nuance by accepting any void pointer, so this
> got broken by the recent refactoring.  Fix all that.

Heh.  Should we replace xfrog_quotactl with typed variants for each
argument?

For now this fix itself looks good, though:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 3/3] xfs_quota: fix XFS_GETQSTAT parameter pointer

[Darrick J. Wong]

On Mon, Sep 21, 2026 at 01:29:17AM -0700, Christoph Hellwig wrote:
> On Sun, Sep 20, 2026 at 11:14:27PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > LOLLM noticed that XFS_GETQSTAT is supposed to take a pointer to a
> > struct fs_quota_stat, not a struct fs_quota_statv.  However, quotactl
> > call throws away all such nuance by accepting any void pointer, so this
> > got broken by the recent refactoring.  Fix all that.
> 
> Heh.  Should we replace xfrog_quotactl with typed variants for each
> argument?

That sounds like a good idea.  I think these are the only quota
commands?

#define Q_XQUOTAON	XQM_CMD(1)	/* enable accounting/enforcement */
#define Q_XQUOTAOFF	XQM_CMD(2)	/* disable accounting/enforcement */
#define Q_XGETQUOTA	XQM_CMD(3)	/* get disk limits and usage */
#define Q_XSETQLIM	XQM_CMD(4)	/* set disk limits */
#define Q_XGETQSTAT	XQM_CMD(5)	/* get quota subsystem status */
#define Q_XQUOTARM	XQM_CMD(6)	/* free disk space used by dquots */
#define Q_XQUOTASYNC	XQM_CMD(7)	/* delalloc flush, updates dquots */
#define Q_XGETQSTATV	XQM_CMD(8)	/* newer version of get quota */
#define Q_XGETNEXTQUOTA	XQM_CMD(9)	/* get disk limits and usage */

> For now this fix itself looks good, though:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Thanks!

--D