[Devel] [PATCH hci-8.1 v5 09/11] dm-ploop: fix use-after-free on the last inflight_md decrement

[Alexander Ivanov <alexander.ivanov@virtuozzo.com>]

The last piwb reference is often dropped from the BAT write completion
in aio context. ploop_bat_write_finish() completes the data pios there,
and once the last original request is done, suspend can finish and a
table reload or remove can free the ploop in ploop_destroy(). The
completing CPU still decrements inflight_md, calls wake_up() on
inflight_md_wq and frees the piwb into ploop->pio_pool, so it can lock
a freed waitqueue or free into a destroyed mempool.

Nothing on the suspend or destroy path waits for inflight_md, and the
one waiter that exists, ploop_suspend_submitting_pios(), checks the
counter without a lock. It can see zero and return before wake_up() and
ploop_free_piwb() are done with the ploop, so the message can finish
and the table can be destroyed under the waker.

Free the piwb first, then decrement inflight_md and wake the waiters
under inflight_md_wq.lock as the last access to the ploop. Have the
waiters take the same lock once they see zero, so they cannot proceed
until the waker has released it, and wait for it in ploop_destroy()
before freeing anything.

https://virtuozzo.atlassian.net/browse/VSTOR-148171
Feature: dm-ploop: ploop target driver

Signed-off-by: Alexander Ivanov <alexander.ivanov@virtuozzo.com>
---
 drivers/md/dm-ploop-cmd.c    |  6 ++++--
 drivers/md/dm-ploop-map.c    | 22 ++++++++++++++++++----
 drivers/md/dm-ploop-target.c |  7 +++++++
 3 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/drivers/md/dm-ploop-cmd.c b/drivers/md/dm-ploop-cmd.c
index ab17520c2d56..977b6920a5b4 100644
--- a/drivers/md/dm-ploop-cmd.c
+++ b/drivers/md/dm-ploop-cmd.c
@@ -154,8 +154,10 @@ static int ploop_suspend_submitting_pios(struct ploop *ploop)
 	if (ret)
 		goto out;
 
-	ret = wait_event_interruptible(ploop->inflight_md_wq,
-				       !atomic_read(&ploop->inflight_md));
+	spin_lock_irq(&ploop->inflight_md_wq.lock);
+	ret = wait_event_interruptible_locked_irq(ploop->inflight_md_wq,
+					!atomic_read(&ploop->inflight_md));
+	spin_unlock_irq(&ploop->inflight_md_wq.lock);
 out:
 	if (ret)
 		ploop_resume_submitting_pios(ploop);
diff --git a/drivers/md/dm-ploop-map.c b/drivers/md/dm-ploop-map.c
index c313f303fff9..e2b70cb3c142 100644
--- a/drivers/md/dm-ploop-map.c
+++ b/drivers/md/dm-ploop-map.c
@@ -909,9 +909,25 @@ static void ploop_free_piwb(struct ploop_index_wb *piwb)
 
 static void ploop_bat_write_finish(struct pio *pio, void *piwb_ptr,
 				     blk_status_t bi_status);
+/*
+ * Waiters do not act on zero inflight_md without holding inflight_md_wq.lock,
+ * so the ploop stays valid while we hold it. Once we drop it, the ploop may
+ * be freed, so this must be the last access to it.
+ */
+static void ploop_dec_inflight_md(struct ploop *ploop)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&ploop->inflight_md_wq.lock, flags);
+	if (atomic_dec_and_test(&ploop->inflight_md))
+		wake_up_locked(&ploop->inflight_md_wq);
+	spin_unlock_irqrestore(&ploop->inflight_md_wq.lock, flags);
+}
+
 static void ploop_put_piwb(struct ploop_index_wb *piwb)
 {
 	if (atomic_dec_and_test(&piwb->count)) {
+		struct ploop *ploop = piwb->ploop;
 
 		ploop_bat_write_finish(piwb->pio, piwb, piwb->bi_status);
 
@@ -925,9 +941,8 @@ static void ploop_put_piwb(struct ploop_index_wb *piwb)
 		 * zero keeps it as is
 		 */
 
-		if (atomic_dec_and_test(&piwb->ploop->inflight_md))
-			wake_up(&piwb->ploop->inflight_md_wq);
 		ploop_free_piwb(piwb);
+		ploop_dec_inflight_md(ploop);
 	}
 }
 
@@ -1105,9 +1120,8 @@ void ploop_break_bat_update(struct ploop *ploop, struct md_page *md,
 	spin_unlock(&md->md_lock);
 	spin_unlock_irqrestore(&ploop->bat_lock, flags);
 
-	if (atomic_dec_and_test(&ploop->inflight_md))
-		wake_up(&ploop->inflight_md_wq);
 	ploop_free_piwb(piwb);
+	ploop_dec_inflight_md(ploop);
 }
 
 static void ploop_bat_page_zero_cluster(struct ploop *ploop,
diff --git a/drivers/md/dm-ploop-target.c b/drivers/md/dm-ploop-target.c
index 3b589579741c..dc9410212831 100644
--- a/drivers/md/dm-ploop-target.c
+++ b/drivers/md/dm-ploop-target.c
@@ -192,6 +192,13 @@ static void ploop_destroy(struct ploop *ploop)
 		if (ploop->kt_allocator)
 			kthread_stop(ploop->kt_allocator->task);
 
+		/* BAT writes may still be completing from aio context */
+		wait_event(ploop->inflight_md_wq,
+			   !atomic_read(&ploop->inflight_md));
+		/* Wait for ploop_dec_inflight_md() to drop the lock */
+		spin_lock_irq(&ploop->inflight_md_wq.lock);
+		spin_unlock_irq(&ploop->inflight_md_wq.lock);
+
 		WARN_ON(!llist_empty(&ploop->pios[PLOOP_LIST_PREPARE]));
 		WARN_ON(!llist_empty(&ploop->llresubmit_pios));
 		WARN_ON(!llist_empty(&ploop->enospc_pios));
-- 
2.53.0

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel