From: Dave Chinner <dchinner@xxxxxxxxxx>
If we are punching out a delalloc extent, xfs_bunmapi() does not
have a transaction context and should not ever need to convert the
on-disk extent format. If such a thing is attempted (e.g. via a
corrupt inode extent count in extent format) then we should abort
with an EFSCORRUPTED error. Unfortunately, we don't do that and
crash instead:
XFS (loop0): page discard on page 0000000005fd24f3, inode 0x75e5, offset 0.
==================================================================
BUG: KASAN: null-ptr-deref in xfs_alloc_get_freelist+0x115/0x350
Read of size 8 at addr 0000000000000028 by task a.out/1406
CPU: 0 PID: 1406 Comm: a.out Not tainted 4.17.0-rc4-kasan #2
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
Call Trace:
dump_stack+0x7b/0xb5
kasan_report+0x10c/0x390
__asan_load8+0x54/0x90
xfs_alloc_get_freelist+0x115/0x350
xfs_alloc_fix_freelist+0x35b/0x830
xfs_alloc_vextent+0x215/0x990
xfs_bmap_extents_to_btree+0x30d/0x940
.....
By returning an error here, we avoid such crashes when punching out
a delalloc page because we don't try to fix up an AG freelist
without a transaction. Hence we get an error like so:
XFS (loop0): page discard on page ffffea00040ae640, inode 0x75e5, offset 0.
XFS (loop0): page discard unable to remove delalloc mapping.
And the filesystem continues to operate and the stale mapping is
cleaned up when the inode is reclaimed.
Reported-by: Wen Xu <wen.xu@xxxxxxxxxx>
Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
---
fs/xfs/libxfs/xfs_bmap.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 01628f0c9a0c..6967ce8088d2 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -5458,10 +5458,18 @@ __xfs_bunmapi(
*rlen = end - start + 1;
/*
- * Convert to a btree if necessary.
+ * Convert the BMBT root format if necessary. This should only occur in
+ * transaction contexts and not when removing delalloc extents from
+ * the in-core extent tree. If we don't have a transaction, then we've
+ * got some form of corruption somewhere, so return an error
+ * immediately.
*/
if (xfs_bmap_needs_btree(ip, whichfork)) {
ASSERT(cur == NULL);
+ if (!tp) {
+ error = -EFSCORRUPTED;
+ goto error0;
+ }
error = xfs_bmap_extents_to_btree(tp, ip, firstblock, dfops,
&cur, 0, &tmp_logflags, whichfork);
logflags |= tmp_logflags;
@@ -5473,6 +5481,10 @@ __xfs_bunmapi(
*/
else if (xfs_bmap_wants_extents(ip, whichfork)) {
ASSERT(cur != NULL);
+ if (!tp) {
+ error = -EFSCORRUPTED;
+ goto error0;
+ }
error = xfs_bmap_btree_to_extents(tp, ip, cur, &tmp_logflags,
whichfork);
logflags |= tmp_logflags;
--
2.17.0
--
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
