[Qemu-devel] [PATCH] rev7: support colon in filenames

[Ram Pai]

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

Changelog w.r.t to iteration 4:
   1) applies to upstream qemu tree

Changelog w.r.t to iteration 5:
   1) fixed a issue with backing_filename for qcow2 files,
                reported by Jamie Lokier.
   2) fixed a compile issue with win32-raw.c reported by Blue Swirl.
                (I do not have the setup to test win32 changes. 
                 Request help with testing)

Changelog w.r.t to iteration 6:
   1) fixed all the issues found with win32. 
        a) changed the call to strnlen() to qemu_strlen() in cutils.c
        b) fixed the call to CreateFile() in qemu_CreateFile()

Signed-off-by: Ram Pai <address@hidden>
         

 block.c           |   38 ++++++++-------------
 block/raw-posix.c |   15 ++++----
 block/raw-win32.c |   26 ++++++++++++--
 block/vvfat.c     |   97 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 cutils.c          |   46 +++++++++++++++++++++++++
 qemu-common.h     |    2 +
 qemu-option.c     |    8 ++++-
 7 files changed, 195 insertions(+), 37 deletions(-)

diff --git a/block.c b/block.c
index 39f726c..da6eaf7 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))
@@ -331,7 +325,6 @@ int bdrv_open2(BlockDriverState *bs, const char *filename, 
int flags,
 {
     int ret, open_flags;
     char tmp_filename[PATH_MAX];
-    char backing_filename[PATH_MAX];
 
     bs->read_only = 0;
     bs->is_temporary = 0;
@@ -343,7 +336,6 @@ 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;
 
@@ -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);
@@ -437,15 +419,23 @@ int bdrv_open2(BlockDriverState *bs, const char 
*filename, int flags,
     }
 #endif
     if (bs->backing_file[0] != '\0') {
+        char *backing_file;
+
         /* if there is a backing file, use it */
         BlockDriver *back_drv = NULL;
         bs->backing_hd = bdrv_new("");
-        path_combine(backing_filename, sizeof(backing_filename),
+
+        if (flags & BDRV_O_SNAPSHOT) {
+            backing_file = bs->backing_file;
+        } else {
+            path_combine(tmp_filename, sizeof(tmp_filename),
                      filename, bs->backing_file);
+            backing_file = tmp_filename;
+        }
+
         if (bs->backing_format[0] != '\0')
             back_drv = bdrv_find_format(bs->backing_format);
-        ret = bdrv_open2(bs->backing_hd, backing_filename, open_flags,
-                         back_drv);
+        ret = bdrv_open2(bs->backing_hd, backing_file, open_flags, back_drv);
         if (ret < 0) {
             bdrv_close(bs);
             return ret;
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 389903e..030796d 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -154,7 +154,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)
@@ -863,7 +863,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) {
         result = -errno;
@@ -914,6 +914,7 @@ static BlockDriver bdrv_raw = {
     .bdrv_getlength = raw_getlength,
 
     .create_options = raw_create_options,
+    .protocol_name = "file",
 };
 
 /***********************************************/
@@ -1010,7 +1011,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 {
@@ -1062,7 +1063,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;
@@ -1158,7 +1159,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;
 
@@ -1263,7 +1264,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");
@@ -1422,7 +1423,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..24d706e 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;
+    HANDLE fd;
+    char *myfile = qemu_malloc(len);
+
+    if (!strstart(filename, "file:", &f)) {
+        prune_strcpy(myfile, len, filename, '\0');
+        f = myfile;
+    }
+    fd = CreateFile(f, access_flags, share_flags, 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..36fd516 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.
+ */
+static 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'
+ */
+static 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 bd9a019..9da3d1f 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 = qemu_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;
@@ -192,3 +218,23 @@ void qemu_iovec_from_buffer(QEMUIOVector *qiov, const void 
*buf, size_t count)
         count -= copy;
     }
 }
+
+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;
+}
diff --git a/qemu-common.h b/qemu-common.h
index 6a15f89..94db4a0 100644
--- a/qemu-common.h
+++ b/qemu-common.h
@@ -104,11 +104,13 @@ 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);
 int stristart(const char *str, const char *val, const char **ptr);
 int qemu_strnlen(const char *s, int max_len);
+int qemu_open(const char *filename, int flags, ...);
 time_t mktimegm(struct tm *tm);
 int qemu_fls(int i);
 
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;