xfs: fix use-after-free race in xfs_buf_rele

[mirror_ubuntu-bionic-kernel.git] / fs / xfs / xfs_buf.c

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index d8d01a0ee3c6a660d71492e2e6cb1f04e138c017..3640f6aa274ebe95e3115679b64bdbbbe39e0d58 100644 (file)
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -60,6 +60,32 @@ static kmem_zone_t *xfs_buf_zone;
 #define xb_to_gfp(flags) \
        ((((flags) & XBF_READ_AHEAD) ? __GFP_NORETRY : GFP_NOFS) | __GFP_NOWARN)
 
+/*
+ * Locking orders
+ *
+ * xfs_buf_ioacct_inc:
+ * xfs_buf_ioacct_dec:
+ *     b_sema (caller holds)
+ *       b_lock
+ *
+ * xfs_buf_stale:
+ *     b_sema (caller holds)
+ *       b_lock
+ *         lru_lock
+ *
+ * xfs_buf_rele:
+ *     b_lock
+ *       pag_buf_lock
+ *         lru_lock
+ *
+ * xfs_buftarg_wait_rele
+ *     lru_lock
+ *       b_lock (trylock due to inversion)
+ *
+ * xfs_buftarg_isolate
+ *     lru_lock
+ *       b_lock (trylock due to inversion)
+ */
 
 static inline int
 xfs_buf_is_vmapped(
@@ -985,8 +1011,18 @@ xfs_buf_rele(
 
        ASSERT(atomic_read(&bp->b_hold) > 0);
 
-       release = atomic_dec_and_lock(&bp->b_hold, &pag->pag_buf_lock);
+       /*
+        * We grab the b_lock here first to serialise racing xfs_buf_rele()
+        * calls. The pag_buf_lock being taken on the last reference only
+        * serialises against racing lookups in xfs_buf_find(). IOWs, the second
+        * to last reference we drop here is not serialised against the last
+        * reference until we take bp->b_lock. Hence if we don't grab b_lock
+        * first, the last "release" reference can win the race to the lock and
+        * free the buffer before the second-to-last reference is processed,
+        * leading to a use-after-free scenario.
+        */
        spin_lock(&bp->b_lock);
+       release = atomic_dec_and_lock(&bp->b_hold, &pag->pag_buf_lock);
        if (!release) {
                /*
                 * Drop the in-flight state if the buffer is already on the LRU