All of lore.kernel.org
 help / color / mirror / Atom feed
From: Catherine Hoang <catherine.hoang@oracle.com>
To: stable@vger.kernel.org
Cc: linux-xfs@vger.kernel.org
Subject: [PATCH 6.6 07/24] xfs: transfer recovered intent item ownership in ->iop_recover
Date: Tue, 26 Mar 2024 17:12:16 -0700	[thread overview]
Message-ID: <20240327001233.51675-8-catherine.hoang@oracle.com> (raw)
In-Reply-To: <20240327001233.51675-1-catherine.hoang@oracle.com>

From: "Darrick J. Wong" <djwong@kernel.org>

commit deb4cd8ba87f17b12c72b3827820d9c703e9fd95 upstream.

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>
Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
Acked-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_log_recover.h |  2 ++
 fs/xfs/xfs_attr_item.c          |  1 +
 fs/xfs/xfs_bmap_item.c          |  2 ++
 fs/xfs/xfs_extfree_item.c       |  2 ++
 fs/xfs/xfs_log_recover.c        | 19 ++++++++++++-------
 fs/xfs/xfs_refcount_item.c      |  1 +
 fs/xfs/xfs_rmap_item.c          |  2 ++
 7 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_log_recover.h b/fs/xfs/libxfs/xfs_log_recover.h
index 271a4ce7375c..13583df9f239 100644
--- 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 6119a7a480a0..82775e9537df 100644
--- 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 3ef55de370b5..b6d63b8bdad5 100644
--- 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 a8245c5ffe49..c9908fb33765 100644
--- 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 ff768217f2c7..cc14cd1c2282 100644
--- 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 3456201aa3e6..f1b259223802 100644
--- 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 dfd5a3e4b1fb..5e8a02d2b045 100644
--- 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 = { };
-- 
2.39.3


  parent reply	other threads:[~2024-03-27  0:13 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-27  0:12 [PATCH 6.6 00/24] xfs backports for 6.6.y (from 6.8) Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 01/24] xfs: move the xfs_rtbitmap.c declarations to xfs_rtbitmap.h Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 02/24] xfs: convert rt bitmap extent lengths to xfs_rtbxlen_t Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 03/24] xfs: consider minlen sized extents in xfs_rtallocate_extent_block Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 04/24] xfs: don't leak recovered attri intent items Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 05/24] xfs: use xfs_defer_pending objects to recover " Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 06/24] xfs: pass the xfs_defer_pending object to iop_recover Catherine Hoang
2024-03-27  0:12 ` Catherine Hoang [this message]
2024-03-27  0:12 ` [PATCH 6.6 08/24] xfs: make rextslog computation consistent with mkfs Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 09/24] xfs: fix 32-bit truncation in xfs_compute_rextslog Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 10/24] xfs: don't allow overly small or large realtime volumes Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 11/24] xfs: make xchk_iget safer in the presence of corrupt inode btrees Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 12/24] xfs: remove unused fields from struct xbtree_ifakeroot Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 13/24] xfs: recompute growfsrtfree transaction reservation while growing rt volume Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 14/24] xfs: fix an off-by-one error in xreap_agextent_binval Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 15/24] xfs: force all buffers to be written during btree bulk load Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 16/24] xfs: add missing nrext64 inode flag check to scrub Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 17/24] xfs: initialise di_crc in xfs_log_dinode Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 18/24] xfs: short circuit xfs_growfs_data_private() if delta is zero Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 19/24] xfs: add lock protection when remove perag from radix tree Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 20/24] xfs: fix perag leak when growfs fails Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 21/24] xfs: ensure logflagsp is initialized in xfs_bmap_del_extent_real Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 22/24] xfs: update dir3 leaf block metadata after swap Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 23/24] xfs: reset XFS_ATTR_INCOMPLETE filter on node removal Catherine Hoang
2024-03-27  0:12 ` [PATCH 6.6 24/24] xfs: remove conditional building of rt geometry validator functions Catherine Hoang
2024-03-29  9:52 ` [PATCH 6.6 00/24] xfs backports for 6.6.y (from 6.8) Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240327001233.51675-8-catherine.hoang@oracle.com \
    --to=catherine.hoang@oracle.com \
    --cc=linux-xfs@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.