Re: [Qemu-devel] [BUG, RFC] block/vmdk.c: File name with space fails to open

[Markus Armbruster]

Philipp Hahn <address@hidden> writes:

> Hello,
>
> I tried to open a "twoGbMaxExtentSparse" VMDK file, which uses spaces in its 
> own and for the referenced file names. This breaks in line 646 of 
> block/vmdk.c because "%511s" stops at the first space and thus fname is 
> incomplete:
>         ret = sscanf(p, "%10s %" SCNd64 " %10s %511s %" SCNd64,
>                 access, &sectors, type, fname, &flat_offset);
>
> I've only checked with our very old VMware workstation version, which refuses 
> to create new images with unsupported characters with the following message:
>> The characters !#%^&*><:;'"<>/? cannot be used.
> So it looks like spaces are valid, at least we have several VMs with spaces 
> in 
> their name.
>
> If the quotes around the file name are required, the simpliest solution would 
> be to change %511s to "%511[^"]":
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 19298c2..045f6a1 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -641,7 +641,7 @@ static int vmdk_parse_extents(const char *desc, 
> BlockDriverState *bs,
>           * RW [size in sectors] SPARSE "file-name.vmdk"
>           */
>          flat_offset = -1;
> -        ret = sscanf(p, "%10s %" SCNd64 " %10s %511s %" SCNd64,
> +        ret = sscanf(p, "%10s %" SCNd64 " %10s \"%511[^\"]\" %" SCNd64,
>                  access, &sectors, type, fname, &flat_offset);
>          if (ret < 4 || strcmp(access, "RW")) {
>              goto next_line;

Suggest to include '\n' in the stop set, like \"%511[^\"\n]\", to better
detect malformed input.

> I don't know how portable %[ together with a maximum width is, because the 
> manual page for sscanf() doesn't mention "max width" for "%[", but it works 
> with Debian/GNU Linux Squeeze.

It's fine according to my reading of C89.

I'm afraid your patch is flawed.  For

    RW 1048576 FLAT ""test-f001.vmdk"" 0

fname is now "test-f001.vmdk" instead of "\"test-f001.vmdk\"".  That's
because you change sscanf() to ignore the double-quotes without dropping
the quote stripping code below.

Care to post a fixed up patch?