From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
In xfs_bui_item_recover, there exists a use-after-free bug with regards
to the inode that is involved in the bmap replay operation. If the
mapping operation does not complete, we call xfs_bmap_unmap_extent to
create a deferred op to finish the unmapping work, and we retain a
pointer to the incore inode.
Unfortunately, the very next thing we do is commit the transaction and
drop the inode. If reclaim tears down the inode before we try to finish
the defer ops, we dereference garbage and blow up. Therefore, create a
way to join inodes to the defer ops freezer so that we can maintain the
xfs_inode reference until we're done with the inode.
Note: This imposes the requirement that there be enough memory to keep
every incore inode in memory throughout recovery.
Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
---
fs/xfs/libxfs/xfs_defer.c | 50 +++++++++++++++++++++++++++++++++++++++++++++
fs/xfs/libxfs/xfs_defer.h | 10 +++++++++
fs/xfs/xfs_bmap_item.c | 7 ++++--
fs/xfs/xfs_icache.c | 19 +++++++++++++++++
4 files changed, 83 insertions(+), 3 deletions(-)
diff --git a/fs/xfs/libxfs/xfs_defer.c b/fs/xfs/libxfs/xfs_defer.c
index ea4d28851bbd..72933fdafcb2 100644
--- a/fs/xfs/libxfs/xfs_defer.c
+++ b/fs/xfs/libxfs/xfs_defer.c
@@ -16,6 +16,7 @@
#include "xfs_inode.h"
#include "xfs_inode_item.h"
#include "xfs_trace.h"
+#include "xfs_icache.h"
/*
* Deferred Operations in XFS
@@ -583,8 +584,19 @@ xfs_defer_thaw(
struct xfs_defer_freezer *dff,
struct xfs_trans *tp)
{
+ int i;
+
ASSERT(tp->t_flags & XFS_TRANS_PERM_LOG_RES);
+ /* Re-acquire the inode locks. */
+ for (i = 0; i < XFS_DEFER_FREEZER_INODES; i++) {
+ if (!dff->dff_inodes[i])
+ break;
+
+ dff->dff_ilocks[i] = XFS_ILOCK_EXCL;
+ xfs_ilock(dff->dff_inodes[i], dff->dff_ilocks[i]);
+ }
+
/* Add the dfops items to the transaction. */
list_splice_init(&dff->dff_dfops, &tp->t_dfops);
tp->t_flags |= dff->dff_tpflags;
@@ -597,5 +609,43 @@ xfs_defer_freeezer_finish(
struct xfs_defer_freezer *dff)
{
xfs_defer_cancel_list(mp, &dff->dff_dfops);
+ xfs_defer_freezer_irele(dff);
kmem_free(dff);
}
+
+/*
+ * Attach an inode to this deferred ops freezer. Callers must hold ILOCK_EXCL,
+ * which will be dropped and reacquired when we're ready to thaw the frozen
+ * deferred ops.
+ */
+int
+xfs_defer_freezer_ijoin(
+ struct xfs_defer_freezer *dff,
+ struct xfs_inode *ip)
+{
+ unsigned int i;
+
+ ASSERT(xfs_isilocked(ip, XFS_ILOCK_EXCL));
+
+ for (i = 0; i < XFS_DEFER_FREEZER_INODES; i++) {
+ if (dff->dff_inodes[i] == ip)
+ goto out;
+ if (dff->dff_inodes[i] == NULL)
+ break;
+ }
+
+ if (i == XFS_DEFER_FREEZER_INODES) {
+ ASSERT(0);
+ return -EFSCORRUPTED;
+ }
+
+ /*
+ * Attach this inode to the freezer and drop its ILOCK because we
+ * assume the caller will need to allocate a transaction.
+ */
+ dff->dff_inodes[i] = ip;
+ dff->dff_ilocks[i] = 0;
+out:
+ xfs_iunlock(ip, XFS_ILOCK_EXCL);
+ return 0;
+}
diff --git a/fs/xfs/libxfs/xfs_defer.h b/fs/xfs/libxfs/xfs_defer.h
index 7ae05e10d750..0052a0313283 100644
--- a/fs/xfs/libxfs/xfs_defer.h
+++ b/fs/xfs/libxfs/xfs_defer.h
@@ -76,6 +76,11 @@ struct xfs_defer_freezer {
/* Deferred ops state saved from the transaction. */
struct list_head dff_dfops;
unsigned int dff_tpflags;
+
+ /* Inodes to hold when we want to finish the deferred work items. */
+#define XFS_DEFER_FREEZER_INODES 2
+ unsigned int dff_ilocks[XFS_DEFER_FREEZER_INODES];
+ struct xfs_inode *dff_inodes[XFS_DEFER_FREEZER_INODES];
};
/* Functions to freeze a chain of deferred operations for later. */
@@ -83,5 +88,10 @@ int xfs_defer_freeze(struct xfs_trans *tp, struct xfs_defer_freezer **dffp);
void xfs_defer_thaw(struct xfs_defer_freezer *dff, struct xfs_trans *tp);
void xfs_defer_freeezer_finish(struct xfs_mount *mp,
struct xfs_defer_freezer *dff);
+int xfs_defer_freezer_ijoin(struct xfs_defer_freezer *dff,
+ struct xfs_inode *ip);
+
+/* These functions must be provided by the xfs implementation. */
+void xfs_defer_freezer_irele(struct xfs_defer_freezer *dff);
#endif /* __XFS_DEFER_H__ */
diff --git a/fs/xfs/xfs_bmap_item.c b/fs/xfs/xfs_bmap_item.c
index c733bdeeeb9b..bbce191d8fcd 100644
--- a/fs/xfs/xfs_bmap_item.c
+++ b/fs/xfs/xfs_bmap_item.c
@@ -530,12 +530,13 @@ xfs_bui_item_recover(
}
error = xlog_recover_trans_commit(tp, dffp);
- xfs_iunlock(ip, XFS_ILOCK_EXCL);
- xfs_irele(ip);
- return error;
+ if (error)
+ goto err_rele;
+ return xfs_defer_freezer_ijoin(*dffp, ip);
err_inode:
xfs_trans_cancel(tp);
+err_rele:
if (ip) {
xfs_iunlock(ip, XFS_ILOCK_EXCL);
xfs_irele(ip);
diff --git a/fs/xfs/xfs_icache.c b/fs/xfs/xfs_icache.c
index 17a0b86fe701..b96ddf5ff334 100644
--- a/fs/xfs/xfs_icache.c
+++ b/fs/xfs/xfs_icache.c
@@ -12,6 +12,7 @@
#include "xfs_sb.h"
#include "xfs_mount.h"
#include "xfs_inode.h"
+#include "xfs_defer.h"
#include "xfs_trans.h"
#include "xfs_trans_priv.h"
#include "xfs_inode_item.h"
@@ -1847,3 +1848,21 @@ xfs_start_block_reaping(
xfs_queue_eofblocks(mp);
xfs_queue_cowblocks(mp);
}
+
+/* Release all the inode resources attached to this freezer. */
+void
+xfs_defer_freezer_irele(
+ struct xfs_defer_freezer *dff)
+{
+ unsigned int i;
+
+ for (i = 0; i < XFS_DEFER_FREEZER_INODES; i++) {
+ if (!dff->dff_inodes[i])
+ break;
+
+ if (dff->dff_ilocks[i])
+ xfs_iunlock(dff->dff_inodes[i], dff->dff_ilocks[i]);
+ xfs_irele(dff->dff_inodes[i]);
+ dff->dff_inodes[i] = NULL;
+ }
+}
