[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <f265594f-6676-459e-9900-ae10f2efa0d5@posteo.net>
Date: Tue, 15 Apr 2025 01:08:50 +0000
From: Charalampos Mitrodimas <charmitro@...teo.net>
To: "Darrick J. Wong" <djwong@...nel.org>
Cc: Carlos Maiolino <cem@...nel.org>, linux-xfs@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] xfs: Verify DA node btree hash order
On 15/4/25 01:15, Darrick J. Wong wrote:
> On Sat, Apr 12, 2025 at 08:03:57PM +0000, Charalampos Mitrodimas wrote:
>> The xfs_da3_node_verify() function checks the integrity of directory
>> and attribute B-tree node blocks. However, it was missing a check to
>> ensure that the hash values of the btree entries within the node are
>> strictly increasing, as required by the B-tree structure.
>>
>> Add a loop to iterate through the btree entries and verify that each
>> entry's hash value is greater than the previous one. If an
>> out-of-order hash value is detected, return failure to indicate
>> corruption.
>>
>> This addresses the "XXX: hash order check?" comment and improves
>> corruption detection for DA node blocks.
>>
>> Signed-off-by: Charalampos Mitrodimas <charmitro@...teo.net>
>> ---
>> fs/xfs/libxfs/xfs_da_btree.c | 11 ++++++++++-
>> 1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
>> index 17d9e6154f1978ce5a5cb82176eea4d6b9cd768d..6c748911e54619c3ceae9b81f55cf61da6735f01 100644
>> --- a/fs/xfs/libxfs/xfs_da_btree.c
>> +++ b/fs/xfs/libxfs/xfs_da_btree.c
>> @@ -247,7 +247,16 @@ xfs_da3_node_verify(
>> ichdr.count > mp->m_attr_geo->node_ents)
>> return __this_address;
>>
>> - /* XXX: hash order check? */
>> + /* Check hash order */
>> + uint32_t prev_hash = be32_to_cpu(ichdr.btree[0].hashval);
>> +
>> + for (int i = 1; i < ichdr.count; i++) {
>> + uint32_t curr_hash = be32_to_cpu(ichdr.btree[i].hashval);
>> +
>> + if (curr_hash <= prev_hash)
> Does XFS support a directory with two names that hash to the same value?
Hi Darrick,
I believe so, yes. These are handled at the leaf level by comparing the
actual names. This adds a check which ensures the intermediate node's
internal B-tree entries are strictly ordered by hash.
C. Mitrodimas
>
> --D
>
>> + return __this_address;
>> + prev_hash = curr_hash;
>> + }
>>
>> return NULL;
>> }
>>
>> ---
>> base-commit: ecd5d67ad602c2c12e8709762717112ef0958767
>> change-id: 20250412-xfs-hash-check-be7397881a2c
>>
>> Best regards,
>> --
>> Charalampos Mitrodimas <charmitro@...teo.net>
>>
>>
Powered by blists - more mailing lists