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

[Pavel Tikhomirov]

Reviewed-by: Pavel Tikhomirov <ptikhomirov at virtuozzo.com>

On 7/6/26 12:59, Konstantin Khorenko wrote:
> 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;

-- 
Best regards, Pavel Tikhomirov
Senior Software Developer, Virtuozzo.