[libvirt] [PATCH] add lock priv->lock protection in remoteClientCloseFunc()

[Lance Liu]

Hi:
    Need any more informations about my patch?

LanceLiu <liu.lance.89 at gmail.com> 于2019年12月2日周一 上午11:16写道：

> sometimes, virsh console session may closed by virsh console existed
> session(when virsh console client
>  exit) and new virsh console --force session simutaneously. So in one
> thread(Job worker), it calls
> daemonStreamEvent() and referencing stream, and in another thread(main
> thread), virNetServerClientClose()
> was called, and the callback remoteClientCloseFunc() was called, without
> protect by priv->lock in
> remoteClientCloseFunc(), it may lead to daemonStreamEvent reference stream
> released by remoteClientCloseFunc(),
> and also need change virNetServerClientClose() to be
> virNetServerClientImmediateClose() in daemonStreamEvent(),
> or it lead to libvirt daemon deadlock by double lock priv->lock in
> daemonStreamEvent()
> ---
>  src/remote/remote_daemon_dispatch.c |  2 ++
>  src/remote/remote_daemon_stream.c   | 14 +++++++-------
>  2 files changed, 9 insertions(+), 7 deletions(-)
>
> diff --git a/src/remote/remote_daemon_dispatch.c
> b/src/remote/remote_daemon_dispatch.c
> index f369ffb..b0982b7 100644
> --- a/src/remote/remote_daemon_dispatch.c
> +++ b/src/remote/remote_daemon_dispatch.c
> @@ -1724,7 +1724,9 @@ static void
> remoteClientCloseFunc(virNetServerClientPtr client)
>  {
>      struct daemonClientPrivate *priv =
> virNetServerClientGetPrivateData(client);
>
> +    virMutexLock(&priv->lock);
>      daemonRemoveAllClientStreams(priv->streams);
> +    virMutexUnlock(&priv->lock);
>
>      remoteClientFreePrivateCallbacks(priv);
>  }
> diff --git a/src/remote/remote_daemon_stream.c
> b/src/remote/remote_daemon_stream.c
> index 73e4d7b..6a84fdf 100644
> --- a/src/remote/remote_daemon_stream.c
> +++ b/src/remote/remote_daemon_stream.c
> @@ -141,7 +141,7 @@ daemonStreamEvent(virStreamPtr st, int events, void
> *opaque)
>          (events & VIR_STREAM_EVENT_WRITABLE)) {
>          if (daemonStreamHandleWrite(client, stream) < 0) {
>              daemonRemoveClientStream(client, stream);
> -            virNetServerClientClose(client);
> +            virNetServerClientImmediateClose(client);
>              goto cleanup;
>          }
>      }
> @@ -151,7 +151,7 @@ daemonStreamEvent(virStreamPtr st, int events, void
> *opaque)
>          events = events & ~(VIR_STREAM_EVENT_READABLE);
>          if (daemonStreamHandleRead(client, stream) < 0) {
>              daemonRemoveClientStream(client, stream);
> -            virNetServerClientClose(client);
> +            virNetServerClientImmediateClose(client);
>              goto cleanup;
>          }
>          /* If we detected EOF during read processing,
> @@ -176,7 +176,7 @@ daemonStreamEvent(virStreamPtr st, int events, void
> *opaque)
>              if (daemonStreamHandleFinish(client, stream, msg) < 0) {
>                  virNetMessageFree(msg);
>                  daemonRemoveClientStream(client, stream);
> -                virNetServerClientClose(client);
> +                virNetServerClientImmediateClose(client);
>                  goto cleanup;
>              }
>              break;
> @@ -186,7 +186,7 @@ daemonStreamEvent(virStreamPtr st, int events, void
> *opaque)
>              if (daemonStreamHandleAbort(client, stream, msg) < 0) {
>                  virNetMessageFree(msg);
>                  daemonRemoveClientStream(client, stream);
> -                virNetServerClientClose(client);
> +                virNetServerClientImmediateClose(client);
>                  goto cleanup;
>              }
>              break;
> @@ -205,7 +205,7 @@ daemonStreamEvent(virStreamPtr st, int events, void
> *opaque)
>          stream->recvEOF = true;
>          if (!(msg = virNetMessageNew(false))) {
>              daemonRemoveClientStream(client, stream);
> -            virNetServerClientClose(client);
> +            virNetServerClientImmediateClose(client);
>              goto cleanup;
>          }
>          msg->cb = daemonStreamMessageFinished;
> @@ -219,7 +219,7 @@ daemonStreamEvent(virStreamPtr st, int events, void
> *opaque)
>                                                "", 0) < 0) {
>              virNetMessageFree(msg);
>              daemonRemoveClientStream(client, stream);
> -            virNetServerClientClose(client);
> +            virNetServerClientImmediateClose(client);
>              goto cleanup;
>          }
>      }
> @@ -262,7 +262,7 @@ daemonStreamEvent(virStreamPtr st, int events, void
> *opaque)
>          }
>          daemonRemoveClientStream(client, stream);
>          if (ret < 0)
> -            virNetServerClientClose(client);
> +               virNetServerClientImmediateClose(client);
>          goto cleanup;
>      }
>
> --
> 1.8.3.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20191205/93dc71e3/attachment-0001.htm>