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

["Marc-André Lureau" <marcandre.lureau@gmail.com>]

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

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

[-- Attachment #2: Type: text/html, Size: 2596 bytes --]