Re: [PATCH] xfs: fix xfs_buf use-after-free in xfs_buf_item_unpin

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Gently ping ...

在 2023/4/20 11:35, yangerkun 写道:
From: yangerkun <yangerkun@xxxxxxxxxx>

commit 84d8949e7707 ("xfs: hold buffer across unpin and potential
shutdown processing") describle a use-after-free bug show as follow.
Call xfs_buf_hold before dec b_pin_count can forbid the problem.

    +-----------------------------+--------------------------------+
      xlog_ioend_work             | xfsaild
      ...                         |  xfs_buf_delwri_submit_buffers
       xfs_buf_item_unpin         |
        dec &bip->bli_refcount    |
        dec &bp->b_pin_count      |
                                  |  // check unpin and go on
                                  |  __xfs_buf_submit
                                  |  xfs_buf_ioend_fail // shutdown
                                  |  xfs_buf_ioend
                                  |  xfs_buf_relse
                                  |  xfs_buf_free(bp)
        xfs_buf_lock(bp) // UAF   |

However with the patch, we still get a UAF with shutdown:

    +-----------------------------+--------------------------------+
      xlog_ioend_work             |  xlog_cil_push_work // now shutdown
      ...                         |   xlog_cil_committed
       xfs_buf_item_unpin         |    ...
       // bli_refcount = 2        |
       dec bli_refcount // 1      |    xfs_buf_item_unpin
                                  |    dec bli_refcount // 0,will free
                                  |    xfs_buf_ioend_fail // free bp
       dec b_pin_count // UAF     |

xlog_cil_push_work will call xlog_cil_committed once we meet some error
like shutdown, and then call xfs_buf_item_unpin with 'remove' equals 1.
xlog_ioend_work can happened same time which trigger xfs_buf_item_unpin
too, and then bli_refcount will down to zero which trigger
xfs_buf_ioend_fail that free the xfs_buf, so the UAF can trigger.

Fix it by call xfs_buf_hold before dec bli_refcount, and release the
hold once we actually do not need it.

Signed-off-by: yangerkun <yangerkun@xxxxxxxxxx>
---
  fs/xfs/xfs_buf_item.c | 7 +++++--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_buf_item.c b/fs/xfs/xfs_buf_item.c
index df7322ed73fa..3880eb2495b8 100644
--- a/fs/xfs/xfs_buf_item.c
+++ b/fs/xfs/xfs_buf_item.c
@@ -502,12 +502,15 @@ xfs_buf_item_unpin(
  	 * completion) at any point before we return. This can be removed once
  	 * the AIL properly holds a reference on the bli.
  	 */
+	xfs_buf_hold(bp);
  	freed = atomic_dec_and_test(&bip->bli_refcount);
-	if (freed && !stale && remove)
-		xfs_buf_hold(bp);
+
  	if (atomic_dec_and_test(&bp->b_pin_count))
  		wake_up_all(&bp->b_waiters);
+ if (!freed || stale || !remove)
+		xfs_buf_rele(bp);
+
  	 /* nothing to do but drop the pin count if the bli is active */
  	if (!freed)
  		return;




[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux