Re: [Qemu-devel] [PATCH v9 07/20] block: deprecate "encryption=on" in favor of "encrypt.format=aes"

[Alberto Garcia]

On Tue 20 Jun 2017 02:02:06 PM CEST, Daniel P. Berrange wrote:
>> > +    if (encryptfmt) {
>> > +        buf = qemu_opt_get_del(opts, BLOCK_OPT_ENCRYPT);
>> > +        if (buf != NULL) {
>> > +            g_free(buf);
>> 
>> If you use qemu_opt_get() instead then you don't need "buf" at all,
>> do you?
>
> IIRC, we needed to delete the option from opts, otherwise something
> will later complain that there are opts that are not consumed.

I don't see how, since once you reached this point it means that you
cannot create the image because the options are already wrong.

Anyway, there's a more important problem with this patch that I just
noticed: in this scenario you're freeing 'buf' twice, first in the snip
I quoted above, and then at the end of qcow2_create().

qcow_create() from qcow.c is safe, but perhaps it's clearer if you put
the declaration of the 'buf' variable inside the 'if (encryptfmt)'
block.

That assuming that you really need to use qemu_opt_get_del() there. With
the double-free bug fixed, I don't see any difference when using
qemu_opt_get_del() vs qemu_opt_get().

Berto