[PATCHBOMB] xfsprogs: last few fixes for 6.14

[PATCHBOMB] xfsprogs: last few fixes for 6.14

[Darrick J. Wong]

Hi everyone,

This is a collection of fixes for bugs that I stumbled over during the
6.14 QA cycle; and a libxfs sync for a cleanup that went in after -rc6.

--D

[PATCHSET 1/2] xfsprogs: new libxfs code from kernel 6.14

[Darrick J. Wong]

Hi all,

Port kernel libxfs code to userspace.

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

This has been running on the djcloud for months with no problems.  Enjoy!
Comments and questions are, as always, welcome.

--D

xfsprogs git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=libxfs-sync-6.14
---
Commits in this patchset:
 * xfs: Use abs_diff instead of XFS_ABSDIFF
---
 include/platform_defs.h |   19 +++++++++++++++++++
 libxfs/xfs_alloc.c      |    8 +++-----
 2 files changed, 22 insertions(+), 5 deletions(-)

[PATCHSET 2/2] xfsprogs: random bug fixes

[Darrick J. Wong]

Hi all,

Here's a pile of assorted bug fixes from around the codebase.

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_repair: don't recreate /quota metadir if there are no quota inodes
 * xfs_repair: fix crash in reset_rt_metadir_inodes
 * xfs_repair: fix infinite loop in longform_dir2_entry_check*
 * xfs_repair: fix stupid argument error in verify_inode_chunk
---
 libxfs/libxfs_api_defs.h |    1 +
 repair/dino_chunks.c     |    2 +-
 repair/phase6.c          |   41 +++++++++++++++++++++++++++++++++++++----
 3 files changed, 39 insertions(+), 5 deletions(-)

[PATCH 1/1] xfs: Use abs_diff instead of XFS_ABSDIFF

[Darrick J. Wong]

From: Matthew Wilcox (Oracle) <willy@infradead.org>

Source kernel commit: ca3ac4bf4dc307cea5781dccccf41c1d14c2f82f

We have a central definition for this function since 2023, used by
a number of different parts of the kernel.

Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org>
Reviewed-by: Carlos Maiolino <cmaiolino@redhat.com>
Reviewed-by: Eric Sandeen <sandeen@redhat.com>
Signed-off-by: Carlos Maiolino <cem@kernel.org>
---
 include/platform_defs.h |   19 +++++++++++++++++++
 libxfs/xfs_alloc.c      |    8 +++-----
 2 files changed, 22 insertions(+), 5 deletions(-)


diff --git a/include/platform_defs.h b/include/platform_defs.h
index 051ee25a5b4fea..9af7b4318f8917 100644
--- a/include/platform_defs.h
+++ b/include/platform_defs.h
@@ -261,4 +261,23 @@ static inline bool __must_check __must_check_overflow(bool overflow)
 	__builtin_add_overflow(__a, __b, __d);	\
 }))
 
+/**
+ * abs_diff - return absolute value of the difference between the arguments
+ * @a: the first argument
+ * @b: the second argument
+ *
+ * @a and @b have to be of the same type. With this restriction we compare
+ * signed to signed and unsigned to unsigned. The result is the subtraction
+ * the smaller of the two from the bigger, hence result is always a positive
+ * value.
+ *
+ * Return: an absolute value of the difference between the @a and @b.
+ */
+#define abs_diff(a, b) ({			\
+	typeof(a) __a = (a);			\
+	typeof(b) __b = (b);			\
+	(void)(&__a == &__b);			\
+	__a > __b ? (__a - __b) : (__b - __a);	\
+})
+
 #endif	/* __XFS_PLATFORM_DEFS_H__ */
diff --git a/libxfs/xfs_alloc.c b/libxfs/xfs_alloc.c
index 9aebe7227a6148..6675be78a7dae8 100644
--- a/libxfs/xfs_alloc.c
+++ b/libxfs/xfs_alloc.c
@@ -29,8 +29,6 @@ struct kmem_cache	*xfs_extfree_item_cache;
 
 struct workqueue_struct *xfs_alloc_wq;
 
-#define XFS_ABSDIFF(a,b)	(((a) <= (b)) ? ((b) - (a)) : ((a) - (b)))
-
 #define	XFSA_FIXUP_BNO_OK	1
 #define	XFSA_FIXUP_CNT_OK	2
 
@@ -406,8 +404,8 @@ xfs_alloc_compute_diff(
 		if (newbno1 != NULLAGBLOCK && newbno2 != NULLAGBLOCK) {
 			if (newlen1 < newlen2 ||
 			    (newlen1 == newlen2 &&
-			     XFS_ABSDIFF(newbno1, wantbno) >
-			     XFS_ABSDIFF(newbno2, wantbno)))
+			     abs_diff(newbno1, wantbno) >
+			     abs_diff(newbno2, wantbno)))
 				newbno1 = newbno2;
 		} else if (newbno2 != NULLAGBLOCK)
 			newbno1 = newbno2;
@@ -423,7 +421,7 @@ xfs_alloc_compute_diff(
 	} else
 		newbno1 = freeend - wantlen;
 	*newbnop = newbno1;
-	return newbno1 == NULLAGBLOCK ? 0 : XFS_ABSDIFF(newbno1, wantbno);
+	return newbno1 == NULLAGBLOCK ? 0 : abs_diff(newbno1, wantbno);
 }
 
 /*

[PATCH 1/4] xfs_repair: don't recreate /quota metadir if there are no quota inodes

[Darrick J. Wong]

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

If repair does not discover even a single quota file, then don't have it
try to create a /quota metadir to hold them.  This avoids pointless
repair failures on quota-less filesystems that are nearly full.

Found via generic/558 on a zoned=1 filesystem.

Cc: <linux-xfs@vger.kernel.org> # v6.13.0
Fixes: b790ab2a303d58 ("xfs_repair: support quota inodes in the metadata directory")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 repair/phase6.c |    5 +++++
 1 file changed, 5 insertions(+)


diff --git a/repair/phase6.c b/repair/phase6.c
index 4064a84b24509f..2d526dda484293 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -3538,6 +3538,11 @@ reset_quota_metadir_inodes(
 	struct xfs_inode	*dp = NULL;
 	int			error;
 
+	if (!has_quota_inode(XFS_DQTYPE_USER) &&
+	    !has_quota_inode(XFS_DQTYPE_GROUP) &&
+	    !has_quota_inode(XFS_DQTYPE_PROJ))
+		return;
+
 	error = -libxfs_dqinode_mkdir_parent(mp, &dp);
 	if (error)
 		do_error(_("failed to create quota metadir (%d)\n"),

[PATCH 2/4] xfs_repair: fix crash in reset_rt_metadir_inodes

[Darrick J. Wong]

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

I observed that xfs_repair -n segfaults during xfs/812 after corrupting
the /rtgroups metadir inode because mp->m_rtdirip isn't loaded.  Fix the
crash and print a warning about the missing inode.

Cc: <linux-xfs@vger.kernel.org> # v6.13.0
Fixes: 7c541c90fd77a2 ("xfs_repair: support realtime groups")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 repair/phase6.c |   14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)


diff --git a/repair/phase6.c b/repair/phase6.c
index 2d526dda484293..c16164c171d07d 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -3397,15 +3397,21 @@ reset_rt_metadir_inodes(
 		unload_rtgroup_inodes(mp);
 
 	if (mp->m_sb.sb_rgcount > 0) {
-		if (!no_modify) {
+		if (no_modify) {
+			if (!mp->m_rtdirip)
+				do_warn(_("would recreate realtime metadir\n"));
+		} else {
 			error = -libxfs_rtginode_mkdir_parent(mp);
 			if (error)
 				do_error(_("failed to create realtime metadir (%d)\n"),
 					error);
 		}
-		mark_ino_inuse(mp, mp->m_rtdirip->i_ino, S_IFDIR,
-				mp->m_metadirip->i_ino);
-		mark_ino_metadata(mp, mp->m_rtdirip->i_ino);
+
+		if (mp->m_rtdirip) {
+			mark_ino_inuse(mp, mp->m_rtdirip->i_ino, S_IFDIR,
+					mp->m_metadirip->i_ino);
+			mark_ino_metadata(mp, mp->m_rtdirip->i_ino);
+		}
 	}
 
 	/*

[PATCH 3/4] xfs_repair: fix infinite loop in longform_dir2_entry_check*

[Darrick J. Wong]

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

If someone corrupts the data fork of a directory to have a bmap record
whose br_startoff only has bits set in the upper 32 bits, the code will
suffer an integer overflow when assigning the 64-bit next_da_bno to the
32-bit da_bno.  This leads to an infinite loop.

Found by fuzzing xfs/812 with u3.bmx[0].startoff = firstbit.

Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 libxfs/libxfs_api_defs.h |    1 +
 repair/phase6.c          |   22 ++++++++++++++++++++++
 2 files changed, 23 insertions(+)


diff --git a/libxfs/libxfs_api_defs.h b/libxfs/libxfs_api_defs.h
index 14a67c8c24dd7e..dcb5dec0a7abd2 100644
--- a/libxfs/libxfs_api_defs.h
+++ b/libxfs/libxfs_api_defs.h
@@ -399,6 +399,7 @@
 #define xfs_verify_agbext		libxfs_verify_agbext
 #define xfs_verify_agino		libxfs_verify_agino
 #define xfs_verify_cksum		libxfs_verify_cksum
+#define xfs_verify_dablk		libxfs_verify_dablk
 #define xfs_verify_dir_ino		libxfs_verify_dir_ino
 #define xfs_verify_fsbext		libxfs_verify_fsbext
 #define xfs_verify_fsbno		libxfs_verify_fsbno
diff --git a/repair/phase6.c b/repair/phase6.c
index c16164c171d07d..44b9bfc3b7e69f 100644
--- a/repair/phase6.c
+++ b/repair/phase6.c
@@ -2169,6 +2169,13 @@ longform_dir2_check_node(
 		if (bmap_next_offset(ip, &next_da_bno))
 			break;
 
+		if (next_da_bno != NULLFILEOFF &&
+		    !libxfs_verify_dablk(mp, next_da_bno)) {
+			do_warn(_("invalid dir leaf block 0x%llx\n"),
+					(unsigned long long)next_da_bno);
+			return 1;
+		}
+
 		/*
 		 * we need to use the da3 node verifier here as it handles the
 		 * fact that reading the leaf hash tree blocks can return either
@@ -2244,6 +2251,13 @@ longform_dir2_check_node(
 		if (bmap_next_offset(ip, &next_da_bno))
 			break;
 
+		if (next_da_bno != NULLFILEOFF &&
+		    !libxfs_verify_dablk(mp, next_da_bno)) {
+			do_warn(_("invalid dir free block 0x%llx\n"),
+					(unsigned long long)next_da_bno);
+			return 1;
+		}
+
 		error = dir_read_buf(ip, da_bno, &bp, &xfs_dir3_free_buf_ops,
 				&fixit);
 		if (error) {
@@ -2379,6 +2393,14 @@ longform_dir2_entry_check(
 			break;
 		}
 
+		if (next_da_bno != NULLFILEOFF &&
+		    !libxfs_verify_dablk(mp, next_da_bno)) {
+			do_warn(_("invalid dir data block 0x%llx\n"),
+					(unsigned long long)next_da_bno);
+			fixit++;
+			goto out_fix;
+		}
+
 		if (fmt == XFS_DIR2_FMT_BLOCK)
 			ops = &xfs_dir3_block_buf_ops;
 		else

[PATCH 4/4] xfs_repair: fix stupid argument error in verify_inode_chunk

[Darrick J. Wong]

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

An arm64 VM running fstests with 64k fsblock size blew up the test
filesystem when the OOM killer whacked xfs_repair as it was rebuilding a
sample filesystem.  A subsequent attempt by fstests to repair the
filesystem printed stuff like this:

inode rec for ino 39144576 (1/5590144) overlaps existing rec (start 1/5590144)
inode rec for ino 39144640 (1/5590208) overlaps existing rec (start 1/5590208)

followed by a lot of errors such as:

cannot read agbno (1/5590208), disk block 734257664
xfs_repair: error - read only 0 of 65536 bytes

Here we're feeding per-AG inode numbers into a block reading function as
if it were a per-AG block number.  This is wrong by a factor of 128x so
we read past the end of the filesystem.  Worse yet, the buffer cache
fills up memory and thus the second repair process is also OOM killed.
The filesystem is not fixed.

Cc: <linux-xfs@vger.kernel.org> # v3.1.8
Fixes: 0553a94f522c17 ("repair: kill check_inode_block")
Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
---
 repair/dino_chunks.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


diff --git a/repair/dino_chunks.c b/repair/dino_chunks.c
index 250985ec264ead..932eaf63f4741f 100644
--- a/repair/dino_chunks.c
+++ b/repair/dino_chunks.c
@@ -132,7 +132,7 @@ verify_inode_chunk(xfs_mount_t		*mp,
 	if (igeo->ialloc_blks == 1)  {
 		if (agbno > max_agbno)
 			return 0;
-		if (check_aginode_block(mp, agno, agino) == 0)
+		if (check_aginode_block(mp, agno, agbno) == 0)
 			return 0;
 
 		lock_ag(agno);

Re: [PATCH 1/4] xfs_repair: don't recreate /quota metadir if there are no quota inodes

[Andrey Albershteyn]

On 2025-03-21 09:31:31, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> If repair does not discover even a single quota file, then don't have it
> try to create a /quota metadir to hold them.  This avoids pointless
> repair failures on quota-less filesystems that are nearly full.
> 
> Found via generic/558 on a zoned=1 filesystem.
> 
> Cc: <linux-xfs@vger.kernel.org> # v6.13.0
> Fixes: b790ab2a303d58 ("xfs_repair: support quota inodes in the metadata directory")
> Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> ---
>  repair/phase6.c |    5 +++++
>  1 file changed, 5 insertions(+)
> 
> 
> diff --git a/repair/phase6.c b/repair/phase6.c
> index 4064a84b24509f..2d526dda484293 100644
> --- a/repair/phase6.c
> +++ b/repair/phase6.c
> @@ -3538,6 +3538,11 @@ reset_quota_metadir_inodes(
>  	struct xfs_inode	*dp = NULL;
>  	int			error;
>  
> +	if (!has_quota_inode(XFS_DQTYPE_USER) &&
> +	    !has_quota_inode(XFS_DQTYPE_GROUP) &&
> +	    !has_quota_inode(XFS_DQTYPE_PROJ))
> +		return;
> +
>  	error = -libxfs_dqinode_mkdir_parent(mp, &dp);
>  	if (error)
>  		do_error(_("failed to create quota metadir (%d)\n"),
> 

LGTM
Reviewed-by: Andrey Albershteyn <aalbersh@kernel.org>

-- 
- Andrey

Re: [PATCH 1/4] xfs_repair: don't recreate /quota metadir if there are no quota inodes

[Christoph Hellwig]

On Fri, Mar 21, 2025 at 09:31:31AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> If repair does not discover even a single quota file, then don't have it
> try to create a /quota metadir to hold them.  This avoids pointless
> repair failures on quota-less filesystems that are nearly full.
> 
> Found via generic/558 on a zoned=1 filesystem.

Interesting, I never saw that fail.  Any interesting options you had
to inject for that?

The patch itself looks fine, though:

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

Re: [PATCH 2/4] xfs_repair: fix crash in reset_rt_metadir_inodes

[Christoph Hellwig]

On Fri, Mar 21, 2025 at 09:31:46AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> I observed that xfs_repair -n segfaults during xfs/812 after corrupting
> the /rtgroups metadir inode because mp->m_rtdirip isn't loaded.  Fix the
> crash and print a warning about the missing inode.

Looks good:

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

Re: [PATCH 3/4] xfs_repair: fix infinite loop in longform_dir2_entry_check*

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 4/4] xfs_repair: fix stupid argument error in verify_inode_chunk

[Christoph Hellwig]

Looks good:

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

Re: [PATCH 1/4] xfs_repair: don't recreate /quota metadir if there are no quota inodes

[Darrick J. Wong]

On Sat, Mar 22, 2025 at 11:33:42PM -0700, Christoph Hellwig wrote:
> On Fri, Mar 21, 2025 at 09:31:31AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > If repair does not discover even a single quota file, then don't have it
> > try to create a /quota metadir to hold them.  This avoids pointless
> > repair failures on quota-less filesystems that are nearly full.
> > 
> > Found via generic/558 on a zoned=1 filesystem.
> 
> Interesting, I never saw that fail.  Any interesting options you had
> to inject for that?

No, just use a pathetically small device to try to pound on ENOSPC
harder. ;)

--D

> The patch itself looks fine, though:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> 
>