From: "Darrick J. Wong" <djwong@kernel.org>
To: Yuto Ohnuki <ytohnuki@amazon.com>
Cc: Carlos Maiolino <cem@kernel.org>,
linux-xfs@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] xfs: check hash ordering in xfs_da3_node_verify
Date: Wed, 15 Apr 2026 08:16:25 -0700 [thread overview]
Message-ID: <20260415151625.GB114209@frogsfrogsfrogs> (raw)
In-Reply-To: <20260415071223.48609-2-ytohnuki@amazon.com>
On Wed, Apr 15, 2026 at 08:12:24AM +0100, Yuto Ohnuki wrote:
> The DA node verifier checks header fields such as level and count, but
> it does not verify that the btree hash values are in non-decreasing
> order.
>
> DA node blocks are traversed by searching the btree array for the first
> hash value that exceeds the lookup hash, which requires the hash values
> to be ordered correctly.
>
> Add a hash order check to xfs_da3_node_verify, similar to the existing
> validation in xfs_dir3_leaf_check_int, so that misordered node
> blocks are detected as metadata corruption.
>
> Signed-off-by: Yuto Ohnuki <ytohnuki@amazon.com>
> ---
> fs/xfs/libxfs/xfs_da_btree.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index ad801b7bd2dd..ef49df22899e 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -225,6 +225,7 @@ xfs_da3_node_verify(
> struct xfs_da_intnode *hdr = bp->b_addr;
> struct xfs_da3_icnode_hdr ichdr;
> xfs_failaddr_t fa;
> + int i;
>
> xfs_da3_node_hdr_from_disk(mp, &ichdr, hdr);
>
> @@ -247,7 +248,12 @@ xfs_da3_node_verify(
> ichdr.count > mp->m_attr_geo->node_ents)
> return __this_address;
>
> - /* XXX: hash order check? */
> + /* Check hash value order. */
> + for (i = 0; i + 1 < ichdr.count; i++) {
I guess that works, though I'd have iterated from i to ichdr.count and
done the test on the previous element (like scrub/dir.c does).
> + if (be32_to_cpu(ichdr.btree[i].hashval) >
> + be32_to_cpu(ichdr.btree[i + 1].hashval))
I think this might be one of the few places where doing kernel style
indentation of the if test flows better:
for (i = 1; i < ichdr.count; i++) {
if (be32_to_cpu(ichdr.btree[i - 1].hashval) >
be32_to_cpu(ichdr.btree[i].hashval))
The logic is solid though.
--D
> + return __this_address;
> + }
>
> return NULL;
> }
> --
> 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
>
>
>
>
next prev parent reply other threads:[~2026-04-15 15:16 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-04-15 7:12 Yuto Ohnuki
2026-04-15 15:16 ` Darrick J. Wong [this message]
2026-04-16 0:39 ` Dave Chinner
2026-04-20 17:54 ` Yuto Ohnuki
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260415151625.GB114209@frogsfrogsfrogs \
--to=djwong@kernel.org \
--cc=cem@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
--cc=ytohnuki@amazon.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®