[PATCH 0/2] xfs: shutdown UAF fixes

* [PATCH 0/2] xfs: shutdown UAF fixes
@ 2022-11-03  8:36 Guo Xuenan
  2022-11-03  8:36 ` [PATCH 1/2] xfs: wait xlog ioend workqueue drained before tearing down AIL Guo Xuenan
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Guo Xuenan @ 2022-11-03  8:36 UTC (permalink / raw)
  To: djwong, dchinner
  Cc: linux-xfs, guoxuenan, houtao1, jack.qiu, fangwei1, yi.zhang,
	zhengbin13, leo.lilong, zengheng4

Hi xfs folks:

The following patches fix some race of xfs force shutdown. 

Patch 1 fix uaf in xfs_trans_ail_delete during xlog force shutdown.
In commit cd6f79d1fb32 ("xfs: run callbacks before waking waiters in
xlog_state_shutdown_callbacks"), seems not enough to avoid UAF of AIL
before it being tear down in umount.

Patch 2 fix uaf of super block buffer log item during xlog shut down,
since xfs buf log item can be reloged, super block buffer is most
frequently modified of all xfs_buf, especially when disable lazy-count
feature, during force shutdown we will unpin and release log item, due
to xfs relog mechanism, which may release the log item alread inserted
in CIL.

I reproduce the two problems, /importantly/, adding following patches and
disable lazy-count feature to increase recurrence probability.

Kernel patch for reproduce the issue of patch 1:
```

diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index 522d450a94b1..b1221d517c00 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -503,6 +503,9 @@ xfs_buf_item_unpin(
         * the AIL properly holds a reference on the bli.
         */
        freed = atomic_dec_and_test(&bip->bli_refcount);
+       if (remove)
+               mdelay(1000);
+
        if (freed && !stale && remove)
                xfs_buf_hold(bp);
        if (atomic_dec_and_test(&bp->b_pin_count))

```

Kernel patch for reproduce the issue of patch 2:
```
diff --git a/fs/xfs/xfs_trans.c b/fs/xfs/xfs_trans.c
index 7bd16fbff534..b1aac4a7576c 100644
--- a/fs/xfs/xfs_trans.c
+++ b/fs/xfs/xfs_trans.c
@@ -25,6 +25,9 @@
 #include "xfs_dquot.h"
 #include "xfs_icache.h"
 
+#include <linux/delay.h>
+#include "xfs_log_priv.h"
+
 struct kmem_cache      *xfs_trans_cache;
 
 #if defined(CONFIG_TRACEPOINTS)
@@ -1002,6 +1005,8 @@ __xfs_trans_commit(
                xfs_trans_apply_sb_deltas(tp);
        xfs_trans_apply_dquot_deltas(tp);
 
+       if (xlog_is_shutdown(log))
+               mdelay(1000);
        xlog_cil_commit(log, tp, &commit_seq, regrant);
 
        xfs_trans_free(tp);
```

Guo Xuenan (2):
  xfs: wait xlog ioend workqueue drained before tearing down AIL
  xfs: fix super block buf log item UAF during force shutdown

 fs/xfs/xfs_buf_item.c  | 8 +++++---
 fs/xfs/xfs_trans_ail.c | 3 +++
 2 files changed, 8 insertions(+), 3 deletions(-)

-- 
2.31.1


^ permalink raw reply related	[flat|nested] 17+ messages in thread