From patchwork Wed Jul 8 08:30:31 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ram Pai X-Patchwork-Id: 34575 Received: from vger.kernel.org (vger.kernel.org [209.132.176.167]) by demeter.kernel.org (8.14.2/8.14.2) with ESMTP id n688Uh5g009965 for ; Wed, 8 Jul 2009 08:30:43 GMT Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758094AbZGHIaj (ORCPT ); Wed, 8 Jul 2009 04:30:39 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1757685AbZGHIaj (ORCPT ); Wed, 8 Jul 2009 04:30:39 -0400 Received: from e32.co.us.ibm.com ([32.97.110.150]:53618 "EHLO e32.co.us.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757576AbZGHIag (ORCPT ); Wed, 8 Jul 2009 04:30:36 -0400 Received: from d03relay04.boulder.ibm.com (d03relay04.boulder.ibm.com [9.17.195.106]) by e32.co.us.ibm.com (8.13.1/8.13.1) with ESMTP id n688QXYU023625 for ; Wed, 8 Jul 2009 02:26:33 -0600 Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168]) by d03relay04.boulder.ibm.com (8.13.8/8.13.8/NCO v9.2) with ESMTP id n688UZpF224720 for ; Wed, 8 Jul 2009 02:30:35 -0600 Received: from d03av02.boulder.ibm.com (loopback [127.0.0.1]) by d03av02.boulder.ibm.com (8.12.11.20060308/8.13.3) with ESMTP id n688UY0m003066 for ; Wed, 8 Jul 2009 02:30:35 -0600 Received: from [9.65.200.132] (sig-9-65-200-132.mts.ibm.com [9.65.200.132]) by d03av02.boulder.ibm.com (8.12.11.20060308/8.12.11) with ESMTP id n688UWud002948; Wed, 8 Jul 2009 02:30:33 -0600 Subject: [PATCH] rev4: support colon in filenames From: Ram Pai Reply-To: linuxram@us.ibm.com To: qemu-devel@nongnu.org, kvm-devel , Jan Kiszka Cc: Anthony Liguori , Kevin Wolf In-Reply-To: <4A4CB39F.5070506@redhat.com> References: <1245862739.6278.7.camel@localhost> <1245866233.6278.17.camel@localhost> <4A434009.5010009@redhat.com> <1245998284.6278.99.camel@localhost> <4A447C8D.5000104@kevin-wolf.de> <1246063310.6278.115.camel@localhost> <1246511321.6429.31.camel@localhost> <4A4C754D.10109@redhat.com> <4A4CAD86.9020607@us.ibm.com> <4A4CB39F.5070506@redhat.com> Organization: IBM Corporation Date: Wed, 08 Jul 2009 01:30:31 -0700 Message-Id: <1247041831.6297.12.camel@localhost> Mime-Version: 1.0 X-Mailer: Evolution 2.24.3 Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org Problem: It is impossible to feed filenames with the character colon because qemu interprets such names as a protocol. For example filename scsi:0, is interpreted as a protocol by name "scsi". This patch allows user to espace colon characters. For example the above filename can now be expressed either as 'scsi\:0' or as file:scsi:0 anything following the "file:" tag is interpreted verbatin. However if "file:" tag is omitted then any colon characters in the string must be escaped using backslash. Here are couple of examples: scsi\:0\:abc is a local file scsi:0:abc http\://myweb is a local file by name http://myweb file:scsi:0:abc is a local file scsi:0:abc file:http://myweb is a local file by name http://myweb fat:c:\path\to\dir\:floppy\: is a fat file by name \path\to\dir:floppy: NOTE:The above example cannot be expressed using the "file:" protocol. Changelog w.r.t to iteration 0: 1) removes flexibility added to nbd semantics eg -- nbd:\::9999 2) introduce the file: protocol to indicate local file Changelog w.r.t to iteration 1: 1) generically handles 'file:' protocol in find_protocol 2) centralizes 'filename' pruning before the call to open(). 3) fixes buffer overflow seen in fill_token() 4) adheres to codying style 5) patch against upstream qemu tree Changelog w.r.t to iteration 2: 1) really really fixes buffer overflow seen in fill_token() (if not, beat me :) 2) the centralized 'filename' pruning had a side effect with qcow2 files and other files. Fixed it. _open() is back. Changelog w.r.t to iteration 3: 1) support added to raw-win32.c (i do not have the setup to test this change. Request help with testing) 2) ability to espace option values containing commas using backslashes eg file=file:abc,, can also be expressed as file=file:abc\, where 'abc,' is a filename 3) fixes a bug (reported by Jan Kiszka) w.r.t support for -snapshot 4) renamed _open() to qemu_open() and removed dependency on PATH_MAX the patch applies to qemu-kvm tree only. Will port the patch to qemu tree after successful test results. Signed-off-by: Ram Pai block.c | 30 +++-------------- block/raw-posix.c | 35 +++++++++++++++---- block/raw-win32.c | 26 ++++++++++++-- block/vvfat.c | 97 +++++++++++++++++++++++++++++++++++++++++++++++++++- cutils.c | 26 ++++++++++++++ qemu-common.h | 1 + qemu-option.c | 8 ++++- 7 files changed, 185 insertions(+), 38 deletions(-) --- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/block.c b/block.c index aca5a6d..be10bf9 100644 --- a/block.c +++ b/block.c @@ -225,7 +225,6 @@ static BlockDriver *find_protocol(const char *filename) { BlockDriver *drv1; char protocol[128]; - int len; const char *p; #ifdef _WIN32 @@ -233,14 +232,9 @@ static BlockDriver *find_protocol(const char *filename) is_windows_drive_prefix(filename)) return bdrv_find_format("raw"); #endif - p = strchr(filename, ':'); - if (!p) + p = prune_strcpy(protocol, sizeof(protocol), filename, ':'); + if (*p != ':') return bdrv_find_format("raw"); - len = p - filename; - if (len > sizeof(protocol) - 1) - len = sizeof(protocol) - 1; - memcpy(protocol, filename, len); - protocol[len] = '\0'; for(drv1 = first_drv; drv1 != NULL; drv1 = drv1->next) { if (drv1->protocol_name && !strcmp(drv1->protocol_name, protocol)) @@ -330,8 +324,6 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags, BlockDriver *drv) { int ret, open_flags; - char tmp_filename[PATH_MAX]; - char backing_filename[PATH_MAX]; bs->read_only = 0; bs->is_temporary = 0; @@ -343,9 +335,9 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags, if (flags & BDRV_O_SNAPSHOT) { BlockDriverState *bs1; int64_t total_size; - int is_protocol = 0; BlockDriver *bdrv_qcow2; QEMUOptionParameter *options; + char tmp_filename[PATH_MAX]; /* if snapshot, we create a temporary backing file and open it instead of opening 'filename' directly */ @@ -359,25 +351,15 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags, } total_size = bdrv_getlength(bs1) >> SECTOR_BITS; - if (bs1->drv && bs1->drv->protocol_name) - is_protocol = 1; - bdrv_delete(bs1); get_tmp_filename(tmp_filename, sizeof(tmp_filename)); - /* Real path is meaningless for protocols */ - if (is_protocol) - snprintf(backing_filename, sizeof(backing_filename), - "%s", filename); - else - realpath(filename, backing_filename); - bdrv_qcow2 = bdrv_find_format("qcow2"); options = parse_option_parameters("", bdrv_qcow2->create_options, NULL); set_option_parameter_int(options, BLOCK_OPT_SIZE, total_size * 512); - set_option_parameter(options, BLOCK_OPT_BACKING_FILE, backing_filename); + set_option_parameter(options, BLOCK_OPT_BACKING_FILE, filename); if (drv) { set_option_parameter(options, BLOCK_OPT_BACKING_FMT, drv->format_name); @@ -440,11 +422,9 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, int flags, /* if there is a backing file, use it */ BlockDriver *back_drv = NULL; bs->backing_hd = bdrv_new(""); - path_combine(backing_filename, sizeof(backing_filename), - filename, bs->backing_file); if (bs->backing_format[0] != '\0') back_drv = bdrv_find_format(bs->backing_format); - ret = bdrv_open2(bs->backing_hd, backing_filename, open_flags, + ret = bdrv_open2(bs->backing_hd, bs->backing_file, open_flags, back_drv); if (ret < 0) { bdrv_close(bs); diff --git a/block/raw-posix.c b/block/raw-posix.c index 41bfa37..fa2559c 100644 --- a/block/raw-posix.c +++ b/block/raw-posix.c @@ -124,6 +124,26 @@ static int fd_open(BlockDriverState *bs); static int cdrom_reopen(BlockDriverState *bs); #endif +static int qemu_open(const char *filename, int flags, ...) +{ + const char *f; + int len = strlen(filename)+1; + int fd, cflags; + va_list ap; + char *myfile = qemu_malloc(len); + + va_start(ap, flags); + cflags = va_arg(ap, int); + + if (!strstart(filename, "file:", &f)) { + prune_strcpy(myfile, len, filename, '\0'); + f = myfile; + } + fd = open(f, flags, cflags); + qemu_free(myfile); + return fd; +} + static int raw_open_common(BlockDriverState *bs, const char *filename, int flags) { @@ -151,7 +171,7 @@ static int raw_open_common(BlockDriverState *bs, const char *filename, s->open_flags |= O_DSYNC; s->fd = -1; - fd = open(filename, s->open_flags, 0644); + fd = qemu_open(filename, s->open_flags, 0644); if (fd < 0) { ret = -errno; if (ret == -EROFS) @@ -844,7 +864,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options) options++; } - fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, + fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644); if (fd < 0) return -EIO; @@ -889,6 +909,7 @@ static BlockDriver bdrv_raw = { .bdrv_getlength = raw_getlength, .create_options = raw_create_options, + .protocol_name = "file", }; /***********************************************/ @@ -985,7 +1006,7 @@ static int hdev_open(BlockDriverState *bs, const char *filename, int flags) if ( bsdPath[ 0 ] != '\0' ) { strcat(bsdPath,"s0"); /* some CDs don't have a partition 0 */ - fd = open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE); + fd = qemu_open(bsdPath, O_RDONLY | O_BINARY | O_LARGEFILE); if (fd < 0) { bsdPath[strlen(bsdPath)-1] = '1'; } else { @@ -1037,7 +1058,7 @@ static int fd_open(BlockDriverState *bs) #endif return -EIO; } - s->fd = open(bs->filename, s->open_flags & ~O_NONBLOCK); + s->fd = qemu_open(bs->filename, s->open_flags & ~O_NONBLOCK); if (s->fd < 0) { s->fd_error_time = qemu_get_clock(rt_clock); s->fd_got_error = 1; @@ -1133,7 +1154,7 @@ static int hdev_create(const char *filename, QEMUOptionParameter *options) options++; } - fd = open(filename, O_WRONLY | O_BINARY); + fd = qemu_open(filename, O_WRONLY | O_BINARY); if (fd < 0) return -EIO; @@ -1239,7 +1260,7 @@ static int floppy_eject(BlockDriverState *bs, int eject_flag) close(s->fd); s->fd = -1; } - fd = open(bs->filename, s->open_flags | O_NONBLOCK); + fd = qemu_open(bs->filename, s->open_flags | O_NONBLOCK); if (fd >= 0) { if (ioctl(fd, FDEJECT, 0) < 0) perror("FDEJECT"); @@ -1399,7 +1420,7 @@ static int cdrom_reopen(BlockDriverState *bs) */ if (s->fd >= 0) close(s->fd); - fd = open(bs->filename, s->open_flags, 0644); + fd = qemu_open(bs->filename, s->open_flags, 0644); if (fd < 0) { s->fd = -1; return -EIO; diff --git a/block/raw-win32.c b/block/raw-win32.c index 72acad5..b4ec4a5 100644 --- a/block/raw-win32.c +++ b/block/raw-win32.c @@ -38,6 +38,25 @@ typedef struct BDRVRawState { char drive_path[16]; /* format: "d:\" */ } BDRVRawState; +static HANDLE qemu_CreateFile(const char *filename, int access_flags, + int share_flags, LPSECURITY_ATTRIBUTES sec, + int create_flags, DWORD overlapped, HANDLE handle) +{ + const char *f; + int len = strlen(filename)+1; + int fd; + char *myfile = qemu_malloc(len); + + if (!strstart(filename, "file:", &f)) { + prune_strcpy(myfile, len, filename, '\0'); + f = myfile; + } + fd = CreateFile(filename, access_flags, share_flag, sec, + create_flags, overlapped, handle); + qemu_free(myfile); + return fd; +} + int qemu_ftruncate64(int fd, int64_t length) { LARGE_INTEGER li; @@ -96,7 +115,7 @@ static int raw_open(BlockDriverState *bs, const char *filename, int flags) overlapped |= FILE_FLAG_NO_BUFFERING | FILE_FLAG_WRITE_THROUGH; else if (!(flags & BDRV_O_CACHE_WB)) overlapped |= FILE_FLAG_WRITE_THROUGH; - s->hfile = CreateFile(filename, access_flags, + s->hfile = qemu_CreateFile(filename, access_flags, FILE_SHARE_READ, NULL, create_flags, overlapped, NULL); if (s->hfile == INVALID_HANDLE_VALUE) { @@ -223,7 +242,7 @@ static int raw_create(const char *filename, QEMUOptionParameter *options) options++; } - fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, + fd = qemu_open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY, 0644); if (fd < 0) return -EIO; @@ -255,6 +274,7 @@ static BlockDriver bdrv_raw = { .bdrv_getlength = raw_getlength, .create_options = raw_create_options, + .protocol_name = "file", }; /***********************************************/ @@ -349,7 +369,7 @@ static int hdev_open(BlockDriverState *bs, const char *filename, int flags) overlapped |= FILE_FLAG_NO_BUFFERING | FILE_FLAG_WRITE_THROUGH; else if (!(flags & BDRV_O_CACHE_WB)) overlapped |= FILE_FLAG_WRITE_THROUGH; - s->hfile = CreateFile(filename, access_flags, + s->hfile = qemu_CreateFile(filename, access_flags, FILE_SHARE_READ, NULL, create_flags, overlapped, NULL); if (s->hfile == INVALID_HANDLE_VALUE) { diff --git a/block/vvfat.c b/block/vvfat.c index 1e37b9f..6a0410e 100644 --- a/block/vvfat.c +++ b/block/vvfat.c @@ -76,6 +76,99 @@ typedef struct array_t { unsigned int size,next,item_size; } array_t; +/* + * prunes out all escape characters as per the following rule + * '\\' -> '\' + * '\:' -> ':' + * '\,' -> ',' + * '\x' -> '\x' + * return a new pruned string. + * NOTE: remember to free that string. + */ +char *escape_strdup(const char *str) +{ +#define NORMAL 0 +#define ESCAPED 1 + int len = strlen(str); + char *s = qemu_malloc(len+1); + char *q = s; + const char *p=str; + int state=NORMAL; + + while (p < str+len) { + switch (state) { + case NORMAL: + switch (*p) { + case '\\' : state=ESCAPED; + p++ ; + break; + default: *q++=*p++; + break; + } + break; + case ESCAPED: + switch (*p) { + case '\\' : + case ',' : + case ':': break; + default: *q++='\\'; + break; + } + state = NORMAL; + *q++=*p++; + break; + } + } + *q = '\0'; + return s; +} + +/* + * return the index of the rightmost delimitor in the string 'str' + */ +int find_rdelim(const char *str, const char delimitor) +{ +#define NOT_FOUND 1 +#define MAY_HAVE_FOUND 2 +#define MAY_NOT_HAVE_FOUND 3 + const char *f = str + strlen(str) -1; + char state = NOT_FOUND; + const char *loc = f; + + while (f >= str) { + char c = *f--; + switch (state) { + case NOT_FOUND: + if (c == delimitor) { + state=MAY_HAVE_FOUND; + loc=f+1; + } + break; + case MAY_HAVE_FOUND: + if (c == '\\') { + state=MAY_NOT_HAVE_FOUND; + } else { + goto out; + } + break; + case MAY_NOT_HAVE_FOUND: + if (c == '\\') { + state=MAY_HAVE_FOUND; + } else if ( c == delimitor ) { + state=MAY_HAVE_FOUND; + loc=f+1; + } else { + state=NOT_FOUND; + } + break; + } + } + loc=f; +out: + return (loc-str); +} + + static inline void array_init(array_t* array,unsigned int item_size) { array->pointer = NULL; @@ -882,7 +975,7 @@ static int init_directories(BDRVVVFATState* s, mapping->dir_index = 0; mapping->info.dir.parent_mapping_index = -1; mapping->first_mapping_index = -1; - mapping->path = strdup(dirname); + mapping->path = escape_strdup(dirname); i = strlen(mapping->path); if (i > 0 && mapping->path[i - 1] == '/') mapping->path[i - 1] = '\0'; @@ -1055,7 +1148,7 @@ DLOG(if (stderr == NULL) { bs->read_only = 0; } - i = strrchr(dirname, ':') - dirname; + i = find_rdelim(dirname, ':'); /* find the rightmost unescaped colon */ assert(i >= 3); if (dirname[i-2] == ':' && qemu_isalpha(dirname[i-1])) /* workaround for DOS drive names */ diff --git a/cutils.c b/cutils.c index 6ea0c49..09f5d39 100644 --- a/cutils.c +++ b/cutils.c @@ -24,6 +24,32 @@ #include "qemu-common.h" #include "host-utils.h" +/* + * copy contents of 'str' into buf until the first unescaped + * character 'c'. Escape character '\' is pruned off. + * Return pointer to the delimiting character + */ +const char *prune_strcpy(char *buf, int len, const char *str, const char c) +{ + const char *p=str; + char *q=buf; + + len = strnlen(str, (len > 0 ? len-1 : 0)); + while (p < str+len) { + if (*p == c) + break; + if (*p == '\\') { + p++; + if (*p == '\0') + break; + } + *q++ = *p++; + } + *q='\0'; + return p; +} + + void pstrcpy(char *buf, int buf_size, const char *str) { int c; diff --git a/qemu-common.h b/qemu-common.h index 2dcb224..230c41e 100644 --- a/qemu-common.h +++ b/qemu-common.h @@ -104,6 +104,7 @@ void qemu_get_timedate(struct tm *tm, int offset); int qemu_timedate_diff(struct tm *tm); /* cutils.c */ +const char *prune_strcpy(char *buf, int buf_size, const char *str, const char); void pstrcpy(char *buf, int buf_size, const char *str); char *pstrcat(char *buf, int buf_size, const char *s); int strstart(const char *str, const char *val, const char **ptr); diff --git a/qemu-option.c b/qemu-option.c index 646bbad..75c2fc0 100644 --- a/qemu-option.c +++ b/qemu-option.c @@ -62,7 +62,7 @@ const char *get_opt_name(char *buf, int buf_size, const char *p, char delim) * * This function is comparable to get_opt_name with the difference that the * delimiter is fixed to be comma which starts a new option. To specify an - * option value that contains commas, double each comma. + * option value that contains commas, double each comma or backslash the comma. */ const char *get_opt_value(char *buf, int buf_size, const char *p) { @@ -74,6 +74,12 @@ const char *get_opt_value(char *buf, int buf_size, const char *p) if (*(p + 1) != ',') break; p++; + } else if (*p == '\\') { + if (*(p + 1) == ',') + p++; + if (q && (q - buf) < buf_size - 1) + *q++ = *p; + p++; } if (q && (q - buf) < buf_size - 1) *q++ = *p;