Skip to content

Commit

Permalink
xfs: transfer recovered intent item ownership in ->iop_recover
Browse files Browse the repository at this point in the history
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: 2e76f18 ("xfs: cancel intents immediately if process_intents fails")
Signed-off-by: Darrick J. Wong <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
  • Loading branch information
Darrick J. Wong committed Dec 7, 2023
1 parent a050acd commit deb4cd8
Show file tree
Hide file tree
Showing 7 changed files with 22 additions and 7 deletions.
2 changes: 2 additions & 0 deletions fs/xfs/libxfs/xfs_log_recover.h
Original file line number Diff line number Diff line change
Expand Up @@ -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__ */
1 change: 1 addition & 0 deletions fs/xfs/xfs_attr_item.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 2 additions & 0 deletions fs/xfs/xfs_bmap_item.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
2 changes: 2 additions & 0 deletions fs/xfs/xfs_extfree_item.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
19 changes: 12 additions & 7 deletions fs/xfs/xfs_log_recover.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions fs/xfs/xfs_refcount_item.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 = { };
Expand Down
2 changes: 2 additions & 0 deletions fs/xfs/xfs_rmap_item.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 = { };
Expand Down

0 comments on commit deb4cd8

Please sign in to comment.