[Qemu-devel] [PATCH 2/3] net: Real fix for check_params users

[Mark McLoughlin]

From: Jan Kiszka <address@hidden>

OK, last try: 8e4416af45 broke -net socket, ffad4116b9 tried to fix it
but broke error reporting of invalid parameters. So this patch widely
reverts ffad4116b9 again and intead fixes those callers of check_params
that originally suffered from overwritten buffers by using separate
ones.

Signed-off-by: Jan Kiszka <address@hidden>
Signed-off-by: Mark McLoughlin <address@hidden>
---
 net.c    |   23 ++++++++++++-----------
 sysemu.h |    3 ++-
 vl.c     |   39 ++++++++++++++-------------------------
 3 files changed, 28 insertions(+), 37 deletions(-)

diff --git a/net.c b/net.c
index 390d6a6..723e934 100644
--- a/net.c
+++ b/net.c
@@ -1911,7 +1911,7 @@ int net_client_init(const char *device, const char *p)
         uint8_t *macaddr;
         int idx = nic_get_free_idx();
 
-        if (check_params(nic_params, p) < 0) {
+        if (check_params(buf, sizeof(buf), nic_params, p) < 0) {
             fprintf(stderr, "qemu: invalid parameter '%s' in '%s'\n",
                     buf, p);
             return -1;
@@ -1962,7 +1962,7 @@ int net_client_init(const char *device, const char *p)
         static const char * const slirp_params[] = {
             "vlan", "name", "hostname", "restrict", "ip", NULL
         };
-        if (check_params(slirp_params, p) < 0) {
+        if (check_params(buf, sizeof(buf), slirp_params, p) < 0) {
             fprintf(stderr, "qemu: invalid parameter '%s' in '%s'\n",
                     buf, p);
             return -1;
@@ -2013,7 +2013,7 @@ int net_client_init(const char *device, const char *p)
         };
         char ifname[64];
 
-        if (check_params(tap_params, p) < 0) {
+        if (check_params(buf, sizeof(buf), tap_params, p) < 0) {
             fprintf(stderr, "qemu: invalid parameter '%s' in '%s'\n",
                     buf, p);
             return -1;
@@ -2029,12 +2029,12 @@ int net_client_init(const char *device, const char *p)
 #elif defined (_AIX)
 #else
     if (!strcmp(device, "tap")) {
-        char ifname[64];
+        char ifname[64], chkbuf[64];
         char setup_script[1024], down_script[1024];
         int fd;
         vlan->nb_host_devs++;
         if (get_param_value(buf, sizeof(buf), "fd", p) > 0) {
-            if (check_params(fd_params, p) < 0) {
+            if (check_params(chkbuf, sizeof(chkbuf), fd_params, p) < 0) {
                 fprintf(stderr, "qemu: invalid parameter '%s' in '%s'\n",
                         buf, p);
                 return -1;
@@ -2047,7 +2047,7 @@ int net_client_init(const char *device, const char *p)
             static const char * const tap_params[] = {
                 "vlan", "name", "ifname", "script", "downscript", NULL
             };
-            if (check_params(tap_params, p) < 0) {
+            if (check_params(chkbuf, sizeof(chkbuf), tap_params, p) < 0) {
                 fprintf(stderr, "qemu: invalid parameter '%s' in '%s'\n",
                         buf, p);
                 return -1;
@@ -2066,9 +2066,10 @@ int net_client_init(const char *device, const char *p)
     } else
 #endif
     if (!strcmp(device, "socket")) {
+        char chkbuf[64];
         if (get_param_value(buf, sizeof(buf), "fd", p) > 0) {
             int fd;
-            if (check_params(fd_params, p) < 0) {
+            if (check_params(chkbuf, sizeof(chkbuf), fd_params, p) < 0) {
                 fprintf(stderr, "qemu: invalid parameter '%s' in '%s'\n",
                         buf, p);
                 return -1;
@@ -2081,7 +2082,7 @@ int net_client_init(const char *device, const char *p)
             static const char * const listen_params[] = {
                 "vlan", "name", "listen", NULL
             };
-            if (check_params(listen_params, p) < 0) {
+            if (check_params(chkbuf, sizeof(chkbuf), listen_params, p) < 0) {
                 fprintf(stderr, "qemu: invalid parameter '%s' in '%s'\n",
                         buf, p);
                 return -1;
@@ -2091,7 +2092,7 @@ int net_client_init(const char *device, const char *p)
             static const char * const connect_params[] = {
                 "vlan", "name", "connect", NULL
             };
-            if (check_params(connect_params, p) < 0) {
+            if (check_params(chkbuf, sizeof(chkbuf), connect_params, p) < 0) {
                 fprintf(stderr, "qemu: invalid parameter '%s' in '%s'\n",
                         buf, p);
                 return -1;
@@ -2101,7 +2102,7 @@ int net_client_init(const char *device, const char *p)
             static const char * const mcast_params[] = {
                 "vlan", "name", "mcast", NULL
             };
-            if (check_params(mcast_params, p) < 0) {
+            if (check_params(chkbuf, sizeof(chkbuf), mcast_params, p) < 0) {
                 fprintf(stderr, "qemu: invalid parameter '%s' in '%s'\n",
                         buf, p);
                 return -1;
@@ -2122,7 +2123,7 @@ int net_client_init(const char *device, const char *p)
         char vde_sock[1024], vde_group[512];
        int vde_port, vde_mode;
 
-        if (check_params(vde_params, p) < 0) {
+        if (check_params(buf, sizeof(buf), vde_params, p) < 0) {
             fprintf(stderr, "qemu: invalid parameter '%s' in '%s'\n",
                     buf, p);
             return -1;
diff --git a/sysemu.h b/sysemu.h
index 92501ed..b57f6bb 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -267,7 +267,8 @@ void usb_info(Monitor *mon);
 
 int get_param_value(char *buf, int buf_size,
                     const char *tag, const char *str);
-int check_params(const char * const *params, const char *str);
+int check_params(char *buf, int buf_size,
+                 const char * const *params, const char *str);
 
 void register_devices(void);
 
diff --git a/vl.c b/vl.c
index 975e811..4d04d65 100644
--- a/vl.c
+++ b/vl.c
@@ -1835,45 +1835,34 @@ int get_param_value(char *buf, int buf_size,
     return 0;
 }
 
-int check_params(const char * const *params, const char *str)
+int check_params(char *buf, int buf_size,
+                 const char * const *params, const char *str)
 {
-    int name_buf_size = 1;
     const char *p;
-    char *name_buf;
-    int i, len;
-    int ret = 0;
-
-    for (i = 0; params[i] != NULL; i++) {
-        len = strlen(params[i]) + 1;
-        if (len > name_buf_size) {
-            name_buf_size = len;
-        }
-    }
-    name_buf = qemu_malloc(name_buf_size);
+    int i;
 
     p = str;
     while (*p != '\0') {
-        p = get_opt_name(name_buf, name_buf_size, p, '=');
+        p = get_opt_name(buf, buf_size, p, '=');
         if (*p != '=') {
-            ret = -1;
-            break;
+            return -1;
         }
         p++;
-        for(i = 0; params[i] != NULL; i++)
-            if (!strcmp(params[i], name_buf))
+        for (i = 0; params[i] != NULL; i++) {
+            if (!strcmp(params[i], buf)) {
                 break;
+            }
+        }
         if (params[i] == NULL) {
-            ret = -1;
-            break;
+            return -1;
         }
         p = get_opt_value(NULL, 0, p);
-        if (*p != ',')
+        if (*p != ',') {
             break;
+        }
         p++;
     }
-
-    qemu_free(name_buf);
-    return ret;
+    return 0;
 }
 
 /***********************************************************/
@@ -2226,7 +2215,7 @@ int drive_init(struct drive_opt *arg, int snapshot, void 
*opaque)
                                            "cache", "format", "serial", 
"werror",
                                            NULL };
 
-    if (check_params(params, str) < 0) {
+    if (check_params(buf, sizeof(buf), params, str) < 0) {
          fprintf(stderr, "qemu: unknown parameter '%s' in '%s'\n",
                          buf, str);
          return -1;
-- 
1.6.2.2