Re: [Qemu-devel] [PULL 05/10] ui: refactor VncDisplay to allow multiple listening sockets

[Peter Maydell]

On 9 February 2017 at 13:01, Gerd Hoffmann <address@hidden> wrote:
> From: "Daniel P. Berrange" <address@hidden>
>
> Currently there is only a single listener for plain VNC and
> a single listener for websockets VNC. This means that if
> getaddrinfo() returns multiple IP addresses, for a hostname,
> the VNC server can only listen on one of them. This is
> just bearable if listening on wildcard interface, or if
> the host only has a single network interface to listen on,
> but if there are multiple NICs and the VNC server needs
> to listen on 2 or more specific IP addresses, it can't be
> done.
>
> This refactors the VncDisplay state so that it holds an
> array of listening sockets, but still only listens on
> one socket.
>
> Reviewed-by: Eric Blake <address@hidden>
> Signed-off-by: Daniel P. Berrange <address@hidden>
> Message-id: address@hidden
> Signed-off-by: Gerd Hoffmann <address@hidden>

> @@ -3153,24 +3166,33 @@ void vnc_display_init(const char *id)
>
>  static void vnc_display_close(VncDisplay *vd)
>  {
> +    size_t i;
>      if (!vd) {
>          return;
>      }
>      vd->is_unix = false;
> -    if (vd->lsock != NULL) {
> -        if (vd->lsock_tag) {
> -            g_source_remove(vd->lsock_tag);
> +    for (i = 0; i < vd->nlsock; i++) {
> +        if (vd->lsock_tag[i]) {
> +            g_source_remove(vd->lsock_tag[i]);
>          }
> -        object_unref(OBJECT(vd->lsock));
> -        vd->lsock = NULL;
> +        object_unref(OBJECT(vd->lsock[i]));
>      }
> -    if (vd->lwebsock != NULL) {
> -        if (vd->lwebsock_tag) {
> -            g_source_remove(vd->lwebsock_tag);
> +    g_free(vd->lsock);
> +    g_free(vd->lsock_tag);
> +    vd->lsock = NULL;
> +    vd->nlsock = 0;

Coverity points out that this results in a double-free,
because vnc_display_open() has code paths which result in
calling vnc_display_close() twice on the same VncDisplay*,
and this code frees vd->lsock_tag without then setting it
to NULL.

Similarly for vd->lwebsock_tag and vd->led.

(Coverity issues CID 1371242, 1371243, 1371244.)

thanks
-- PMM