[Devel] [PATCH hci-8.1 v2 3/3] dm-qcow2: fix SEEK_HOLE/SEEK_DATA using an image freed by a merge

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

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

qcow2_llseek_hole() starts at tgt->top and follows qcow2->lower, parsing
the metadata of each image, with nothing keeping the images alive. dm
core holds only the table SRCU around the call, which does not keep a
target message out. merge_backward complete unlinks the merged image,
waits in qcow2_inflight_ref_switch() for the qios holding refs and
destroys the image, so a seek already walking it goes on with a freed
struct qcow2, its md pages and its file. A SEEK_DATA scan of a large
sparse image keeps the walk running long enough to hit it.

Hold a per-target rw_semaphore for read over the walk, and for write
while merge_backward complete unlinks and destroys the image. An
inflight ref held over the walk would also do, but every inflight ref
switch would then wait for the whole scan, including the one starting a
backward merge with the top image paused, which stalls guest I/O for as
long as the scan runs.

Fixes: 412d5a4f7bf1 ("dm/dm-qcow2: add llseek_hole")
https://virtuozzo.atlassian.net/browse/VSTOR-153050
Feature: dm-qcow2: block device over QCOW2 files driver
Signed-off-by: Denis V. Lunev <den@openvz.org>
---
 drivers/md/dm-qcow2-cmd.c    |  2 ++
 drivers/md/dm-qcow2-map.c    | 10 ++++++++--
 drivers/md/dm-qcow2-target.c |  1 +
 drivers/md/dm-qcow2.h        |  2 ++
 4 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/md/dm-qcow2-cmd.c b/drivers/md/dm-qcow2-cmd.c
index cb28cf6456df..d9a6c823b817 100644
--- a/drivers/md/dm-qcow2-cmd.c
+++ b/drivers/md/dm-qcow2-cmd.c
@@ -373,6 +373,7 @@ static int qcow2_merge_backward_complete(struct qcow2_target *tgt)
 	if (tgt->backward_merge.state != BACKWARD_MERGE_WAIT_COMPLETION)
 		return -EBUSY;
 
+	down_write(&tgt->seek_rwsem);
 	qcow2 = *tgt->backward_merge.pqcow2;
 	*tgt->backward_merge.pqcow2 = qcow2->lower;
 
@@ -394,6 +395,7 @@ static int qcow2_merge_backward_complete(struct qcow2_target *tgt)
 			QC_ERR(tgt->ti, "Can't unuse merged img (%d)", ret);
 	}
 	qcow2_destroy(qcow2);
+	up_write(&tgt->seek_rwsem);
 
 	tgt->backward_merge.state = BACKWARD_MERGE_STOPPED;
 	__backward_merge_update_stage(tgt, BACKWARD_MERGE_STAGE_NONE);
diff --git a/drivers/md/dm-qcow2-map.c b/drivers/md/dm-qcow2-map.c
index 8ab7cb472560..b180d8ecf7b4 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -4841,18 +4841,24 @@ static int qcow2_llseek_hole_qio(struct qio *qio, int whence, loff_t *result)
 
 loff_t qcow2_llseek_hole(struct dm_target *ti, loff_t offset, int whence)
 {
-	struct qcow2 *qcow2 = to_qcow2_target(ti)->top;
+	struct qcow2_target *tgt = to_qcow2_target(ti);
 	loff_t result = -EINVAL;
+	struct qcow2 *qcow2;
 	struct qio *qio;
 	int ret;
 
+	down_read(&tgt->seek_rwsem);
+	qcow2 = tgt->top;
 	qio = alloc_seek_qio(qcow2, NULL, qcow2->hdr.size);
-	if (!qio)
+	if (!qio) {
+		up_read(&tgt->seek_rwsem);
 		return -ENOMEM;
+	}
 
 	seek_qio_set_sector(qio, to_sector(offset));
 
 	ret = qcow2_llseek_hole_qio(qio, whence, &result);
+	up_read(&tgt->seek_rwsem);
 	/* In case of error remap ENXIO as it have special meaning for llseek */
 	if (ret < 0)
 		return (ret == -ENXIO) ? -EINVAL : ret;
diff --git a/drivers/md/dm-qcow2-target.c b/drivers/md/dm-qcow2-target.c
index be09615eb8e2..343957730022 100644
--- a/drivers/md/dm-qcow2-target.c
+++ b/drivers/md/dm-qcow2-target.c
@@ -491,6 +491,7 @@ static struct qcow2_target *alloc_qcow2_target(struct dm_target *ti)
 	init_completion(&tgt->inflight_ref_comp);
 	spin_lock_init(&tgt->event_lock);
 	mutex_init(&tgt->ctl_mutex);
+	init_rwsem(&tgt->seek_rwsem);
 	init_waitqueue_head(&tgt->service_wq);
 	INIT_WORK(&tgt->event_work, qcow2_event_work);
 	INIT_LIST_HEAD(&tgt->enospc_qios);
diff --git a/drivers/md/dm-qcow2.h b/drivers/md/dm-qcow2.h
index d9b8c38e093f..de685300cb48 100644
--- a/drivers/md/dm-qcow2.h
+++ b/drivers/md/dm-qcow2.h
@@ -212,6 +212,8 @@ struct qcow2_target {
 	struct work_struct event_work;
 	spinlock_t event_lock;
 	struct mutex ctl_mutex;
+	/* Keeps merge_backward complete from freeing images a seek walks */
+	struct rw_semaphore seek_rwsem;
 
 	struct qcow2_backward_merge backward_merge;
 };
-- 
2.53.0

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