[Qemu-devel] [STABLE][PATCH] char: Fix closing of various char devices

[Qemu-devel] [STABLE][PATCH] char: Fix closing of various char devices

[Jan Kiszka]

This patch fixes several issues around closing char devices. Affected
were pty (timer was left behind, even running), udp (no close handling
at all) and tcp (missing async IO handler cleanup). The bugs either
caused segfaults or stalled the qemu process. So far, hot-unplugging USB
serial adapters suffered from this.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 qemu-char.c |   21 +++++++++++++++++++--
 1 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/qemu-char.c b/qemu-char.c
index c92507b..7a852b7 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -929,6 +929,8 @@ static void pty_chr_close(struct CharDriverState *chr)
 
     qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
     close(s->fd);
+    qemu_del_timer(s->timer);
+    qemu_free_timer(s->timer);
     qemu_free(s);
 }
 
@@ -1758,6 +1760,16 @@ static void udp_chr_update_read_handler(CharDriverState *chr)
     }
 }
 
+static void udp_chr_close(CharDriverState *chr)
+{
+    NetCharDriver *s = chr->opaque;
+    if (s->fd >= 0) {
+        qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
+        closesocket(s->fd);
+    }
+    qemu_free(s);
+}
+
 static CharDriverState *qemu_chr_open_udp(const char *def)
 {
     CharDriverState *chr = NULL;
@@ -1791,6 +1803,7 @@ static CharDriverState *qemu_chr_open_udp(const char *def)
     chr->opaque = s;
     chr->chr_write = udp_chr_write;
     chr->chr_update_read_handler = udp_chr_update_read_handler;
+    chr->chr_close = udp_chr_close;
     return chr;
 
 return_err:
@@ -1993,10 +2006,14 @@ static void tcp_chr_accept(void *opaque)
 static void tcp_chr_close(CharDriverState *chr)
 {
     TCPCharDriver *s = chr->opaque;
-    if (s->fd >= 0)
+    if (s->fd >= 0) {
+        qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
         closesocket(s->fd);
-    if (s->listen_fd >= 0)
+    }
+    if (s->listen_fd >= 0) {
+        qemu_set_fd_handler(s->listen_fd, NULL, NULL, NULL);
         closesocket(s->listen_fd);
+    }
     qemu_free(s);
 }
 

Re: [Qemu-devel] [STABLE][PATCH] char: Fix closing of various char devices

[Anthony Liguori]

Jan Kiszka wrote:
> This patch fixes several issues around closing char devices. Affected
> were pty (timer was left behind, even running), udp (no close handling
> at all) and tcp (missing async IO handler cleanup). The bugs either
> caused segfaults or stalled the qemu process. So far, hot-unplugging USB
> serial adapters suffered from this.
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>   

Applied to trunk and stable.  Thanks.

Regards,

Anthony Liguori

> ---
>
>  qemu-char.c |   21 +++++++++++++++++++--
>  1 files changed, 19 insertions(+), 2 deletions(-)
>
> diff --git a/qemu-char.c b/qemu-char.c
> index c92507b..7a852b7 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -929,6 +929,8 @@ static void pty_chr_close(struct CharDriverState *chr)
>  
>      qemu_set_fd_handler2(s->fd, NULL, NULL, NULL, NULL);
>      close(s->fd);
> +    qemu_del_timer(s->timer);
> +    qemu_free_timer(s->timer);
>      qemu_free(s);
>  }
>  
> @@ -1758,6 +1760,16 @@ static void udp_chr_update_read_handler(CharDriverState *chr)
>      }
>  }
>  
> +static void udp_chr_close(CharDriverState *chr)
> +{
> +    NetCharDriver *s = chr->opaque;
> +    if (s->fd >= 0) {
> +        qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
> +        closesocket(s->fd);
> +    }
> +    qemu_free(s);
> +}
> +
>  static CharDriverState *qemu_chr_open_udp(const char *def)
>  {
>      CharDriverState *chr = NULL;
> @@ -1791,6 +1803,7 @@ static CharDriverState *qemu_chr_open_udp(const char *def)
>      chr->opaque = s;
>      chr->chr_write = udp_chr_write;
>      chr->chr_update_read_handler = udp_chr_update_read_handler;
> +    chr->chr_close = udp_chr_close;
>      return chr;
>  
>  return_err:
> @@ -1993,10 +2006,14 @@ static void tcp_chr_accept(void *opaque)
>  static void tcp_chr_close(CharDriverState *chr)
>  {
>      TCPCharDriver *s = chr->opaque;
> -    if (s->fd >= 0)
> +    if (s->fd >= 0) {
> +        qemu_set_fd_handler(s->fd, NULL, NULL, NULL);
>          closesocket(s->fd);
> -    if (s->listen_fd >= 0)
> +    }
> +    if (s->listen_fd >= 0) {
> +        qemu_set_fd_handler(s->listen_fd, NULL, NULL, NULL);
>          closesocket(s->listen_fd);
> +    }
>      qemu_free(s);
>  }
>  
>
>
>
>