| Message ID | 1419692504-29373-2-git-send-email-peter@lekensteyn.nl |
|---|---|
| State | New |
| Headers | show |
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>
On Friday 02 January 2015 18:58:00 John Snow wrote: > > 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> The real length can be stored, but it takes more effort to do so. See the stalled work on this bdrv-getlength-conversion branch[1] and in particular "block: do not directly set total_sectors"[2].
On Sat, Dec 27, 2014 at 04:01:35PM +0100, Peter Wu wrote: > 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; dmg_open() should pass in Error *errp so a detailed error reporting can be used: if (length < 0) { error_setg_errno(errp, -length, "Failed to get file size while reading UDIF trailer"); return length; } else if (length < 512) { error_set(errp, "dmg file must be at least 512 bytes long"); return -EINVAL; } This makes it much easier to pinpoint errors (instead of just -EINVAL) and also gives the user a hint about the cause. > + } > + 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; bdrv_pread() does not return short reads. The return value will either be length or an error. This could be just: if (ret < 0) { error_setg_errno(errp, -ret, "Failed to read last sectors in dmg file"); return ret; } (The unique error string makes it easy to track down the location where the error occurs.) > + } > + 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; error_set(errp, "Not a dmg file, unable to find UDIF footer"); return -EINVAL;
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) {
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(-)