xfs: abort extended attribute list operation if btree is obviously weird
PATCH 1/1 xfs: abort extended attribute list operation if btree is obviously weird
Abort an attribute list operation if the attr btree has obvious problems like loops back to the root or pointers don't point down the tree. Found by fuzzing btree[0].before to zero in xfs/402.
--- fs/xfs/xfs_attr_list.c | 54 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 11 deletions(-)
fs/xfs/xfs_attr_list.c +43 -11
@@ -205,18 +205,21 @@ xfs_attr_shortform_list(xfs_attr_list_context_t *context) } STATIC int -xfs_attr_node_list (xfs_attr_list_context_t *context) +xfs_attr_node_list ( +struct xfs_attr_list_context *context) { -attrlist_cursor_kern_t *cursor; -xfs_attr_leafblock_t *leaf; -xfs_da_intnode_t *node; -struct xfs_attr3_icleaf_hdr leafhdr; -struct xfs_da3_icnode_hdr nodehdr; -struct xfs_da_node_entry *btree; -int error, i; -struct xfs_buf *bp; -struct xfs_inode *dp = context->dp; -struct xfs_mount *mp = dp->i_mount; +struct xfs_attr3_icleaf_hdr leafhdr; +struct xfs_da3_icnode_hdr nodehdr; +struct attrlist_cursor_kern *cursor; +struct xfs_attr_leafblock *leaf; +struct xfs_da_intnode *node; +struct xfs_da_node_entry *btree; +struct xfs_buf *bp; +struct xfs_inode *dp = context->dp; +struct xfs_mount *mp = dp->i_mount; +int i; +unsigned int expected_level; +int error;trace_xfs_attr_node_list (context);
@@ -277,6 +280,7 @@ xfs_attr_node_list(xfs_attr_list_context_t *context)* Note that start of node block is same as start of leaf block. */ if (bp ==NULL ) { + expected_level =-1U ; cursor->blkno =0 ;for (;;) {uint16_t magic;
@@ -302,6 +306,30 @@ xfs_attr_node_list(xfs_attr_list_context_t *context) } dp->d_ops->node_hdr_from_disk (&nodehdr, node); + +/* Tree taller than we can handle; bail out! */ +if (nodehdr.level >=XFS_DA_NODE_MAXDEPTH ) { +xfs_trans_brelse (context->tp, bp); +return -EFSCORRUPTED ; + } + +if (cursor->blkno ==0 ) { +/* +* This is the root node, set up for the +* next level we want to see. +*/ + expected_level = nodehdr.level -1 ; + }else if (expected_level != nodehdr.level) { +/* +* Not the level we were expecting, which +* implies that the tree is bad. +*/ +xfs_trans_brelse (context->tp, bp); +return -EFSCORRUPTED ; + }else { + expected_level--; + } + btree = dp->d_ops->node_tree_p (node);for (i =0 ; i < nodehdr.count; btree++, i++) { if (cursor->hashval
@@ -317,6 +345,10 @@ xfs_attr_node_list(xfs_attr_list_context_t *context)return 0 ; }xfs_trans_brelse (context->tp, bp); + +/* We can't point back to the root. */ +if (cursor->blkno ==0 ) +return -EFSCORRUPTED ; } }ASSERT (bp !=NULL );
Quoted 13 lines · Darrick J. Wong
Abort an attribute list operation if the attr btree has obvious problems like loops back to the root or pointers don't point down the tree. Found by fuzzing btree[0].before to zero in xfs/402.
Signed-off-by Darrick J. Wong <[email protected]>--- fs/xfs/xfs_attr_list.c | 54 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 11 deletions(-)fs/xfs/xfs_attr_list.c +0
-0
...
Quoted 18 lines
fs/xfs/xfs_attr_list.c +14
-0@@ -302,6 +306,30 @@ xfs_attr_node_list(xfs_attr_list_context_t *context) } dp->d_ops->node_hdr_from_disk (&nodehdr, node); + +/* Tree taller than we can handle; bail out! */ +if (nodehdr.level >=XFS_DA_NODE_MAXDEPTH ) { +xfs_trans_brelse (context->tp, bp); +return -EFSCORRUPTED ; + } + +if (cursor->blkno ==0 ) { +/* +* This is the root node, set up for the +* next level we want to see. +*/ + expected_level = nodehdr.level -1 ; + }else if (expected_level != nodehdr.level) {
fs/xfs/xfs_attr_list.c xfs_attr_node_list(xfs_attr_list_context_t *context) · line 332
Quoted 10 lines
fs/xfs/xfs_attr_list.c +10
-0+/* +* Not the level we were expecting, which +* implies that the tree is bad. +*/ +xfs_trans_brelse (context->tp, bp); +return -EFSCORRUPTED ; + }else { + expected_level--; + } +
It looks like we wouldn't catch the case of a two level tree having a bad level, since we'd break out on leaf magic check above. Perhaps we should check or assert that expected_level reaches 0? Otherwise looks Ok to me.
Brian
Quoted 18 lines
fs/xfs/xfs_attr_list.c +4
-1btree = dp->d_ops->node_tree_p (node);for (i =0 ; i < nodehdr.count; btree++, i++) { if (cursor->hashval@@ -317,6 +345,10 @@ xfs_attr_node_list(xfs_attr_list_context_t *context)return 0 ; }xfs_trans_brelse (context->tp, bp); + +/* We can't point back to the root. */ +if (cursor->blkno ==0 ) +return -EFSCORRUPTED ; } }ASSERT (bp !=NULL ); --To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 40 lines · Brian Foster
Quoted 13 lines · Darrick J. Wong
Abort an attribute list operation if the attr btree has obvious problems like loops back to the root or pointers don't point down the tree. Found by fuzzing btree[0].before to zero in xfs/402.
Signed-off-by Darrick J. Wong <[email protected]>--- fs/xfs/xfs_attr_list.c | 54 ++++++++++++++++++++++++++++++++++++++---------- 1 file changed, 43 insertions(+), 11 deletions(-)fs/xfs/xfs_attr_list.c +0
-0...
Quoted 25 lines
fs/xfs/xfs_attr_list.c +21
-0@@ -302,6 +306,30 @@ xfs_attr_node_list(xfs_attr_list_context_t *context) } dp->d_ops->node_hdr_from_disk (&nodehdr, node); + +/* Tree taller than we can handle; bail out! */ +if (nodehdr.level >=XFS_DA_NODE_MAXDEPTH ) { +xfs_trans_brelse (context->tp, bp); +return -EFSCORRUPTED ; + } + +if (cursor->blkno ==0 ) { +/* +* This is the root node, set up for the +* next level we want to see. +*/ + expected_level = nodehdr.level -1 ; + }else if (expected_level != nodehdr.level) { +/* +* Not the level we were expecting, which +* implies that the tree is bad. +*/ +xfs_trans_brelse (context->tp, bp); +return -EFSCORRUPTED ; + }else {
Quoted 8 lines
fs/xfs/xfs_attr_list.c xfs_attr_node_list(xfs_attr_list_context_t *context) · line 332
Quoted 3 lines
fs/xfs/xfs_attr_list.c +3
-0+ expected_level--; + } +It looks like we wouldn't catch the case of a two level tree having a bad level, since we'd break out on leaf magic check above. Perhaps we should check or assert that
expected_levelreaches 0? Otherwise looks Ok to me.
Ok, will do.
--D
Quoted 25 lines
Brian
Quoted 15 lines
fs/xfs/xfs_attr_list.c +4
-1btree = dp->d_ops->node_tree_p (node);for (i =0 ; i < nodehdr.count; btree++, i++) { if (cursor->hashval@@ -317,6 +345,10 @@ xfs_attr_node_list(xfs_attr_list_context_t *context)return 0 ; }xfs_trans_brelse (context->tp, bp); + +/* We can't point back to the root. */ +if (cursor->blkno ==0 ) +return -EFSCORRUPTED ; } }ASSERT (bp !=NULL ); --Quoted 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html