Re: [Qemu-devel] [PATCH 01/10] block/dmg: properly detect the UDIF trailer

[John Snow <jsnow@redhat.com>]

On 12/27/2014 10:01 AM, Peter Wu wrote:
> DMG files have a variable length with a UDIF trailer at the end of a
> file. This UDIF trailer is essential as it describes the contents of
> the image. At the moment however, the start of this trailer is almost
> always incorrect as bdrv_getlength() returns a multiple of the block
> size (rounded up). This results in a failure to recognize DMG files,
> resulting in Invalid argument (EINVAL) errors.
>
> As there is no API to retrieve the real file size, look for the magic
> header in the last two sectors to find the start of this 512-byte UDIF
> trailer (the "koly" block).
>
> The resource fork offset ("info_begin") has its offset adjusted as the
> initial value of offset does not mean "end of file" anymore, but "begin
> of UDIF trailer".
>
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> ---
>   block/dmg.c | 40 ++++++++++++++++++++++++++++++++++++----
>   1 file changed, 36 insertions(+), 4 deletions(-)
>
> diff --git a/block/dmg.c b/block/dmg.c
> index e455886..df274f9 100644
> --- a/block/dmg.c
> +++ b/block/dmg.c
> @@ -131,6 +131,39 @@ static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk,
>       }
>   }
>
> +static int64_t dmg_find_koly_offset(BlockDriverState *file_bs)
> +{
> +    int64_t length;
> +    int64_t offset = 0;
> +    uint8_t buffer[515];
> +    int i, ret;
> +
> +    /* bdrv_getlength returns a multiple of block size (512), rounded up. Since
> +     * dmg images can have odd sizes, try to look for the "koly" magic which
> +     * marks the begin of the UDIF trailer (512 bytes). This magic can be found
> +     * in the last 511 bytes of the second-last sector or the first 4 bytes of
> +     * the last sector (search space: 515 bytes) */
> +    length = bdrv_getlength(file_bs);
> +    if (length < 512) {
> +        return length < 0 ? length : -EINVAL;
> +    }
> +    if (length > 511 + 512) {
> +        offset = length - 511 - 512;
> +    }
> +    length = length < 515 ? length : 515;
> +    ret = bdrv_pread(file_bs, offset, buffer, length);
> +    if (ret < 4) {
> +        return ret < 0 ? ret : -EINVAL;
> +    }
> +    for (i = 0; i < length - 3; i++) {
> +        if (buffer[i] == 'k' && buffer[i+1] == 'o' &&
> +            buffer[i+2] == 'l' && buffer[i+3] == 'y') {
> +            return offset + i;
> +        }
> +    }
> +    return -EINVAL;
> +}
> +
>   static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
>                       Error **errp)
>   {
> @@ -145,15 +178,14 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
>       s->n_chunks = 0;
>       s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
>
> -    /* read offset of info blocks */
> -    offset = bdrv_getlength(bs->file);
> +    /* locate the UDIF trailer */
> +    offset = dmg_find_koly_offset(bs->file);
>       if (offset < 0) {
>           ret = offset;
>           goto fail;
>       }
> -    offset -= 0x1d8;
>
> -    ret = read_uint64(bs, offset, &info_begin);
> +    ret = read_uint64(bs, offset + 0x28, &info_begin);
>       if (ret < 0) {
>           goto fail;
>       } else if (info_begin == 0) {
>

If there really is no convenient way to retrieve the real length ...
(Stefan: Would that be difficult to add?)

Reviewed-by: John Snow <jsnow@redhat.com>