[PATCH 1/2] xfs: transactionless xfs_bunmapi shouldn't do format conversion

[Dave Chinner <david@fromorbit.com>]

From: Dave Chinner <dchinner@redhat.com>

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@gatech.edu>
Signed-off-by: Dave Chinner <dchinner@redhat.com>
---
 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