* [PATCH] xfs: avoid mount-time deadlock in CoW extent recovery
@ 2017-05-16 19:27 Darrick J. Wong
2017-05-17 0:39 ` Dave Chinner
0 siblings, 1 reply; 3+ messages in thread
From: Darrick J. Wong @ 2017-05-16 19:27 UTC (permalink / raw)
To: xfs
If a malicious user corrupts the refcount btree to cause a cycle between
different levels of the tree, the next mount attempt will deadlock in
the CoW recovery routine while grabbing buffer locks. We can use the
ability to re-grab a buffer that was previous locked to a transaction to
avoid deadlocks, so do that here.
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
fs/xfs/libxfs/xfs_refcount.c | 29 +++++++++++++++++++----------
1 file changed, 19 insertions(+), 10 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
index b177ef3..6fd337c 100644
--- a/fs/xfs/libxfs/xfs_refcount.c
+++ b/fs/xfs/libxfs/xfs_refcount.c
@@ -1629,10 +1629,14 @@ xfs_refcount_recover_cow_leftovers(
if (mp->m_sb.sb_agblocks >= XFS_REFC_COW_START)
return -EOPNOTSUPP;
- error = xfs_alloc_read_agf(mp, NULL, agno, 0, &agbp);
+ error = xfs_trans_alloc_empty(mp, &tp);
if (error)
return error;
- cur = xfs_refcountbt_init_cursor(mp, NULL, agbp, agno, NULL);
+
+ error = xfs_alloc_read_agf(mp, tp, agno, 0, &agbp);
+ if (error)
+ goto out_trans;
+ cur = xfs_refcountbt_init_cursor(mp, tp, agbp, agno, NULL);
/* Find all the leftover CoW staging extents. */
INIT_LIST_HEAD(&debris);
@@ -1645,10 +1649,11 @@ xfs_refcount_recover_cow_leftovers(
if (error)
goto out_cursor;
xfs_btree_del_cursor(cur, XFS_BTREE_NOERROR);
- xfs_buf_relse(agbp);
+ xfs_trans_brelse(tp, agbp);
+ xfs_trans_cancel(tp);
/* Now iterate the list to free the leftovers */
- list_for_each_entry(rr, &debris, rr_list) {
+ list_for_each_entry_safe(rr, n, &debris, rr_list) {
/* Set up transaction. */
error = xfs_trans_alloc(mp, &M_RES(mp)->tr_write, 0, 0, 0, &tp);
if (error)
@@ -1676,8 +1681,15 @@ xfs_refcount_recover_cow_leftovers(
error = xfs_trans_commit(tp);
if (error)
goto out_free;
+
+ list_del(&rr->rr_list);
+ kmem_free(rr);
}
+ return error;
+out_defer:
+ xfs_defer_cancel(&dfops);
+ xfs_trans_cancel(tp);
out_free:
/* Free the leftover list */
list_for_each_entry_safe(rr, n, &debris, rr_list) {
@@ -1688,11 +1700,8 @@ xfs_refcount_recover_cow_leftovers(
out_cursor:
xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
- xfs_buf_relse(agbp);
- goto out_free;
-
-out_defer:
- xfs_defer_cancel(&dfops);
+ xfs_trans_brelse(tp, agbp);
+out_trans:
xfs_trans_cancel(tp);
- goto out_free;
+ return error;
}
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] xfs: avoid mount-time deadlock in CoW extent recovery
2017-05-16 19:27 [PATCH] xfs: avoid mount-time deadlock in CoW extent recovery Darrick J. Wong
@ 2017-05-17 0:39 ` Dave Chinner
2017-05-17 1:06 ` Darrick J. Wong
0 siblings, 1 reply; 3+ messages in thread
From: Dave Chinner @ 2017-05-17 0:39 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: xfs
On Tue, May 16, 2017 at 12:27:48PM -0700, Darrick J. Wong wrote:
> If a malicious user corrupts the refcount btree to cause a cycle between
> different levels of the tree, the next mount attempt will deadlock in
> the CoW recovery routine while grabbing buffer locks. We can use the
> ability to re-grab a buffer that was previous locked to a transaction to
> avoid deadlocks, so do that here.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
Missing comments in the code explaining the deadlock being avoided
by using xfs_trans_alloc_empty()....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] xfs: avoid mount-time deadlock in CoW extent recovery
2017-05-17 0:39 ` Dave Chinner
@ 2017-05-17 1:06 ` Darrick J. Wong
0 siblings, 0 replies; 3+ messages in thread
From: Darrick J. Wong @ 2017-05-17 1:06 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs
On Wed, May 17, 2017 at 10:39:31AM +1000, Dave Chinner wrote:
> On Tue, May 16, 2017 at 12:27:48PM -0700, Darrick J. Wong wrote:
> > If a malicious user corrupts the refcount btree to cause a cycle between
> > different levels of the tree, the next mount attempt will deadlock in
> > the CoW recovery routine while grabbing buffer locks. We can use the
> > ability to re-grab a buffer that was previous locked to a transaction to
> > avoid deadlocks, so do that here.
> >
> > Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
>
> Missing comments in the code explaining the deadlock being avoided
> by using xfs_trans_alloc_empty()....
Heh, ok.
--D
>
> Cheers,
>
> Dave.
> --
> Dave Chinner
> david@fromorbit.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-05-17 1:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-16 19:27 [PATCH] xfs: avoid mount-time deadlock in CoW extent recovery Darrick J. Wong
2017-05-17 0:39 ` Dave Chinner
2017-05-17 1:06 ` Darrick J. Wong
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.