Let's fix a buffer overflow, and add the Signed-off-by line ... Rich.
-- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top
>From b22afc3464ec124d131ab21fc4270ef575c7b47d Mon Sep 17 00:00:00 2001 From: Richard Jones <rjo...@redhat.com> Date: Fri, 18 Dec 2009 15:30:03 +0000 Subject: [PATCH 1/2] VMDK4: Parse the VMDK descriptor explicitly, improve handling of CIDs The VMDK monolithic file format[1] contains an embedded text descriptor. This is at a variable location in the file, pointed to by fields in the header. An example of this embedded descriptor might be: # Disk DescriptorFile version=1 CID=12345678 parentCID=87654321 createType="monolithicSparse" # Extent description RW 16777216 SPARSE "generated-stream.vmdk" The current code assumes the descriptor is at offset 0x200 with a fixed size, and doesn't really parse the descriptor properly eg when reading and writing the CID and parentCID fields. This patch contains code to properly locate and parse the embedded descriptor in monolithic files (qemu doesn't support "split" VMDK files at all). Also this modifies the code which updates the CID fields so it doesn't rewrite the whole of the descriptor. [1] The VMDK 1.1 (ie. "VMDK4") format is partially described here: http://www.vmware.com/app/vmdk/?src=vmdk [PDF link] Signed-off-by: Richard Jones <rjo...@redhat.com> --- block/vmdk.c | 174 +++++++++++++++++++++++++++++++++++++++++++++++---------- 1 files changed, 143 insertions(+), 31 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index 4e48622..0f59ea5 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -77,6 +77,10 @@ typedef struct BDRVVmdkState { unsigned int cluster_sectors; uint32_t parent_cid; int is_parent; + + /* VMDK4 fields */ + int64_t cid_offset; + int64_t parent_cid_offset; } BDRVVmdkState; typedef struct VmdkMetaData { @@ -112,57 +116,160 @@ static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename) #define CHECK_CID 1 #define SECTOR_SIZE 512 -#define DESC_SIZE 20*SECTOR_SIZE // 20 sectors of 512 bytes each #define HEADER_SIZE 512 // first sector of 512 bytes +/* Read and parse the VMDK4 descriptor. + * + * So-called "monolithic" VMDK4 files (which are the only ones we can + * currently read) have a text descriptor section embedded within the + * file at an offset described in the header. + * + * The other format for VMDK4 files ("split") has a toplevel .vmdk + * file which is the plain text descriptor, and that points to other + * files that contain the disk data. We can't read this sort of file + * yet. + */ +#define MAX_DESC_SIZE 100*512 + +#define VMDK4_ACCESS_RW 1 +#define VMDK4_ACCESS_RDONLY 2 +#define VMDK4_ACCESS_NOACCESS 3 + +static int vmdk4_parse_desc_extent(BlockDriverState *bs, VMDK4Header *header, + int access_type, char *p) +{ + /* We should probably check that the monolithic file + * has only one extent. + */ + return 0; +} + +static int vmdk4_parse_desc_assignment(BlockDriverState *bs, + VMDK4Header *header, char *p, size_t len, + int64_t file_offset) +{ + BDRVVmdkState *s = bs->opaque; + + if (strncmp(p, "version=", 8) == 0 && strcmp(p, "version=1") != 0) + fprintf (stderr, + "warning: vmdk4_parse_desc: descriptor version != 1 (%s)\n", + p); + + if (!strncmp(p, "CID=", 4)) + s->cid_offset = file_offset + 4; + + if (!strncmp(p, "parentCID=", 10)) + s->parent_cid_offset = file_offset + 10; + + return 0; +} + +static int vmdk4_parse_desc(BlockDriverState *bs, VMDK4Header *header) +{ + BDRVVmdkState *s = bs->opaque; + int64_t desc_offset, desc_size; + char desc[MAX_DESC_SIZE]; + char *p, *pnext; + size_t len; + + s->cid_offset = 0; + s->parent_cid_offset = 0; + + /* Stored in the header are the offset/size in sectors. */ + desc_size = le64_to_cpu(header->desc_size) * SECTOR_SIZE; + desc_offset = le64_to_cpu(header->desc_offset) * SECTOR_SIZE; + + if (desc_size > MAX_DESC_SIZE) { + fprintf (stderr, + "vmdk4_parse_desc: descriptor size > maximum size " + "(%" PRIi64 " > %d)\n", desc_size, MAX_DESC_SIZE); + return -1; + } + + if (bdrv_pread(s->hd, desc_offset, (uint8_t *)desc, desc_size) != desc_size) { + fprintf (stderr, + "vmdk4_parse_desc: could not read descriptor from " + "%" PRIi64 " size %" PRIi64 "\n", + desc_offset, desc_size); + return -1; + } + + if (!strncmp(desc, "# Disk DescriptorFile", 22)) { + fprintf (stderr, "vmdk4_parse_desc: unrecognized descriptor"); + return -1; + } + + /* Parse the descriptor line by line. */ + for (p = desc; p && *p; p = pnext) { + pnext = strchr (p, '\n'); + if (pnext != NULL) { + *pnext = '\0'; + pnext++; + } + + while (*p && isspace (*p)) + p++; + while ((len = strlen (p)) > 0 && isspace (p[len-1])) + p[len-1] = '\0'; + + if (!*p) + continue; + if (p[0] == '#') + continue; + + /* Ignore any lines that we can't parse. */ + if (!strncmp(p, "RW ", 3)) + vmdk4_parse_desc_extent (bs, header, VMDK4_ACCESS_RW, p + 3); + else if (!strncmp(p, "RDONLY ", 7)) + vmdk4_parse_desc_extent (bs, header, VMDK4_ACCESS_RDONLY, p + 7); + else if (!strncmp(p, "NOACCESS ", 9)) + vmdk4_parse_desc_extent (bs, header, VMDK4_ACCESS_NOACCESS, p + 9); + else /* else expecting name = value */ + vmdk4_parse_desc_assignment (bs, header, p, len, + p-desc+desc_offset); + } + + return 0; +} + static uint32_t vmdk_read_cid(BlockDriverState *bs, int parent) { BDRVVmdkState *s = bs->opaque; - char desc[DESC_SIZE]; + int64_t offset; + char str[9]; uint32_t cid; - const char *p_name, *cid_str; - size_t cid_str_size; - /* the descriptor offset = 0x200 */ - if (bdrv_pread(s->hd, 0x200, desc, DESC_SIZE) != DESC_SIZE) - return 0; + if (parent) + offset = s->parent_cid_offset; + else + offset = s->cid_offset; - if (parent) { - cid_str = "parentCID"; - cid_str_size = sizeof("parentCID"); - } else { - cid_str = "CID"; - cid_str_size = sizeof("CID"); - } + if (offset == 0) + return 0; - if ((p_name = strstr(desc,cid_str)) != NULL) { - p_name += cid_str_size; - sscanf(p_name,"%x",&cid); - } + if (bdrv_pread(s->hd, offset, str, 8) != 8) + return 0; + str[8] = '\0'; + sscanf(str, "%x", &cid); return cid; } static int vmdk_write_cid(BlockDriverState *bs, uint32_t cid) { BDRVVmdkState *s = bs->opaque; - char desc[DESC_SIZE], tmp_desc[DESC_SIZE]; - char *p_name, *tmp_str; + int64_t offset; + char str[9]; - /* the descriptor offset = 0x200 */ - if (bdrv_pread(s->hd, 0x200, desc, DESC_SIZE) != DESC_SIZE) - return -1; + offset = s->cid_offset; + if (offset == 0) + return 0; - tmp_str = strstr(desc,"parentCID"); - pstrcpy(tmp_desc, sizeof(tmp_desc), tmp_str); - if ((p_name = strstr(desc,"CID")) != NULL) { - p_name += sizeof("CID"); - snprintf(p_name, sizeof(desc) - (p_name - desc), "%x\n", cid); - pstrcat(desc, sizeof(desc), tmp_desc); - } + snprintf(str, sizeof str, "%08x", cid); - if (bdrv_pwrite(s->hd, 0x200, desc, DESC_SIZE) != DESC_SIZE) + if (bdrv_pwrite(s->hd, offset, str, 8) != 8) return -1; + return 0; } @@ -184,6 +291,8 @@ static int vmdk_is_cid_valid(BlockDriverState *bs) return 1; } +#define DESC_SIZE 20*SECTOR_SIZE + static int vmdk_snapshot_create(const char *filename, const char *backing_file) { int snp_fd, p_fd; @@ -410,6 +519,9 @@ static int vmdk_open(BlockDriverState *bs, const char *filename, int flags) s->l1_table_offset = le64_to_cpu(header.rgd_offset) << 9; s->l1_backup_table_offset = le64_to_cpu(header.gd_offset) << 9; + if (vmdk4_parse_desc(bs, &header) == -1) + goto fail; + if (parent_open) s->is_parent = 1; else -- 1.6.5.2
