Hi Darrick,
On 2018年04月18日 10:41, Shan Hai wrote:
On 2018年04月18日 10:24, Darrick J. Wong wrote:
On Wed, Apr 18, 2018 at 10:13:22AM +0800, Shan Hai wrote:
On 2018年04月18日 10:09, Shan Hai wrote:
Fuzzing tool reports a write to null pointer error in the
xfs_bmap_extents_to_btree, fix it by bailing out on encountering
a null pointer.
Signed-off-by: Shan Hai <shan.hai@xxxxxxxxxx>
This one supposed to be applied on top of below:
https://www.spinics.net/lists/linux-xfs/msg17254.html
[PATCH] xfs: set format back to extents if xfs_bmap_extents_to_btree
fails
Thanks
Shan Hai
---
fs/xfs/libxfs/xfs_bmap.c | 24 ++++++++++++++++--------
1 file changed, 16 insertions(+), 8 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 040eeda..90b743d 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -724,19 +724,14 @@ xfs_bmap_extents_to_btree(
args.wasdel = wasdel;
*logflagsp = 0;
if ((error = xfs_alloc_vextent(&args))) {
- xfs_iroot_realloc(ip, -1, whichfork);
ASSERT(ifp->if_broot == NULL);
- XFS_IFORK_FMT_SET(ip, whichfork, XFS_DINODE_FMT_EXTENTS);
- xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
- return error;
+ goto err1;
}
if (WARN_ON_ONCE(args.fsbno == NULLFSBLOCK)) {
- xfs_iroot_realloc(ip, -1, whichfork);
ASSERT(ifp->if_broot == NULL);
- XFS_IFORK_FMT_SET(ip, whichfork, XFS_DINODE_FMT_EXTENTS);
- xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
- return -ENOSPC;
+ error = -ENOSPC;
+ goto err1;
}
/*
* Allocation can't fail, the space was reserved.
@@ -748,6 +743,10 @@ xfs_bmap_extents_to_btree(
ip->i_d.di_nblocks++;
xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_BCOUNT, 1L);
abp = xfs_btree_get_bufl(mp, tp, args.fsbno, 0);
+ if (!abp) {
When does this happen? We got args.fsbno from the allocator, so we're
not out of space. Or are you saying that something fuzzed the free
space btree, therefore the allocator gave back a nonsense block number
(say pointing past the end of the fs), and therefore the _get_bufl call
returned NULL?
Seems the memory page allocation fails and the xfs_btree_get_bufl
returns NULL, but I have no reliable reproducer, sorry.
I have got another report of a possible NULL pointer deference in the
code as below,
this time from a static code analysis tool:
xfs_bmap_extents_to_btree
->abp = xfs_btree_get_bufl
->return xfs_trans_get_buf
->return xfs_trans_get_buf_map
->bp = xfs_buf_get_map
if (bp == NULL) {
return NULL;
}
As you know that the corrupted block numbers from block allocator is checked
in the _xfs_buf_find, which returns NULL as well after a WARN_ON if the
the bno is
invalid.
And the maximum allocation size in the xfs_buf_get_map is a single page, so
because of the PAGE_ALLOC_COSTLY_ORDER logic it's highly possible that the
process is killed the OOM killer instead of returning NULL from both
kmalloc/alloc_page.
Even though this NULL pointer problem is rarely seen in the real
workload but it's
logically correct to check the pointer validity as this patch does in my
opinion.
Thanks
Shan Hai
Thanks
Shan Hai
If so, maybe we need to change that WARN_ON_ONCE thing above to:
if (WARN_ON_ONCE(...) || !xfs_verify_fsbno(..., args.fsbno)) {
/* undo state and return */
}
--D
+ error = -ENOSPC;
+ goto err2;
+ }
/*
* Fill in the child block.
*/
@@ -787,6 +786,15 @@ xfs_bmap_extents_to_btree(
*curp = cur;
*logflagsp = XFS_ILOG_CORE | xfs_ilog_fbroot(whichfork);
return 0;
+
+err2:
+ xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_BCOUNT, -1L);
+err1:
+ xfs_iroot_realloc(ip, -1, whichfork);
+ XFS_IFORK_FMT_SET(ip, whichfork, XFS_DINODE_FMT_EXTENTS);
+ xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
+
+ return error;
}
/*
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
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 majordomo@xxxxxxxxxxxxxxx
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 majordomo@xxxxxxxxxxxxxxx
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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
