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(a)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(a)redhat.com>
Jano