[PATCH v2] xfs: check directory data block header padding in scrub

[PATCH v2] xfs: check directory data block header padding in scrub

[Yuto Ohnuki]

The pad field in xfs_dir3_data_hdr exists for 64-bit alignment and
should always be zero.

First, fix xfs_dir3_data_init to zero the full xfs_dir3_data_hdr instead
of just xfs_dir3_blk_hdr so that the pad field is covered by the memset.

Then, add the missing scrub check for the pad field in directory data
block headers. Since old kernels may have written non-zero padding
without issue, flag this as an optimization opportunity (preen) rather
than corruption. Add xchk_fblock_set_preen helper for this purpose.

Signed-off-by: Yuto Ohnuki <ytohnuki@amazon.com>
---
Changes in v2:
- Fix xfs_dir3_data_init to zero the full xfs_dir3_data_hdr instead of
  just xfs_dir3_blk_hdr so that the pad field is covered by the memset.
- Use xchk_fblock_set_preen instead of xchk_fblock_set_corrupt since
  old kernels may have written non-zero padding without issues.
- Add xchk_fblock_set_preen helper function.
- Link to v1: https://lore.kernel.org/all/20260404125032.37693-2-ytohnuki@amazon.com/#t
---
 fs/xfs/libxfs/xfs_dir2_data.c | 10 +++++-----
 fs/xfs/scrub/common.c         | 11 +++++++++++
 fs/xfs/scrub/common.h         |  2 ++
 fs/xfs/scrub/dir.c            |  7 ++++++-
 4 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
index 80ba94f51e5c..037f9449835d 100644
--- a/fs/xfs/libxfs/xfs_dir2_data.c
+++ b/fs/xfs/libxfs/xfs_dir2_data.c
@@ -745,13 +745,13 @@ xfs_dir3_data_init(
 	 */
 	hdr = bp->b_addr;
 	if (xfs_has_crc(mp)) {
-		struct xfs_dir3_blk_hdr *hdr3 = bp->b_addr;
+		struct xfs_dir3_data_hdr *hdr3 = bp->b_addr;
 
 		memset(hdr3, 0, sizeof(*hdr3));
-		hdr3->magic = cpu_to_be32(XFS_DIR3_DATA_MAGIC);
-		hdr3->blkno = cpu_to_be64(xfs_buf_daddr(bp));
-		hdr3->owner = cpu_to_be64(args->owner);
-		uuid_copy(&hdr3->uuid, &mp->m_sb.sb_meta_uuid);
+		hdr3->hdr.magic = cpu_to_be32(XFS_DIR3_DATA_MAGIC);
+		hdr3->hdr.blkno = cpu_to_be64(xfs_buf_daddr(bp));
+		hdr3->hdr.owner = cpu_to_be64(args->owner);
+		uuid_copy(&hdr3->hdr.uuid, &mp->m_sb.sb_meta_uuid);
 
 	} else
 		hdr->magic = cpu_to_be32(XFS_DIR2_DATA_MAGIC);
diff --git a/fs/xfs/scrub/common.c b/fs/xfs/scrub/common.c
index 20e63069088b..3d40cb0b2496 100644
--- a/fs/xfs/scrub/common.c
+++ b/fs/xfs/scrub/common.c
@@ -251,6 +251,17 @@ xchk_ino_set_preen(
 	trace_xchk_ino_preen(sc, ino, __return_address);
 }
 
+/* Record a block indexed by a file fork that could be optimized. */
+void
+xchk_fblock_set_preen(
+	struct xfs_scrub        *sc,
+	int                     whichfork,
+	xfs_fileoff_t           offset)
+{
+	sc->sm->sm_flags |= XFS_SCRUB_OFLAG_PREEN;
+	trace_xchk_fblock_preen(sc, whichfork, offset, __return_address);
+}
+
 /* Record something being wrong with the filesystem primary superblock. */
 void
 xchk_set_corrupt(
diff --git a/fs/xfs/scrub/common.h b/fs/xfs/scrub/common.h
index f2ecc68538f0..b494d747c008 100644
--- a/fs/xfs/scrub/common.h
+++ b/fs/xfs/scrub/common.h
@@ -25,6 +25,8 @@ bool xchk_fblock_xref_process_error(struct xfs_scrub *sc,
 void xchk_block_set_preen(struct xfs_scrub *sc,
 		struct xfs_buf *bp);
 void xchk_ino_set_preen(struct xfs_scrub *sc, xfs_ino_t ino);
+void xchk_fblock_set_preen(struct xfs_scrub *sc,
+		int whichfork, xfs_fileoff_t offset);
 
 void xchk_set_corrupt(struct xfs_scrub *sc);
 void xchk_block_set_corrupt(struct xfs_scrub *sc,
diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
index e09724cd3725..0a1ec94961ed 100644
--- a/fs/xfs/scrub/dir.c
+++ b/fs/xfs/scrub/dir.c
@@ -492,7 +492,12 @@ xchk_directory_data_bestfree(
 		goto out;
 	xchk_buffer_recheck(sc, bp);
 
-	/* XXX: Check xfs_dir3_data_hdr.pad is zero once we start setting it. */
+	if (xfs_has_crc(sc->mp)) {
+		struct xfs_dir3_data_hdr    *hdr3 = bp->b_addr;
+
+		if (hdr3->pad != cpu_to_be32(0))
+			xchk_fblock_set_preen(sc, XFS_DATA_FORK, lblk);
+	}
 
 	if (sc->sm->sm_flags & XFS_SCRUB_OFLAG_CORRUPT)
 		goto out_buf;
-- 
2.50.1




Amazon Web Services EMEA SARL, 38 avenue John F. Kennedy, L-1855 Luxembourg, R.C.S. Luxembourg B186284

Amazon Web Services EMEA SARL, Irish Branch, One Burlington Plaza, Burlington Road, Dublin 4, Ireland, branch registration number 908705

Re: [PATCH v2] xfs: check directory data block header padding in scrub

[Dave Chinner]

On Wed, Apr 08, 2026 at 06:27:50PM +0100, Yuto Ohnuki wrote:
> The pad field in xfs_dir3_data_hdr exists for 64-bit alignment and
> should always be zero.
> 
> First, fix xfs_dir3_data_init to zero the full xfs_dir3_data_hdr instead
> of just xfs_dir3_blk_hdr so that the pad field is covered by the memset.
> 
> Then, add the missing scrub check for the pad field in directory data
> block headers. Since old kernels may have written non-zero padding
> without issue, flag this as an optimization opportunity (preen) rather
> than corruption. Add xchk_fblock_set_preen helper for this purpose.
> 
> Signed-off-by: Yuto Ohnuki <ytohnuki@amazon.com>
> ---
> Changes in v2:
> - Fix xfs_dir3_data_init to zero the full xfs_dir3_data_hdr instead of
>   just xfs_dir3_blk_hdr so that the pad field is covered by the memset.
> - Use xchk_fblock_set_preen instead of xchk_fblock_set_corrupt since
>   old kernels may have written non-zero padding without issues.
> - Add xchk_fblock_set_preen helper function.
> - Link to v1: https://lore.kernel.org/all/20260404125032.37693-2-ytohnuki@amazon.com/#t
> ---
>  fs/xfs/libxfs/xfs_dir2_data.c | 10 +++++-----
>  fs/xfs/scrub/common.c         | 11 +++++++++++
>  fs/xfs/scrub/common.h         |  2 ++
>  fs/xfs/scrub/dir.c            |  7 ++++++-
>  4 files changed, 24 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
> index 80ba94f51e5c..037f9449835d 100644
> --- a/fs/xfs/libxfs/xfs_dir2_data.c
> +++ b/fs/xfs/libxfs/xfs_dir2_data.c
> @@ -745,13 +745,13 @@ xfs_dir3_data_init(
>  	 */
>  	hdr = bp->b_addr;
>  	if (xfs_has_crc(mp)) {
> -		struct xfs_dir3_blk_hdr *hdr3 = bp->b_addr;
> +		struct xfs_dir3_data_hdr *hdr3 = bp->b_addr;
>  
>  		memset(hdr3, 0, sizeof(*hdr3));
> -		hdr3->magic = cpu_to_be32(XFS_DIR3_DATA_MAGIC);
> -		hdr3->blkno = cpu_to_be64(xfs_buf_daddr(bp));
> -		hdr3->owner = cpu_to_be64(args->owner);
> -		uuid_copy(&hdr3->uuid, &mp->m_sb.sb_meta_uuid);
> +		hdr3->hdr.magic = cpu_to_be32(XFS_DIR3_DATA_MAGIC);
> +		hdr3->hdr.blkno = cpu_to_be64(xfs_buf_daddr(bp));
> +		hdr3->hdr.owner = cpu_to_be64(args->owner);
> +		uuid_copy(&hdr3->hdr.uuid, &mp->m_sb.sb_meta_uuid);
>  
>  	} else
>  		hdr->magic = cpu_to_be32(XFS_DIR2_DATA_MAGIC);

This seems .... a bit of a hack.

For simplicity, performance and long term maintenance of the code,
we should zero the whole directory block header region
unconditionally. This means we don't have to change the dir3 blk header
initialisation code (except to remove the zeroing), we can remove
the manual bestfree zeroing loop, all the padding (implicit and
explicit) will be zeroed, and we know that any future
changes to the dir header structure will automatically be
initialised to zero....

i.e.
	/* initialise the whole directory header region to zero */
	memset(bp->b_addr, 0, geo->data_entry_offset);

> diff --git a/fs/xfs/scrub/dir.c b/fs/xfs/scrub/dir.c
> index e09724cd3725..0a1ec94961ed 100644
> --- a/fs/xfs/scrub/dir.c
> +++ b/fs/xfs/scrub/dir.c
> @@ -492,7 +492,12 @@ xchk_directory_data_bestfree(
>  		goto out;
>  	xchk_buffer_recheck(sc, bp);
>  
> -	/* XXX: Check xfs_dir3_data_hdr.pad is zero once we start setting it. */
> +	if (xfs_has_crc(sc->mp)) {
> +		struct xfs_dir3_data_hdr    *hdr3 = bp->b_addr;
> +
> +		if (hdr3->pad != cpu_to_be32(0))

You don't need endian conversion on a value of 0.

> +			xchk_fblock_set_preen(sc, XFS_DATA_FORK, lblk);
> +	}

Why only update scrub? Why not add code that unconditionally
sets the padding to 0 on directory data block writes? e.g. in
xfs_dir3_data_write_verify() alongside the writing of the LSN into
the header?

That way any directory that is modified ends up having the padding
zeroed at runtime with no additional cost, and so filesystems will
slowly correct themselves over time without needing to run repair...

-Dave.
-- 
Dave Chinner
dgc@kernel.org