qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 08/12] block/dmg: fix sector data offset calc


From: John Snow
Subject: Re: [Qemu-devel] [PATCH v2 08/12] block/dmg: fix sector data offset calculation
Date: Wed, 07 Jan 2015 13:08:24 -0500
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0



On 01/06/2015 12:48 PM, Peter Wu wrote:
This patch addresses two issues:

  - The data fork offset was not taken into account, resulting in failure
    to read an InstallESD.dmg file (5164763151 bytes) which had a
    non-zero DataForkOffset field.
  - The offset of the previous block ("partition") was unconditionally
    added to the current block because older files would start the input
    offset of a new block at zero. Newer files (including vlc-2.1.5.dmg,
    tuxpaint-0.9.15-macosx.dmg and OS X Yosemite [MAS].dmg) failed in
    reads because these files have chunk offsets, relative to the begin
    of a data fork.

Now the data offset of the mish is taken into account. While we could
check that the data_offset is within the data fork, let's not do that
here as it would only result in parse failures on invalid files (rather
than gracefully handling such bad files). dmg_read will error out if
the offset is incorrect.

Signed-off-by: Peter Wu <address@hidden>
---
  v2: use sector and data offset as provided by the BLKX header. This
      allows us to drop last_in_offset. The previous heuristics to detect
      relative offsets is not needed anymore. Squashed the data fork
      offset length check into this patch.
---
  block/dmg.c | 26 ++++++++++++++++++++------
  1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/block/dmg.c b/block/dmg.c
index 57feb1b..130efac 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -188,7 +188,7 @@ static int64_t dmg_find_koly_offset(BlockDriverState 
*file_bs, Error **errp)
  typedef struct DmgHeaderState {
      /* used internally by dmg_read_mish_block to remember offsets of blocks
       * across calls */
-    uint64_t last_in_offset;
+    uint64_t data_fork_offset;
      uint64_t last_out_offset;
      /* exported for dmg_open */
      uint32_t max_compressed_size;
@@ -203,6 +203,8 @@ static int dmg_read_mish_block(BDRVDMGState *s, 
DmgHeaderState *ds,
      size_t new_size;
      uint32_t chunk_count;
      int64_t offset = 0;
+    uint64_t data_offset;
+    uint64_t in_offset = ds->data_fork_offset;

      type = buff_read_uint32(buffer, offset);
      /* skip data that is not a valid MISH block (invalid magic or too small) 
*/
@@ -211,8 +213,12 @@ static int dmg_read_mish_block(BDRVDMGState *s, 
DmgHeaderState *ds,
          return 0;
      }

-    offset += 4;
-    offset += 200;
+    /* location in data fork for (compressed) blob (in bytes) */
+    data_offset = buff_read_uint64(buffer, offset + 0x18);
+    in_offset += data_offset;
+
+    /* move to begin of chunk entries */
+    offset += 204;

      chunk_count = (count - 204) / 40;
      new_size = sizeof(uint64_t) * (s->n_chunks + chunk_count);
@@ -228,7 +234,6 @@ static int dmg_read_mish_block(BDRVDMGState *s, 
DmgHeaderState *ds,
          if (s->types[i] != 0x80000005 && s->types[i] != 1 &&
              s->types[i] != 2) {
              if (s->types[i] == 0xffffffff && i > 0) {
-                ds->last_in_offset = s->offsets[i - 1] + s->lengths[i - 1];

Great!

                  ds->last_out_offset = s->sectors[i - 1] +
                                        s->sectorcounts[i - 1];
              }
@@ -255,7 +260,7 @@ static int dmg_read_mish_block(BDRVDMGState *s, 
DmgHeaderState *ds,
          }

          s->offsets[i] = buff_read_uint64(buffer, offset);
-        s->offsets[i] += ds->last_in_offset;
+        s->offsets[i] += in_offset;
          offset += 8;

          s->lengths[i] = buff_read_uint64(buffer, offset);
@@ -413,7 +418,7 @@ static int dmg_open(BlockDriverState *bs, QDict *options, 
int flags,
      s->n_chunks = 0;
      s->offsets = s->lengths = s->sectors = s->sectorcounts = NULL;
      /* used by dmg_read_mish_block to keep track of the current I/O position 
*/
-    ds.last_in_offset = 0;
+    ds.data_fork_offset = 0;
      ds.last_out_offset = 0;
      ds.max_compressed_size = 1;
      ds.max_sectors_per_chunk = 1;
@@ -425,6 +430,15 @@ static int dmg_open(BlockDriverState *bs, QDict *options, 
int flags,
          goto fail;
      }

+    /* offset of data fork (DataForkOffset) */
+    ret = read_uint64(bs, offset + 0x18, &ds.data_fork_offset);
+    if (ret < 0) {
+        goto fail;
+    } else if (ds.data_fork_offset > offset) {
+        ret = -EINVAL;
+        goto fail;
+    }
+
      /* offset of resource fork (RsrcForkOffset) */
      ret = read_uint64(bs, offset + 0x28, &rsrc_fork_offset);
      if (ret < 0) {


Reviewed-by: John Snow <address@hidden>



reply via email to

[Prev in Thread] Current Thread [Next in Thread]