From: "Darrick J. Wong" djwong@kernel.org
[ Upstream commit deb4cd8ba87f17b12c72b3827820d9c703e9fd95 ]
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: Leah Rumancik leah.rumancik@gmail.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 @@ -153,7 +153,9 @@ xlog_recover_resv(const struct xfs_trans_res *r) return ret; }
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 @@ -630,10 +630,11 @@ xfs_attri_item_recover( if (error) goto out;
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);
error = xfs_xattri_finish_update(attr, done_item); diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c index 30c05eb862d6..317cf49e6cd6 100644 --- a/fs/xfs/xfs_bmap_item.c +++ b/fs/xfs/xfs_bmap_item.c @@ -489,10 +489,12 @@ xfs_bui_item_recover( XFS_EXTENTADD_SPACE_RES(mp, XFS_DATA_FORK), 0, 0, &tp); if (error) 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);
if (fake.bi_type == XFS_BMAP_MAP) iext_delta = XFS_IEXT_ADD_NOSPLIT_CNT; diff --git a/fs/xfs/xfs_extfree_item.c b/fs/xfs/xfs_extfree_item.c index 6cfd9339f872..9c726f082285 100644 --- a/fs/xfs/xfs_extfree_item.c +++ b/fs/xfs/xfs_extfree_item.c @@ -624,11 +624,13 @@ xfs_efi_item_recover(
resv = xlog_recover_resv(&M_RES(mp)->tr_itruncate); 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 = { .xefi_owner = XFS_RMAP_OWN_UNKNOWN, .xefi_agresv = XFS_AG_RESV_NONE, diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c index 303bf9728c03..e7992a651740 100644 --- a/fs/xfs/xfs_log_recover.c +++ b/fs/xfs/xfs_log_recover.c @@ -2591,17 +2591,10 @@ xlog_recover_process_intents( trace_xlog_intent_recovery_failed(log->l_mp, error, ops->iop_recover); 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) goto err;
@@ -2631,10 +2624,22 @@ xlog_recover_cancel_intents(
xfs_defer_cancel_recovery(log->l_mp, dfp); } }
+/* + * 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. */ STATIC void diff --git a/fs/xfs/xfs_refcount_item.c b/fs/xfs/xfs_refcount_item.c index e158dd9f86b0..2e8bdd598296 100644 --- a/fs/xfs/xfs_refcount_item.c +++ b/fs/xfs/xfs_refcount_item.c @@ -497,10 +497,11 @@ xfs_cui_item_recover( XFS_TRANS_RESERVE, &tp); if (error) 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 = { }; struct xfs_phys_extent *refc;
diff --git a/fs/xfs/xfs_rmap_item.c b/fs/xfs/xfs_rmap_item.c index b3b4de68b41b..f29fadf68ed2 100644 --- a/fs/xfs/xfs_rmap_item.c +++ b/fs/xfs/xfs_rmap_item.c @@ -524,11 +524,13 @@ xfs_rui_item_recover( resv = xlog_recover_resv(&M_RES(mp)->tr_itruncate); error = xfs_trans_alloc(mp, &resv, mp->m_rmap_maxlevels, 0, 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++) { rmap = &ruip->rui_format.rui_extents[i]; state = (rmap->me_flags & XFS_RMAP_EXTENT_UNWRITTEN) ? XFS_EXT_UNWRITTEN : XFS_EXT_NORM;