aboutsummaryrefslogtreecommitdiffstats
path: root/fs/xfs/xfs_rmap_item.c
diff options
context:
space:
mode:
authorDave Chinner <dchinner@redhat.com>2018-04-02 20:08:27 -0700
committerDarrick J. Wong <darrick.wong@oracle.com>2018-04-02 20:08:27 -0700
commit0612d1166330697d91b8d2d1e71e41485bb0b18e (patch)
tree3f7110808ec01042bf6ad6756ccb1e5752f7b7b6 /fs/xfs/xfs_rmap_item.c
parentxfs: Remove "committed" argument of xfs_dir_ialloc (diff)
downloadlinux-dev-0612d1166330697d91b8d2d1e71e41485bb0b18e.tar.xz
linux-dev-0612d1166330697d91b8d2d1e71e41485bb0b18e.zip
xfs: fix intent use-after-free on abort
When an intent is aborted during it's initial commit through xfs_defer_trans_abort(), there is a use after free. The current report is for a RUI through this path in generic/388: Freed by task 6274: __kasan_slab_free+0x136/0x180 kmem_cache_free+0xe7/0x4b0 xfs_trans_free_items+0x198/0x2e0 __xfs_trans_commit+0x27f/0xcc0 xfs_trans_roll+0x17b/0x2a0 xfs_defer_trans_roll+0x6ad/0xe60 xfs_defer_finish+0x2a6/0x2140 xfs_alloc_file_space+0x53a/0xf90 xfs_file_fallocate+0x5c6/0xac0 vfs_fallocate+0x2f5/0x930 ioctl_preallocate+0x1dc/0x320 do_vfs_ioctl+0xfe4/0x1690 The problem is that the RUI has two active references - one in the current transaction, and another held by the defer_ops structure that is passed to the RUD (intent done) so that both the intent and the intent done structures are freed on commit of the intent done. Hence during abort, we need to release the intent item, because the defer_ops reference is released separately via ->abort_intent callback. Fix all the intent code to do this correctly. Signed-Off-By: Dave Chinner <dchinner@redhat.com> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Diffstat (limited to 'fs/xfs/xfs_rmap_item.c')
-rw-r--r--fs/xfs/xfs_rmap_item.c38
1 files changed, 19 insertions, 19 deletions
diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
index 49d3124863a8..06a07846c9b3 100644
--- a/fs/xfs/xfs_rmap_item.c
+++ b/fs/xfs/xfs_rmap_item.c
@@ -52,6 +52,24 @@ xfs_rui_item_free(
kmem_zone_free(xfs_rui_zone, ruip);
}
+/*
+ * Freeing the RUI requires that we remove it from the AIL if it has already
+ * been placed there. However, the RUI may not yet have been placed in the AIL
+ * when called by xfs_rui_release() from RUD processing due to the ordering of
+ * committed vs unpin operations in bulk insert operations. Hence the reference
+ * count to ensure only the last caller frees the RUI.
+ */
+void
+xfs_rui_release(
+ struct xfs_rui_log_item *ruip)
+{
+ ASSERT(atomic_read(&ruip->rui_refcount) > 0);
+ if (atomic_dec_and_test(&ruip->rui_refcount)) {
+ xfs_trans_ail_remove(&ruip->rui_item, SHUTDOWN_LOG_IO_ERROR);
+ xfs_rui_item_free(ruip);
+ }
+}
+
STATIC void
xfs_rui_item_size(
struct xfs_log_item *lip,
@@ -141,7 +159,7 @@ xfs_rui_item_unlock(
struct xfs_log_item *lip)
{
if (lip->li_flags & XFS_LI_ABORTED)
- xfs_rui_item_free(RUI_ITEM(lip));
+ xfs_rui_release(RUI_ITEM(lip));
}
/*
@@ -233,24 +251,6 @@ xfs_rui_copy_format(
return 0;
}
-/*
- * Freeing the RUI requires that we remove it from the AIL if it has already
- * been placed there. However, the RUI may not yet have been placed in the AIL
- * when called by xfs_rui_release() from RUD processing due to the ordering of
- * committed vs unpin operations in bulk insert operations. Hence the reference
- * count to ensure only the last caller frees the RUI.
- */
-void
-xfs_rui_release(
- struct xfs_rui_log_item *ruip)
-{
- ASSERT(atomic_read(&ruip->rui_refcount) > 0);
- if (atomic_dec_and_test(&ruip->rui_refcount)) {
- xfs_trans_ail_remove(&ruip->rui_item, SHUTDOWN_LOG_IO_ERROR);
- xfs_rui_item_free(ruip);
- }
-}
-
static inline struct xfs_rud_log_item *RUD_ITEM(struct xfs_log_item *lip)
{
return container_of(lip, struct xfs_rud_log_item, rud_item);