[libvirt] [PATCH] remote_daemon_stream: Hold an extra reference to stream in daemonStreamFilter

[Ján Tomko]

On Mon, Nov 25, 2019 at 05:49:35PM +0100, Michal Privoznik wrote:
>In v5.9.0-273-g8ecab214de I've tried to fix a lock ordering
>problem, but introduced a crasher. Problem is that because the
>client lock is unlocked (in order to honour lock ordering) the
>stream we are currently checking in daemonStreamFilter() might be
>freed and thus stream->priv might not even exist when the control
>get to virMutexLock() call.
>
>To resolve this, grab an extra reference to the stream and handle
>its cleanup should the refcounter reach zero after the deref.
>If that's the case and we are the only ones holding a reference
>to the stream, we MUST return a positive value to make
>virNetServerClientDispatchRead() break its loop where it iterates
>over filters. The problem is, if we did not do so, then
>"filter = filter->next" line will read from a memory that was
>just freed (freeing a stream also unregisters its filter).
>
>Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
>---
>
>Reproducing this issue is very easy:
>
>1) put sleep(5) right after virObjectUnlock(client) in the fist hunk,
>2) virsh console --force $dom   and type something so that the stream
>   has some data to process
>3) while 2) is still running, run the same command from another terminal
>4) observe libvirtd crash
>
> src/remote/remote_daemon_stream.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>

Reviewed-by: Ján Tomko <jtomko at redhat.com>

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20191129/9c734392/attachment-0001.sig>