[Devel] [PATCH hci-8.1 v2 2/3] dm-qcow2: fix backward merge of an inner image racing its reads

["Denis V. Lunev" <den@openvz.org>]

From: Denis V. Lunev <den@openvz.org>

set_backward_merge_in_process() sets the flag of an image other than the
top one without stopping submission, as nothing writes to it. It is
still read: a read of a cluster the top image does not map goes on to
the lower image, and when that image is writable its worker tests the
flag twice for the qio. When the merge starts between the two tests, the
first one, in handle_metadata(), skips prepare_backward_merge() and
leaves qio->ext NULL, the second one, in process_one_qio(), takes the
merge path and submit_top_delta_read() dereferences qio->ext.

Pause submission and switch the inflight refs for every image. The
pause flag is only tested on the top image, where all qios enter, so
pause that one; clearing the flag of an inner image paused the inner
image itself, which held nothing back.

Fixes: 8e522723a085 ("dm-qcow2: allow specifying depth to merge intermediate images")
https://virtuozzo.atlassian.net/browse/VSTOR-153049
Feature: dm-qcow2: block device over QCOW2 files driver
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 drivers/md/dm-qcow2-cmd.c | 26 ++++++++++----------------
 1 file changed, 10 insertions(+), 16 deletions(-)

diff --git a/drivers/md/dm-qcow2-cmd.c b/drivers/md/dm-qcow2-cmd.c
index e520d8fd6929..cb28cf6456df 100644
--- a/drivers/md/dm-qcow2-cmd.c
+++ b/drivers/md/dm-qcow2-cmd.c
@@ -149,34 +149,28 @@ static int qcow2_break_l1cow(struct qcow2_target *tgt, struct qcow2 *qcow2)
 static void set_backward_merge_in_process(struct qcow2_target *tgt,
 				     struct qcow2 *qcow2, bool set)
 {
+	struct qcow2 *top = tgt->top;
 	LIST_HEAD(list);
 
-	/*
-	 * There are no writes if it is not the top qcow2 image.
-	 * so we do not need to stop and flush requests when setting
-	 */
-	if (qcow2 != tgt->top && set) {
-		qcow2->backward_merge_in_process = set;
-		return;
-	}
-
 	/*
 	 * To avoid race between allocations and COWS
 	 * we completely stop queueing qios and wait
 	 * for pending qios. Lock is for visability.
+	 * A lower image has no writes, but it is read by
+	 * qios of the top one, so stop the top image even then.
 	 */
-	spin_lock_irq(&qcow2->deferred_lock);
-	qcow2->pause_submitting_qios = true;
-	spin_unlock_irq(&qcow2->deferred_lock);
+	spin_lock_irq(&top->deferred_lock);
+	top->pause_submitting_qios = true;
+	spin_unlock_irq(&top->deferred_lock);
 	qcow2_inflight_ref_switch(tgt);
 
 	/* queue is stopped */
-	spin_lock_irq(&qcow2->deferred_lock);
+	spin_lock_irq(&top->deferred_lock);
 	WARN_ON_ONCE(qcow2->backward_merge_in_process == set);
 	qcow2->backward_merge_in_process = set;
-	qcow2->pause_submitting_qios = false;
-	list_splice_init(&qcow2->paused_qios, &list);
-	spin_unlock_irq(&qcow2->deferred_lock);
+	top->pause_submitting_qios = false;
+	list_splice_init(&top->paused_qios, &list);
+	spin_unlock_irq(&top->deferred_lock);
 
 	qcow2_submit_embedded_qios(tgt, &list);
 }
-- 
2.53.0

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