Re: [libvirt] [PATCH 12/30] storagefile: Fix backing format \0 check

On 10/7/19 11:49 PM, Cole Robinson wrote: >> From qemu.git docs/interop/qcow2.txt > >    == String header extensions == > >    Some header extensions (such as the backing file format name and >    the external data file name) are just a single string. In this case, >    the header extension length is the string length and the string is >    not '\0' terminated. (The header extension padding can make it look >    like a string is '\0' terminated, but neither is padding always >    necessary nor is there a guarantee that zero bytes are used >    for padding.) > > So we shouldn't be checking for a \0 byte at the end of the backing > format section. I think in practice there always is a \0 but we > shouldn't depend on that. > > Signed-off-by: Cole Robinson <crobinso(a)redhat.com&gt; > --- >   src/util/virstoragefile.c | 13 +++++++++---- >   1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c > index 8cd576a463..5a4e4b24ae 100644 > --- a/src/util/virstoragefile.c > +++ b/src/util/virstoragefile.c > @@ -506,13 +506,18 @@ qcow2GetExtensions(const char *buf, >           case QCOW2_HDR_EXTENSION_END: >               goto done; > -        case QCOW2_HDR_EXTENSION_BACKING_FORMAT: > -            if (buf[offset+len] != '\0') > -                break; > -            *backingFormat = > virStorageFileFormatTypeFromString(buf+offset); > +        case QCOW2_HDR_EXTENSION_BACKING_FORMAT: { > +            VIR_AUTOFREE(char *) tmp = NULL; > +            if (VIR_ALLOC_N(tmp, len + 1) < 0) > +                return -1; > +            memcpy(tmp, buf + offset, len); > +            tmp[len] = '\0'; > + > +            *backingFormat = virStorageFileFormatTypeFromString(tmp); >               if (*backingFormat <= VIR_STORAGE_FILE_NONE) >                   return -1; >           } > +        } Pre-existing, but there's missing 'break;' here. Since you're touching these lines, might be worth putting it here.