[Devel] [PATCH hci-8.1 2/2] dm-qcow2: fix a merge reporting success after a failed write

[Denis V. Lunev]

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

qcow2_service_iter() collects the status of its service qios in a
function-local static shared by every dm-qcow2 device, and resets it
to OK when it starts. A merge or L1 COW break starting on one device
while another one runs wipes the error the other one recorded. That
one then finishes and reports success although a qio failed, so
userspace may complete the merge and drop the top image, losing data
that never reached the lower image.

Keep the status on the stack. The qios carry a pointer to it, so the
function must not return while one of them is outstanding: every exit
already waits for service_qios to drain, and a BUG_ON after the wait
stops the kernel rather than let a late qio write into a dead stack
frame if that ever changes.

Fixes: 222b6f1291c1 ("dm-qcow2: Introduce driver to create block devices over QCOW2 files")

Signed-off-by: Denis V. Lunev <den at openvz.org>

Feature: dm-qcow2: block device over QCOW2 files driver
---
 drivers/md/dm-qcow2-cmd.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/md/dm-qcow2-cmd.c b/drivers/md/dm-qcow2-cmd.c
index 2865940cff30..c7e77677d8e6 100644
--- a/drivers/md/dm-qcow2-cmd.c
+++ b/drivers/md/dm-qcow2-cmd.c
@@ -65,13 +65,11 @@ static bool qcow2_backward_merge_should_stop(struct qcow2_target *tgt);
 static int qcow2_service_iter(struct qcow2_target *tgt, struct qcow2 *qcow2,
 		  loff_t end, loff_t step, unsigned int bi_op, u8 qio_flags)
 {
-	static blk_status_t service_status;
+	blk_status_t service_status = BLK_STS_OK;
 	struct qio *qio;
 	int ret = 0;
 	loff_t pos;
 
-	WRITE_ONCE(service_status, BLK_STS_OK);
-
 	backward_merge_update_max_progress(tgt, end);
 	for (pos = 0; pos < end; pos += step) {
 		backward_merge_update_progress(tgt, pos);
@@ -106,6 +104,8 @@ static int qcow2_service_iter(struct qcow2_target *tgt, struct qcow2 *qcow2,
 	}
 
 	wait_event(tgt->service_wq, !atomic_read(&tgt->service_qios));
+	/* The qios point at service_status on our stack */
+	BUG_ON(atomic_read(&tgt->service_qios));
 	if (!ret) {
 		smp_rmb(); /* Pairs with smp_wmb() in service_qio_endio() */
 		ret = blk_status_to_errno(READ_ONCE(service_status));
-- 
2.53.0