[Devel] [PATCH hci-8.1 v3 3/5] 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.

merge_backward complete now waits for a running seek while it holds
ctl_mutex, so target messages and the presuspend of the device queue
behind it until the scan ends.

Fixes: 412d5a4f7bf1 ("dm/dm-qcow2: add llseek_hole")
https://virtuozzo.atlassian.net/browse/VSTOR-153050
Feature: dm-qcow2: block device over QCOW2 files driver
Reviewed-by: Andrey Zhadchenko <andrey.zhadchenko@virtuozzo.com>
Reviewed-by: Konstantin Khorenko <khorenko@virtuozzo.com>
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 0e2c810f657b..45c517ce0ef8 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 ac4dbe0623ea..fa535d7716da 100644
--- a/drivers/md/dm-qcow2-map.c
+++ b/drivers/md/dm-qcow2-map.c
@@ -4844,18 +4844,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 4a0e5747b77a..7c572bd66e1c 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 8f10c5812a1b..ad5286b16883 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