| Message ID | 20250412-xfs-hash-check-v1-1-fec1fef5d006@posteo.net (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | xfs: Verify DA node btree hash order | expand |
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@posteo.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? --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@posteo.net> > >
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@posteo.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@posteo.net> >> >>
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) + return __this_address; + prev_hash = curr_hash; + } return NULL; }
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@posteo.net> --- fs/xfs/libxfs/xfs_da_btree.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) --- base-commit: ecd5d67ad602c2c12e8709762717112ef0958767 change-id: 20250412-xfs-hash-check-be7397881a2c Best regards,