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

[Stefan Hajnoczi]

On Thu, Jan 24, 2013 at 05:29:27PM +0100, Philipp Hahn wrote:
> 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;
> 
> 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.

sscanf(3) is from the C standard.  I checked that C99 specifies the
length modifier for %[ in "7.19.6.2 The fscanf function" paragraph 12.

I also did a quick sample of vmdk parsers on the net.  It seems
filenames are always double-quoted.  The file format specification also
shows it this way but never explicitly states if they are optional or
not.

Your fix looks good.  Please also drop the '"' trimming code below and
resend with Signed-off-by:.

Thanks,
Stefan