[Qemu-devel] [PATCH STABLE 0/2] Backport -net socket fixes

[Qemu-devel] [PATCH STABLE 0/2] Backport -net socket fixes

[Glauber Costa]

Backport fixes for -net socket option. It fixes a segfault described
at https://bugzilla.redhat.com/show_bug.cgi?id=501264

[Qemu-devel] [PATCH STABLE 1/2] net: Fix -net socket, listen (Jan Kiszka)

[Glauber Costa]

From: aliguori <aliguori@c046a42c-6fe2-441c-8c8c-71466251a162>

In case no symbolic name is provided when requesting VLAN connection via
listening TCP socket ('-net socket,listen=...'), qemu crashes. This
fixes the cause.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

git-svn-id: svn://svn.savannah.nongnu.org/qemu/trunk@7196 c046a42c-6fe2-441c-8c8c-71466251a162

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 net.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net.c b/net.c
index ae54421..3938a02 100644
--- a/net.c
+++ b/net.c
@@ -1468,7 +1468,7 @@ static int net_socket_listen_init(VLANState *vlan,
     }
     s->vlan = vlan;
     s->model = strdup(model);
-    s->name = strdup(name);
+    s->name = name ? strdup(name) : NULL;
     s->fd = fd;
     qemu_set_fd_handler(fd, net_socket_accept, NULL, s);
     return 0;
-- 
1.5.6.6

[Qemu-devel] [PATCH STABLE 2/2] net: Fix -net socket parameter checks

[Glauber Costa]

My commit ea053add700d8abe203cd79a9ffb082aee4eabc0 broke -net socket by
overwriting an intermediate buffer in the added check_param. Fix this
by switching check_param to automatic buffer allocation and release, ie.
callers no longer have to worry about providing a scratch buffer.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 sysemu.h |    3 +--
 vl.c     |   32 +++++++++++++++++++++++---------
 2 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/sysemu.h b/sysemu.h
index 57217c1..3cc36b5 100644
--- a/sysemu.h
+++ b/sysemu.h
@@ -245,7 +245,6 @@ const char *get_opt_name(char *buf, int buf_size, const char *p);
 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(char *buf, int buf_size,
-                 const char * const *params, const char *str);
+int check_params(const char * const *params, const char *str);
 
 #endif
diff --git a/vl.c b/vl.c
index 56623fb..1a7e609 100644
--- a/vl.c
+++ b/vl.c
@@ -1918,29 +1918,43 @@ int get_param_value(char *buf, int buf_size,
     return 0;
 }
 
-int check_params(char *buf, int buf_size,
-                 const char * const *params, const char *str)
+int check_params(const char * const *params, const char *str)
 {
+    int name_buf_size = 1;
     const char *p;
-    int i;
+    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);
 
     p = str;
     for(;;) {
-        p = get_opt_name(buf, buf_size, p);
+        p = get_opt_name(name_buf, name_buf_size, p);
         if (*p != '=')
             return -1;
         p++;
         for(i = 0; params[i] != NULL; i++)
-            if (!strcmp(params[i], buf))
+            if (!strcmp(params[i], name_buf))
                 break;
-        if (params[i] == NULL)
-            return -1;
+        if (params[i] == NULL) {
+            ret = -1;
+            break;
+        }
         p = get_opt_value(NULL, 0, p);
         if (*p != ',')
             break;
         p++;
     }
-    return 0;
+
+    qemu_free(name_buf);
+    return ret;
 }
 
 /***********************************************************/
@@ -2297,7 +2311,7 @@ int drive_init(struct drive_opt *arg, int snapshot, void *opaque)
                                            "cache", "format", "serial", "werror",
                                            NULL };
 
-    if (check_params(buf, sizeof(buf), params, str) < 0) {
+    if (check_params(params, str) < 0) {
          fprintf(stderr, "qemu: unknown parameter '%s' in '%s'\n",
                          buf, str);
          return -1;
-- 
1.5.6.6

[Qemu-devel] Re: [PATCH STABLE 2/2] net: Fix -net socket parameter checks

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 607 bytes --]

Glauber Costa wrote:
> My commit ea053add700d8abe203cd79a9ffb082aee4eabc0 broke -net socket by
> overwriting an intermediate buffer in the added check_param. Fix this
> by switching check_param to automatic buffer allocation and release, ie.
> callers no longer have to worry about providing a scratch buffer.

That commit was buggy, too, and is awaiting to be fixed in unstable by
http://permalink.gmane.org/gmane.comp.emulators.qemu/43041.

But more importantly, it makes no sense in stable because its "heart",
param checking for -net is only in unstable (and that's perfectly fine).

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

[Qemu-devel] Re: [PATCH STABLE 2/2] net: Fix -net socket parameter checks

[Glauber Costa]

On Thu, May 21, 2009 at 08:23:22AM +0200, Jan Kiszka wrote:
> Glauber Costa wrote:
> > My commit ea053add700d8abe203cd79a9ffb082aee4eabc0 broke -net socket by
> > overwriting an intermediate buffer in the added check_param. Fix this
> > by switching check_param to automatic buffer allocation and release, ie.
> > callers no longer have to worry about providing a scratch buffer.
> 
> That commit was buggy, too, and is awaiting to be fixed in unstable by
> http://permalink.gmane.org/gmane.comp.emulators.qemu/43041.
> 
> But more importantly, it makes no sense in stable because its "heart",
> param checking for -net is only in unstable (and that's perfectly fine).
My interest was in the previous one, since it fixes a bug reported by a Fedora
user. I picked this one, only because it fixes the fix ;-)
But I can happily leave it out, if you think it is better.

[Qemu-devel] Re: [PATCH STABLE 2/2] net: Fix -net socket parameter checks

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1080 bytes --]

Glauber Costa wrote:
> On Thu, May 21, 2009 at 08:23:22AM +0200, Jan Kiszka wrote:
>> Glauber Costa wrote:
>>> My commit ea053add700d8abe203cd79a9ffb082aee4eabc0 broke -net socket by
>>> overwriting an intermediate buffer in the added check_param. Fix this
>>> by switching check_param to automatic buffer allocation and release, ie.
>>> callers no longer have to worry about providing a scratch buffer.
>> That commit was buggy, too, and is awaiting to be fixed in unstable by
>> http://permalink.gmane.org/gmane.comp.emulators.qemu/43041.
>>
>> But more importantly, it makes no sense in stable because its "heart",
>> param checking for -net is only in unstable (and that's perfectly fine).
> My interest was in the previous one, since it fixes a bug reported by a Fedora
> user. I picked this one, only because it fixes the fix ;-)
> But I can happily leave it out, if you think it is better.

This patch 2 has never been a fix of patch 1 in your series. I agree
that 1/2 is worth considering for stable, but that one never required
any addon fix.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

[Qemu-devel] Re: [PATCH STABLE 2/2] net: Fix -net socket parameter checks

[Glauber Costa]

On Thu, May 21, 2009 at 06:26:33PM +0200, Jan Kiszka wrote:
> Glauber Costa wrote:
> > On Thu, May 21, 2009 at 08:23:22AM +0200, Jan Kiszka wrote:
> >> Glauber Costa wrote:
> >>> My commit ea053add700d8abe203cd79a9ffb082aee4eabc0 broke -net socket by
> >>> overwriting an intermediate buffer in the added check_param. Fix this
> >>> by switching check_param to automatic buffer allocation and release, ie.
> >>> callers no longer have to worry about providing a scratch buffer.
> >> That commit was buggy, too, and is awaiting to be fixed in unstable by
> >> http://permalink.gmane.org/gmane.comp.emulators.qemu/43041.
> >>
> >> But more importantly, it makes no sense in stable because its "heart",
> >> param checking for -net is only in unstable (and that's perfectly fine).
> > My interest was in the previous one, since it fixes a bug reported by a Fedora
> > user. I picked this one, only because it fixes the fix ;-)
> > But I can happily leave it out, if you think it is better.
> 
> This patch 2 has never been a fix of patch 1 in your series. I agree
> that 1/2 is worth considering for stable, but that one never required
> any addon fix.
I got the wrong impression from your changelog then.

Thanks for taking a look

[Qemu-devel] Re: [PATCH STABLE 2/2] net: Fix -net socket parameter checks

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1423 bytes --]

Glauber Costa wrote:
> On Thu, May 21, 2009 at 06:26:33PM +0200, Jan Kiszka wrote:
>> Glauber Costa wrote:
>>> On Thu, May 21, 2009 at 08:23:22AM +0200, Jan Kiszka wrote:
>>>> Glauber Costa wrote:
>>>>> My commit ea053add700d8abe203cd79a9ffb082aee4eabc0 broke -net socket by
>>>>> overwriting an intermediate buffer in the added check_param. Fix this
>>>>> by switching check_param to automatic buffer allocation and release, ie.
>>>>> callers no longer have to worry about providing a scratch buffer.
>>>> That commit was buggy, too, and is awaiting to be fixed in unstable by
>>>> http://permalink.gmane.org/gmane.comp.emulators.qemu/43041.
>>>>
>>>> But more importantly, it makes no sense in stable because its "heart",
>>>> param checking for -net is only in unstable (and that's perfectly fine).
>>> My interest was in the previous one, since it fixes a bug reported by a Fedora
>>> user. I picked this one, only because it fixes the fix ;-)
>>> But I can happily leave it out, if you think it is better.
>> This patch 2 has never been a fix of patch 1 in your series. I agree
>> that 1/2 is worth considering for stable, but that one never required
>> any addon fix.
> I got the wrong impression from your changelog then.
> 
> Thanks for taking a look

Oh, off-by-one while picking up the hash for this changelog. Should have
been 8e4416af458b5fdfc81950005e358de02511eb9f. My bad.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]