[Devel] [PATCH hci-8.1 v4 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 check the counter under the same lock, so they cannot see zero
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..f673e94d0e38 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 check inflight_md under inflight_md_wq.lock, so they cannot see
+ * zero while we still hold it. Once it is zero, 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..11d25277d60c 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 */
+		spin_lock_irq(&ploop->inflight_md_wq.lock);
+		wait_event_lock_irq(ploop->inflight_md_wq,
+				    !atomic_read(&ploop->inflight_md),
+				    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