qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/2] Fix the segment fault when calling yank_register_instanc


From: Li Zhang
Subject: Re: [PATCH 1/2] Fix the segment fault when calling yank_register_instance
Date: Tue, 16 Mar 2021 10:45:45 +0100

Hi Marc-André, 

The new chardev can get the same label. It is assigned after the function  
 
ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend,
                                  Error **errp)
{
     .....
     chr_new = chardev_new(NULL, object_class_get_name(OBJECT_CLASS(cc)),
                          backend, chr->gcontext, errp);
    if (!chr_new) {
        return NULL;
    }
    chr_new->label = g_strdup(id);
    if (chr->be_open && !chr_new->be_open) {
        qemu_chr_be_event(chr, CHR_EVENT_CLOSED);
        closed_sent = true;
    }

    chr->be = NULL;
    qemu_chr_fe_init(be, chr_new, &error_abort);
       .....
}

It passes parameter NULL in chardev_new, I think it may be because the old chardev isn't released yet. 
It will cause duplicated problems. I need to consider this logic to see if it can be changed. 

Thanks
Li


On Mon, Mar 15, 2021 at 7:51 PM Marc-André Lureau <marcandre.lureau@gmail.com> wrote:
Hi

On Mon, Mar 15, 2021 at 9:22 PM Li Zhang <zhlcindy@gmail.com> wrote:
From: Li Zhang <li.zhang@cloud.ionos.com>

When executing the QMP commands "chardev-change" to change the
backend device to socket, it will cause a segment fault because
it assumes chr->label as non-NULL in function yank_register_instance.
The function qmp_chardev_change calls chardev_new, which label
is NULL when creating a new chardev. The label will be passed to
yank_register_instance which causes a segment fault. The callchain
is as the following:
        chardev_new ->
            qemu_char_open ->
                cc->open ->
                qmp_chardev_open_socket ->
                    yank_register_instance

Signed-off-by: Li Zhang <li.zhang@cloud.ionos.com>
---
 chardev/char-socket.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index c8bced76b7..26d5172682 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1421,10 +1421,12 @@ static void qmp_chardev_open_socket(Chardev *chr,
         qemu_chr_set_feature(chr, QEMU_CHAR_FEATURE_FD_PASS);
     }

-    if (!yank_register_instance(CHARDEV_YANK_INSTANCE(chr->label), errp)) {
-        return;
+    if (chr->label) {
+        if (!yank_register_instance(CHARDEV_YANK_INSTANCE(chr->label), errp)) {
+            return;
+        }
+        s->registered_yank = true;
     }
-    s->registered_yank = true;

     /* be isn't opened until we get a connection */
     *be_opened = false

Looks wrong to me, the new chardev will get the same label, and it should still be possible to call the yank functions then. The registration logic needs to be reworked during chardev-change.

--
Marc-André Lureau

reply via email to

[Prev in Thread] Current Thread [Next in Thread]