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@xxxxxxxxxx> --- 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@xxxxxxxxxx> Jano
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
