<div dir="ltr"><div dir="ltr"><div dir="ltr">We first produce this bug in rhel7.4's libvir daemon。For easily produce the bug, the step can be as follows:<div>1. add sleep(3)  in daemonStreamFilter() pre virMutexLock(&stream->priv->lock), then build libvirtd bin executable, then restart libvirtd</div><div>2. use virsh console open one vm's console, for this console（the vm's kernel need console=ttyS0 boot parameter,then just  input from keyboard on and on</div><div>3. use virsh console --force to break the previous console session,  than you will get libvirt daemon deadlock。</div><div><br></div><div>And for the problem client->privData to be released problem, only virNetServerClientClose() will free client->privData and client, I think this can be fixed by remove virNetServerClientClose() from daemonStreamEvent(),</div><div>and replace with virNetServerClientImmediateClose(), so virNetServerProcessClients() will test the session would be closed。</div><div><br></div><div><br></div></div></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">Michal Privoznik <<a href="mailto:mprivozn@redhat.com">mprivozn@redhat.com</a>> 于2019年11月25日周一 下午5:38写道：<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 11/25/19 8:34 AM, LanceLiu wrote:<br>
> ---<br>
>   src/libvirt_remote.syms           |  1 +<br>
>   src/remote/remote_daemon_stream.c | 10 +++++++++-<br>
>   src/rpc/virnetserverclient.c      | 12 ++++++++++++<br>
>   src/rpc/virnetserverclient.h      |  2 ++<br>
>   4 files changed, 24 insertions(+), 1 deletion(-)<br>
<br>
Please format commit messages following title + message format (look at <br>
git log how other messages are formatted).<br>
<br>
> <br>
> diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms<br>
> index 0493467..c32e234 100644<br>
> --- a/src/libvirt_remote.syms<br>
> +++ b/src/libvirt_remote.syms<br>
> @@ -173,6 +173,7 @@ virNetServerClientPreExecRestart;<br>
>   virNetServerClientRemoteAddrStringSASL;<br>
>   virNetServerClientRemoteAddrStringURI;<br>
>   virNetServerClientRemoveFilter;<br>
> +virNetServerClientCheckFilterExist;<br>
>   virNetServerClientSendMessage;<br>
>   virNetServerClientSetAuthLocked;<br>
>   virNetServerClientSetAuthPendingLocked;<br>
> diff --git a/src/remote/remote_daemon_stream.c b/src/remote/remote_daemon_stream.c<br>
> index 82cadb6..de0dca3 100644<br>
> --- a/src/remote/remote_daemon_stream.c<br>
> +++ b/src/remote/remote_daemon_stream.c<br>
> @@ -292,10 +292,18 @@ daemonStreamFilter(virNetServerClientPtr client,<br>
>   {<br>
>       daemonClientStream *stream = opaque;<br>
>       int ret = 0;<br>
> +    daemonClientPrivatePtr priv = NULL;<br>
> +    int filter_id = stream->filterID;<br>
>   <br>
>       virObjectUnlock(client);<br>
> +    priv = virNetServerClientGetPrivateData(client);<br>
<br>
This is not needed.<br>
<br>
>       virMutexLock(&stream->priv->lock);<br>
>       virObjectLock(client);<br>
> +    if (!virNetServerClientCheckFilterExist(client, filter_id)) {<br>
> +        VIR_WARN("this daemon stream filter: %d have been deleted!", filter_id);<br>
> +        ret = -1;<br>
> +        goto cleanup;<br>
> +    }<br>
>   <br>
>       if (msg->header.type != VIR_NET_STREAM &&<br>
>           msg->header.type != VIR_NET_STREAM_HOLE)<br>
> @@ -317,7 +325,7 @@ daemonStreamFilter(virNetServerClientPtr client,<br>
>       ret = 1;<br>
>   <br>
>    cleanup:<br>
> -    virMutexUnlock(&stream->priv->lock);<br>
> +    virMutexUnlock(&priv->lock);<br>
<br>
This is not needed: stream->priv and priv are the same structure.<br>
<br>
>       return ret;<br>
>   }<br>
>   <br>
<br>
Anyway, this still doesn't work. Problem is, that if a stream is <br>
removed, the private data might be removed too and hence <br>
virMutexLock(&stream->priv->lock) will do something undefined (besides <br>
accessing freed memory). In my testing the daemon deadlocks because it's <br>
trying to lock stream-priv->lock which is locked.<br>
<br>
As I said in the other thread - we need to re-evaluate the first commit. <br>
Do you have a reproducer for the original problem please?<br>
<br>
Michal<br>
<br>
</blockquote></div>