[Qemu-devel] [PATCH 04/11] net: Real fix for check_params users

[Jan Kiszka]

OK, last try: ea053add70 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>
---

 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 6a5c698..cf00c9c 100644
--- a/net.c
+++ b/net.c
@@ -1826,7 +1826,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;
@@ -1877,7 +1877,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;
@@ -1928,7 +1928,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;
@@ -1944,12 +1944,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;
@@ -1962,7 +1962,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;
@@ -1981,9 +1981,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;
@@ -1996,7 +1997,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;
@@ -2006,7 +2007,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;
@@ -2016,7 +2017,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;
@@ -2037,7 +2038,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 9bb9fbc..50438a6 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -257,6 +257,7 @@ const char *get_opt_name(char *buf, int buf_size, const 
char *p, char delim);
 const char *get_opt_value(char *buf, int buf_size, const char *p);
 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);
 
 #endif
diff --git a/vl.c b/vl.c
index 81569f0..fad386b 100644
--- a/vl.c
+++ b/vl.c
@@ -1869,45 +1869,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;
 }
 
 /***********************************************************/
@@ -2260,7 +2249,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;