[Devel] [PATCH vz10 21/24] dm-qcow2: fix NULL deref in merge_backward complete without an active merge

[Konstantin Khorenko]

qcow2_merge_backward_complete() loaded *tgt->backward_merge.pqcow2 in its
variable initializer, before checking the merge state:

	struct qcow2 *qcow2 = *tgt->backward_merge.pqcow2, *i;
	...
	if (tgt->backward_merge.state != BACKWARD_MERGE_WAIT_COMPLETION)
		return -EBUSY;

tgt->backward_merge.pqcow2 stays NULL until a "merge_backward start" message
sets it, so a `dmsetup message <dev> 0 "merge_backward complete"` on a device
that never started a backward merge dereferences NULL in the kernel and
crashes the host (NULL-ptr deref at qcow2_merge_backward_complete+0x19,
offset backward_merge(632)+pqcow2(72) = 0x2c0). Any unprivileged-of-the-VE
holder of the dm control device can trigger it.

Move the pqcow2 load below the state check. When the state really is
BACKWARD_MERGE_WAIT_COMPLETION, pqcow2 was set by "start" under the same
ctl_mutex and is never cleared while in that state, so the normal
start->...->complete path is unchanged; the no-active-merge case now returns
-EBUSY (the driver's existing "wrong merge state" convention) instead of
oopsing. The sibling verbs (cancel/update_eventfd/progress) already guard
their pqcow2 use behind the state and are not affected.

Fixes: 82e65c319889 ("dm-qcow2: allow specifying depth to merge intermediate images")
Feature: dm-qcow2: block device over QCOW2 files driver
https://virtuozzo.atlassian.net/browse/VSTOR-137234
Signed-off-by: Konstantin Khorenko <khorenko at virtuozzo.com>
---
 drivers/md/dm-qcow2-cmd.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/md/dm-qcow2-cmd.c b/drivers/md/dm-qcow2-cmd.c
index c15c46a0fe8b..a184181f7df6 100644
--- a/drivers/md/dm-qcow2-cmd.c
+++ b/drivers/md/dm-qcow2-cmd.c
@@ -366,14 +366,20 @@ void qcow2_merge_backward_work(struct work_struct *work)
 
 static int qcow2_merge_backward_complete(struct qcow2_target *tgt)
 {
-	struct qcow2 *qcow2 = *tgt->backward_merge.pqcow2, *i;
+	struct qcow2 *qcow2, *i;
 	int ret;
 
 	lockdep_assert_held(&tgt->ctl_mutex);
 
+	/*
+	 * Check the state before dereferencing pqcow2: it is NULL
+	 * until the first "merge_backward start" and is valid while
+	 * the state is BACKWARD_MERGE_WAIT_COMPLETION.
+	 */
 	if (tgt->backward_merge.state != BACKWARD_MERGE_WAIT_COMPLETION)
 		return -EBUSY;
 
+	qcow2 = *tgt->backward_merge.pqcow2;
 	*tgt->backward_merge.pqcow2 = qcow2->lower;
 
 	i = tgt->top;
-- 
2.47.1