[libvirt] [PATCH] Fix bug libvirt daemon segfault when new force console vm session break down existed console session. When force console vm command arrived, libvirtd will break down existed console session, and in this procedure, it will call daemonStreamEvent() to release resources , so daemonStreamFilter() need to check if stream filter existed when get client object lock and client->privData's lock, if not existed, just return -1
Michal Privoznik
mprivozn at redhat.com
Mon Nov 25 09:38:08 UTC 2019
- Previous message (by thread): [libvirt] [PATCH] Fix bug libvirt daemon segfault when new force console vm session break down existed console session. When force console vm command arrived, libvirtd will break down existed console session, and in this procedure, it will call daemonStreamEvent() to release resources , so daemonStreamFilter() need to check if stream filter existed when get client object lock and client->privData's lock, if not existed, just return -1
- Next message (by thread): [libvirt] [PATCH] Fix bug libvirt daemon segfault when new force console vm session break down existed console session. When force console vm command arrived, libvirtd will break down existed console session, and in this procedure, it will call daemonStreamEvent() to release resources , so daemonStreamFilter() need to check if stream filter existed when get client object lock and client->privData's lock, if not existed, just return -1
- Messages sorted by:
[ date ]
[ thread ]
[ subject ]
[ author ]
On 11/25/19 8:34 AM, LanceLiu wrote:
> ---
> src/libvirt_remote.syms | 1 +
> src/remote/remote_daemon_stream.c | 10 +++++++++-
> src/rpc/virnetserverclient.c | 12 ++++++++++++
> src/rpc/virnetserverclient.h | 2 ++
> 4 files changed, 24 insertions(+), 1 deletion(-)
Please format commit messages following title + message format (look at
git log how other messages are formatted).
>
> diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms
> index 0493467..c32e234 100644
> --- a/src/libvirt_remote.syms
> +++ b/src/libvirt_remote.syms
> @@ -173,6 +173,7 @@ virNetServerClientPreExecRestart;
> virNetServerClientRemoteAddrStringSASL;
> virNetServerClientRemoteAddrStringURI;
> virNetServerClientRemoveFilter;
> +virNetServerClientCheckFilterExist;
> virNetServerClientSendMessage;
> virNetServerClientSetAuthLocked;
> virNetServerClientSetAuthPendingLocked;
> diff --git a/src/remote/remote_daemon_stream.c b/src/remote/remote_daemon_stream.c
> index 82cadb6..de0dca3 100644
> --- a/src/remote/remote_daemon_stream.c
> +++ b/src/remote/remote_daemon_stream.c
> @@ -292,10 +292,18 @@ daemonStreamFilter(virNetServerClientPtr client,
> {
> daemonClientStream *stream = opaque;
> int ret = 0;
> + daemonClientPrivatePtr priv = NULL;
> + int filter_id = stream->filterID;
>
> virObjectUnlock(client);
> + priv = virNetServerClientGetPrivateData(client);
This is not needed.
> virMutexLock(&stream->priv->lock);
> virObjectLock(client);
> + if (!virNetServerClientCheckFilterExist(client, filter_id)) {
> + VIR_WARN("this daemon stream filter: %d have been deleted!", filter_id);
> + ret = -1;
> + goto cleanup;
> + }
>
> if (msg->header.type != VIR_NET_STREAM &&
> msg->header.type != VIR_NET_STREAM_HOLE)
> @@ -317,7 +325,7 @@ daemonStreamFilter(virNetServerClientPtr client,
> ret = 1;
>
> cleanup:
> - virMutexUnlock(&stream->priv->lock);
> + virMutexUnlock(&priv->lock);
This is not needed: stream->priv and priv are the same structure.
> return ret;
> }
>
Anyway, this still doesn't work. Problem is, that if a stream is
removed, the private data might be removed too and hence
virMutexLock(&stream->priv->lock) will do something undefined (besides
accessing freed memory). In my testing the daemon deadlocks because it's
trying to lock stream-priv->lock which is locked.
As I said in the other thread - we need to re-evaluate the first commit.
Do you have a reproducer for the original problem please?
Michal
- Previous message (by thread): [libvirt] [PATCH] Fix bug libvirt daemon segfault when new force console vm session break down existed console session. When force console vm command arrived, libvirtd will break down existed console session, and in this procedure, it will call daemonStreamEvent() to release resources , so daemonStreamFilter() need to check if stream filter existed when get client object lock and client->privData's lock, if not existed, just return -1
- Next message (by thread): [libvirt] [PATCH] Fix bug libvirt daemon segfault when new force console vm session break down existed console session. When force console vm command arrived, libvirtd will break down existed console session, and in this procedure, it will call daemonStreamEvent() to release resources , so daemonStreamFilter() need to check if stream filter existed when get client object lock and client->privData's lock, if not existed, just return -1
- Messages sorted by:
[ date ]
[ thread ]
[ subject ]
[ author ]
More information about the libvir-list
mailing list