linux-kernel - Re: [PATCH v4 2/3] xfs: update dir3 leaf block metadata after swap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20231205060630.GM361584@frogsfrogsfrogs>
Date:   Mon, 4 Dec 2023 22:06:30 -0800
From:   "Darrick J. Wong" <djwong@...nel.org>
To:     Jiachen Zhang <zhangjiachen.jaycee@...edance.com>
Cc:     Chandan Babu R <chandan.babu@...cle.com>,
        Dave Chinner <dchinner@...hat.com>,
        Allison Henderson <allison.henderson@...cle.com>,
        Zhang Tianci <zhangtianci.1997@...edance.com>,
        Brian Foster <bfoster@...hat.com>, linux-xfs@...r.kernel.org,
        linux-kernel@...r.kernel.org, xieyongji@...edance.com, me@...x.top,
        Dave Chinner <david@...morbit.com>
Subject: Re: [PATCH v4 2/3] xfs: update dir3 leaf block metadata after swap

On Tue, Dec 05, 2023 at 01:58:59PM +0800, Jiachen Zhang wrote:
> From: Zhang Tianci <zhangtianci.1997@...edance.com>
> 
> xfs_da3_swap_lastblock() copy the last block content to the dead block,
> but do not update the metadata in it. We need update some metadata
> for some kinds of type block, such as dir3 leafn block records its
> blkno, we shall update it to the dead block blkno. Otherwise,
> before write the xfs_buf to disk, the verify_write() will fail in
> blk_hdr->blkno != xfs_buf->b_bn, then xfs will be shutdown.
> 
> We will get this warning:
> 
>   XFS (dm-0): Metadata corruption detected at xfs_dir3_leaf_verify+0xa8/0xe0 [xfs], xfs_dir3_leafn block 0x178
>   XFS (dm-0): Unmount and run xfs_repair
>   XFS (dm-0): First 128 bytes of corrupted metadata buffer:
>   00000000e80f1917: 00 80 00 0b 00 80 00 07 3d ff 00 00 00 00 00 00  ........=.......
>   000000009604c005: 00 00 00 00 00 00 01 a0 00 00 00 00 00 00 00 00  ................
>   000000006b6fb2bf: e4 44 e3 97 b5 64 44 41 8b 84 60 0e 50 43 d9 bf  .D...dDA..`.PC..
>   00000000678978a2: 00 00 00 00 00 00 00 83 01 73 00 93 00 00 00 00  .........s......
>   00000000b28b247c: 99 29 1d 38 00 00 00 00 99 29 1d 40 00 00 00 00  .).8.....).@....
>   000000002b2a662c: 99 29 1d 48 00 00 00 00 99 49 11 00 00 00 00 00  .).H.....I......
>   00000000ea2ffbb8: 99 49 11 08 00 00 45 25 99 49 11 10 00 00 48 fe  .I....E%.I....H.
>   0000000069e86440: 99 49 11 18 00 00 4c 6b 99 49 11 20 00 00 4d 97  .I....Lk.I. ..M.
>   XFS (dm-0): xfs_do_force_shutdown(0x8) called from line 1423 of file fs/xfs/xfs_buf.c.  Return address = 00000000c0ff63c1
>   XFS (dm-0): Corruption of in-memory data detected.  Shutting down filesystem
>   XFS (dm-0): Please umount the filesystem and rectify the problem(s)
> 
> From the log above, we know xfs_buf->b_no is 0x178, but the block's hdr record
> its blkno is 0x1a0.
> 
> Fixes: 24df33b45ecf ("xfs: add CRC checking to dir2 leaf blocks")
> Signed-off-by: Zhang Tianci <zhangtianci.1997@...edance.com>
> Suggested-by: Dave Chinner <david@...morbit.com>

still looks fine to me
Reviewed-by: Darrick J. Wong <djwong@...nel.org>

--D

> ---
>  fs/xfs/libxfs/xfs_da_btree.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index e576560b46e9..282c7cf032f4 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -2316,10 +2316,17 @@ xfs_da3_swap_lastblock(
>  		return error;
>  	/*
>  	 * Copy the last block into the dead buffer and log it.
> +	 * On CRC-enabled file systems, also update the stamped in blkno.
>  	 */
>  	memcpy(dead_buf->b_addr, last_buf->b_addr, args->geo->blksize);
> +	if (xfs_has_crc(mp)) {
> +		struct xfs_da3_blkinfo *da3 = dead_buf->b_addr;
> +
> +		da3->blkno = cpu_to_be64(xfs_buf_daddr(dead_buf));
> +	}
>  	xfs_trans_log_buf(tp, dead_buf, 0, args->geo->blksize - 1);
>  	dead_info = dead_buf->b_addr;
> +
>  	/*
>  	 * Get values from the moved block.
>  	 */
> -- 
> 2.20.1
> 
>