xfs: transfer recovered intent item ownership in ->iop_recover

Now that we pass the xfs_defer_pending object into the intent item
recovery functions, we know exactly when ownership of the sole refcount
passes from the recovery context to the intent done item.  At that
point, we need to null out dfp_intent so that the recovery mechanism
won't release it.  This should fix the UAF problem reported by Long Li.

Note that we still want to recreate the full deferred work state.  That
will be addressed in the next patches.

Fixes: 2e76f188fd90 ("xfs: cancel intents immediately if process_intents fails")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Christoph Hellwig <hch@lst.de>

diff --git a/fs/xfs/libxfs/xfs_log_recover.h b/fs/xfs/libxfs/xfs_log_recover.h
index 271a4ce7375cb576ef3b9069a389292c24f50f1d..13583df9f2397f4cc5a75eef46655aa4b720b11b 100644 (file)
--- a/fs/xfs/libxfs/xfs_log_recover.h
+++ b/fs/xfs/libxfs/xfs_log_recover.h
@@ -155,5 +155,7 @@ xlog_recover_resv(const struct xfs_trans_res *r)
 
 void xlog_recover_intent_item(struct xlog *log, struct xfs_log_item *lip,
                xfs_lsn_t lsn, unsigned int dfp_type);
+void xlog_recover_transfer_intent(struct xfs_trans *tp,
+               struct xfs_defer_pending *dfp);
 
 #endif /* __XFS_LOG_RECOVER_H__ */

diff --git a/fs/xfs/xfs_attr_item.c b/fs/xfs/xfs_attr_item.c
index 6119a7a480a05c992a68cb1e79c480e0bad83616..82775e9537df8b3f0f9bc523dad494dfbf6537e3 100644 (file)
--- a/fs/xfs/xfs_attr_item.c
+++ b/fs/xfs/xfs_attr_item.c
@@ -632,6 +632,7 @@ xfs_attri_item_recover(
 
        args->trans = tp;
        done_item = xfs_trans_get_attrd(tp, attrip);
+       xlog_recover_transfer_intent(tp, dfp);
 
        xfs_ilock(ip, XFS_ILOCK_EXCL);
        xfs_trans_ijoin(tp, ip, 0);

diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
index 3ef55de370b5e5e09e1489befb9b25918e3b2d91..b6d63b8bdad5a22d01a2365a80386248c8419c78 100644 (file)
--- a/fs/xfs/xfs_bmap_item.c
+++ b/fs/xfs/xfs_bmap_item.c
@@ -524,6 +524,8 @@ xfs_bui_item_recover(
                goto err_rele;
 
        budp = xfs_trans_get_bud(tp, buip);
+       xlog_recover_transfer_intent(tp, dfp);
+
        xfs_ilock(ip, XFS_ILOCK_EXCL);
        xfs_trans_ijoin(tp, ip, 0);
 

diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c
index a8245c5ffe496a9c9f59d19a7109afefb0a07940..c9908fb337657a144ed011db7fd4a3c5d9973c50 100644 (file)
--- a/fs/xfs/xfs_extfree_item.c
+++ b/fs/xfs/xfs_extfree_item.c
@@ -689,7 +689,9 @@ xfs_efi_item_recover(
        error = xfs_trans_alloc(mp, &resv, 0, 0, 0, &tp);
        if (error)
                return error;
+
        efdp = xfs_trans_get_efd(tp, efip, efip->efi_format.efi_nextents);
+       xlog_recover_transfer_intent(tp, dfp);
 
        for (i = 0; i < efip->efi_format.efi_nextents; i++) {
                struct xfs_extent_free_item     fake = {

diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
index ff768217f2c795572d403e7647f7a2b570444fb7..cc14cd1c2282f09920f3ca7a7ce8708c7c024c24 100644 (file)
--- a/fs/xfs/xfs_log_recover.c
+++ b/fs/xfs/xfs_log_recover.c
@@ -2590,13 +2590,6 @@ xlog_recover_process_intents(
                        break;
                }
 
-               /*
-                * XXX: @lip could have been freed, so detach the log item from
-                * the pending item before freeing the pending item.  This does
-                * not fix the existing UAF bug that occurs if ->iop_recover
-                * fails after creating the intent done item.
-                */
-               dfp->dfp_intent = NULL;
                xfs_defer_cancel_recovery(log->l_mp, dfp);
        }
        if (error)
@@ -2630,6 +2623,18 @@ xlog_recover_cancel_intents(
        }
 }
 
+/*
+ * Transfer ownership of the recovered log intent item to the recovery
+ * transaction.
+ */
+void
+xlog_recover_transfer_intent(
+       struct xfs_trans                *tp,
+       struct xfs_defer_pending        *dfp)
+{
+       dfp->dfp_intent = NULL;
+}
+
 /*
  * This routine performs a transaction to null out a bad inode pointer
  * in an agi unlinked inode hash bucket.

diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c
index 3456201aa3e61ac6a523ec625f911e676553c64d..f1b2592238022e4216e9517a2a0011275622c082 100644 (file)
--- a/fs/xfs/xfs_refcount_item.c
+++ b/fs/xfs/xfs_refcount_item.c
@@ -523,6 +523,7 @@ xfs_cui_item_recover(
                return error;
 
        cudp = xfs_trans_get_cud(tp, cuip);
+       xlog_recover_transfer_intent(tp, dfp);
 
        for (i = 0; i < cuip->cui_format.cui_nextents; i++) {
                struct xfs_refcount_intent      fake = { };

diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c
index dfd5a3e4b1fb5e7e2b8ab8ba9944b2d894e7e3f5..5e8a02d2b045de5443bbc584de4532613b7ea339 100644 (file)
--- a/fs/xfs/xfs_rmap_item.c
+++ b/fs/xfs/xfs_rmap_item.c
@@ -537,7 +537,9 @@ xfs_rui_item_recover(
                        XFS_TRANS_RESERVE, &tp);
        if (error)
                return error;
+
        rudp = xfs_trans_get_rud(tp, ruip);
+       xlog_recover_transfer_intent(tp, dfp);
 
        for (i = 0; i < ruip->rui_format.rui_nextents; i++) {
                struct xfs_rmap_intent  fake = { };