diff mbox series

[4/5] chardev/char.c: Always pass id to chardev_new

Message ID a378d9426980c97a5cbbf2e72815f366c158ef71.1616368879.git.lukasstraub2@web.de
State New
Headers show
Series yank: Add chardev tests and fixes | expand

Commit Message

Lukas Straub March 21, 2021, 11:31 p.m. UTC
Always pass the id to chardev_new, since it is needed to register
the yank instance for the chardev. Also, after checking that
nothing calls chardev_new with id=NULL, assert() that id!=NULL.

This fixes a crash when using chardev-change to change a chardev
to chardev-socket, which attempts to register a yank instance.
This in turn tries to dereference the NULL-pointer.

Signed-off-by: Lukas Straub <lukasstraub2@web.de>
---
 chardev/char.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

--
2.30.2

Comments

Marc-André Lureau March 22, 2021, 8:33 a.m. UTC | #1
On Mon, Mar 22, 2021 at 3:31 AM Lukas Straub <lukasstraub2@web.de> wrote:

> Always pass the id to chardev_new, since it is needed to register
> the yank instance for the chardev. Also, after checking that
> nothing calls chardev_new with id=NULL, assert() that id!=NULL.
>
> This fixes a crash when using chardev-change to change a chardev
> to chardev-socket, which attempts to register a yank instance.
> This in turn tries to dereference the NULL-pointer.
>
> Signed-off-by: Lukas Straub <lukasstraub2@web.de>
>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

---
>  chardev/char.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/chardev/char.c b/chardev/char.c
> index 1584865027..ad416c0714 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -964,6 +964,7 @@ static Chardev *chardev_new(const char *id, const char
> *typename,
>      bool be_opened = true;
>
>      assert(g_str_has_prefix(typename, "chardev-"));
> +    assert(id);
>
>      obj = object_new(typename);
>      chr = CHARDEV(obj);
> @@ -1092,12 +1093,11 @@ ChardevReturn *qmp_chardev_change(const char *id,
> ChardevBackend *backend,
>          return NULL;
>      }
>
> -    chr_new = chardev_new(NULL, object_class_get_name(OBJECT_CLASS(cc)),
> +    chr_new = chardev_new(id, 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);
> --
> 2.30.2
>
>
Li Zhang March 22, 2021, 10:32 a.m. UTC | #2
Looks good to me. I have tested this series, it works well.
Tested-by: Li Zhang <li.zhang@cloud.ionos.com>

Thanks
Li

On Mon, Mar 22, 2021 at 12:35 AM Lukas Straub <lukasstraub2@web.de> wrote:

> Always pass the id to chardev_new, since it is needed to register
> the yank instance for the chardev. Also, after checking that
> nothing calls chardev_new with id=NULL, assert() that id!=NULL.
>
> This fixes a crash when using chardev-change to change a chardev
> to chardev-socket, which attempts to register a yank instance.
> This in turn tries to dereference the NULL-pointer.
>
> Signed-off-by: Lukas Straub <lukasstraub2@web.de>
> ---
>  chardev/char.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/chardev/char.c b/chardev/char.c
> index 1584865027..ad416c0714 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -964,6 +964,7 @@ static Chardev *chardev_new(const char *id, const char
> *typename,
>      bool be_opened = true;
>
>      assert(g_str_has_prefix(typename, "chardev-"));
> +    assert(id);
>
>      obj = object_new(typename);
>      chr = CHARDEV(obj);
> @@ -1092,12 +1093,11 @@ ChardevReturn *qmp_chardev_change(const char *id,
> ChardevBackend *backend,
>          return NULL;
>      }
>
> -    chr_new = chardev_new(NULL, object_class_get_name(OBJECT_CLASS(cc)),
> +    chr_new = chardev_new(id, 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);
> --
> 2.30.2
>
>
diff mbox series

Patch

diff --git a/chardev/char.c b/chardev/char.c
index 1584865027..ad416c0714 100644
--- a/chardev/char.c
+++ b/chardev/char.c
@@ -964,6 +964,7 @@  static Chardev *chardev_new(const char *id, const char *typename,
     bool be_opened = true;

     assert(g_str_has_prefix(typename, "chardev-"));
+    assert(id);

     obj = object_new(typename);
     chr = CHARDEV(obj);
@@ -1092,12 +1093,11 @@  ChardevReturn *qmp_chardev_change(const char *id, ChardevBackend *backend,
         return NULL;
     }

-    chr_new = chardev_new(NULL, object_class_get_name(OBJECT_CLASS(cc)),
+    chr_new = chardev_new(id, 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);