Re: [Devel] [PATCH hci-8.1 v5 1/3] dm-qcow2: add qcow2_md_page_read_sync() helper

[Konstantin Khorenko <khorenko@virtuozzo.com>]

> + * The caller must ensure no read of this page is in flight
> + * (e.g., the device is suspended): if the page is cached but
> + * not MD_UPTODATE yet, qcow2_md_page_find_or_postpone() returns
> + * NULL and the following insert fails.

The requirement is stronger: nobody else may look this page up while
the helper runs. A qio that finds the page before MD_UPTODATE is set
gets queued on md->wait_list, which this helper never wakes. On the
error path the page is freed with that qio still on it. Neither caller
can hit this today, but the comment should say "no concurrent lookup"
rather than "no read in flight".

> +     ret = qcow2_rw_page_sync(READ, qcow2, id, (*md)->page);
> +     spin_lock_irq(&qcow2->md_pages_lock);
> +     if (ret)
> +             qcow2_md_page_erase(qcow2, *md);
> +     else
> +             (*md)->status |= MD_UPTODATE;
> +     spin_unlock_irq(&qcow2->md_pages_lock);
> +     if (ret)
> +             qcow2_free_md_page(*md);
> +     return ret;

On both error paths *md is left pointing to freed memory: here, and on
-EEXIST from qcow2_alloc_and_insert_md_page(). Please set *md = NULL
there.

-- 
Konstantin Khorenko <khorenko@virtuozzo.com>
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel