qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Gerd Hoffmann <kraxel@redhat.com>
To: qemu-devel@nongnu.org
Cc: Gerd Hoffmann <kraxel@redhat.com>,
	Anthony Liguori <aliguori@amazon.com>,
	Kirill Batuzov <batuzovk@ispras.ru>
Subject: [Qemu-devel] [PULL 1/5] console: Abort on property access errors
Date: Mon, 26 May 2014 10:39:26 +0200	[thread overview]
Message-ID: <1401093570-4161-2-git-send-email-kraxel@redhat.com> (raw)
In-Reply-To: <1401093570-4161-1-git-send-email-kraxel@redhat.com>

From: Kirill Batuzov <batuzovk@ispras.ru>

All defined properties of QemuConsole are mandatory and no access to them
should fail. Nevertheless not checking returned errors is bad because in case
of unexpected failure it will hide the bug and cause a memory leak.

Abort in case of unexpected property access errors. This change exposed a bug
where an attempt was made to write to a read-only property "head".

Set "head" property's value at creation time and do not attempt to change it
later. This fixes the bug mentioned above.

Signed-off-by: Kirill Batuzov <batuzovk@ispras.ru>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/console.c | 30 +++++++++++++-----------------
 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/ui/console.c b/ui/console.c
index 34d1eaa..c92df8b 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1167,9 +1167,9 @@ static void text_console_update(void *opaque, console_ch_t *chardata)
     }
 }
 
-static QemuConsole *new_console(DisplayState *ds, console_type_t console_type)
+static QemuConsole *new_console(DisplayState *ds, console_type_t console_type,
+                                uint32_t head)
 {
-    Error *local_err = NULL;
     Object *obj;
     QemuConsole *s;
     int i;
@@ -1179,13 +1179,14 @@ static QemuConsole *new_console(DisplayState *ds, console_type_t console_type)
 
     obj = object_new(TYPE_QEMU_CONSOLE);
     s = QEMU_CONSOLE(obj);
+    s->head = head;
     object_property_add_link(obj, "device", TYPE_DEVICE,
                              (Object **)&s->device,
                              object_property_allow_set_link,
                              OBJ_PROP_LINK_UNREF_ON_RELEASE,
-                             &local_err);
+                             &error_abort);
     object_property_add_uint32_ptr(obj, "head",
-                                   &s->head, &local_err);
+                                   &s->head, &error_abort);
 
     if (!active_console || ((active_console->console_type != GRAPHIC_CONSOLE) &&
         (console_type == GRAPHIC_CONSOLE))) {
@@ -1560,7 +1561,6 @@ static DisplayState *get_alloc_displaystate(void)
  */
 DisplayState *init_displaystate(void)
 {
-    Error *local_err = NULL;
     gchar *name;
     int i;
 
@@ -1579,7 +1579,7 @@ DisplayState *init_displaystate(void)
          * doesn't change any more */
         name = g_strdup_printf("console[%d]", i);
         object_property_add_child(container_get(object_get_root(), "/backend"),
-                                  name, OBJECT(consoles[i]), &local_err);
+                                  name, OBJECT(consoles[i]), &error_abort);
         g_free(name);
     }
 
@@ -1590,7 +1590,6 @@ QemuConsole *graphic_console_init(DeviceState *dev, uint32_t head,
                                   const GraphicHwOps *hw_ops,
                                   void *opaque)
 {
-    Error *local_err = NULL;
     int width = 640;
     int height = 480;
     QemuConsole *s;
@@ -1598,14 +1597,12 @@ QemuConsole *graphic_console_init(DeviceState *dev, uint32_t head,
 
     ds = get_alloc_displaystate();
     trace_console_gfx_new();
-    s = new_console(ds, GRAPHIC_CONSOLE);
+    s = new_console(ds, GRAPHIC_CONSOLE, head);
     s->hw_ops = hw_ops;
     s->hw = opaque;
     if (dev) {
-        object_property_set_link(OBJECT(s), OBJECT(dev),
-                                 "device", &local_err);
-        object_property_set_int(OBJECT(s), head,
-                                "head", &local_err);
+        object_property_set_link(OBJECT(s), OBJECT(dev), "device",
+                                 &error_abort);
     }
 
     s->surface = qemu_create_displaysurface(width, height);
@@ -1622,7 +1619,6 @@ QemuConsole *qemu_console_lookup_by_index(unsigned int index)
 
 QemuConsole *qemu_console_lookup_by_device(DeviceState *dev, uint32_t head)
 {
-    Error *local_err = NULL;
     Object *obj;
     uint32_t h;
     int i;
@@ -1632,12 +1628,12 @@ QemuConsole *qemu_console_lookup_by_device(DeviceState *dev, uint32_t head)
             continue;
         }
         obj = object_property_get_link(OBJECT(consoles[i]),
-                                       "device", &local_err);
+                                       "device", &error_abort);
         if (DEVICE(obj) != dev) {
             continue;
         }
         h = object_property_get_int(OBJECT(consoles[i]),
-                                    "head", &local_err);
+                                    "head", &error_abort);
         if (h != head) {
             continue;
         }
@@ -1811,9 +1807,9 @@ static CharDriverState *text_console_init(ChardevVC *vc)
 
     trace_console_txt_new(width, height);
     if (width == 0 || height == 0) {
-        s = new_console(NULL, TEXT_CONSOLE);
+        s = new_console(NULL, TEXT_CONSOLE, 0);
     } else {
-        s = new_console(NULL, TEXT_CONSOLE_FIXED_SIZE);
+        s = new_console(NULL, TEXT_CONSOLE_FIXED_SIZE, 0);
         s->surface = qemu_create_displaysurface(width, height);
     }
 
-- 
1.8.3.1

  reply	other threads:[~2014-05-26  8:39 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-26  8:39 [Qemu-devel] [PULL 0/5] console: text terminal updates Gerd Hoffmann
2014-05-26  8:39 ` Gerd Hoffmann [this message]
2014-05-26  8:39 ` [Qemu-devel] [PULL 2/5] console: nicer initial screen Gerd Hoffmann
2014-05-26  8:39 ` [Qemu-devel] [PULL 3/5] console: update text terminal surface unconditionally Gerd Hoffmann
2014-05-26  8:39 ` [Qemu-devel] [PULL 4/5] console: rework text terminal cursor logic Gerd Hoffmann
2014-05-26  8:39 ` [Qemu-devel] [PULL 5/5] console: add kbd_put_keysym_console Gerd Hoffmann

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1401093570-4161-2-git-send-email-kraxel@redhat.com \
    --to=kraxel@redhat.com \
    --cc=aliguori@amazon.com \
    --cc=batuzovk@ispras.ru \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).