From nobody Wed Apr 24 03:20:37 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 205.139.110.61 as permitted sender) client-ip=205.139.110.61; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-1.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 205.139.110.61 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1574700605; cv=none; d=zohomail.com; s=zohoarc; b=OmxTlTLOKo4PNU43zCyqCQt5LhA76YwhPX1ENFTuinsjTLjRiQJECSThUoccg9QuWHFh8jyGlu2CdwLDIWWhVkIxIb/JgExYz3KX+Qq1UI5SuzkjqCOOwW8rtlIlhvT6THp6TIbcTA3te6P7LQ5ofey9CV4+JJ0MpaEJ4Fko49M= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1574700605; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To; bh=x6G7fakVX0WMaigRZYcKu2ka6Ft9Lc6UyQsqWKZUzZ8=; b=PwEQcXV1pVLJMOeTQnhaQXGoQ79DiHHl2k1DrhmjPNBQbQVna0E4yDArUaItFtIjGAwSXOKqexzpbu0ZecXJ/BKCl2oqbrbezJniNYsMPUGiKIceQlPByOWtuDXx7XAMIRyQtQU1HnzqGy53ypxeAqHCE2fVOzgBzkUiR3BgqVg= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 205.139.110.61 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from us-smtp-delivery-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.61]) by mx.zohomail.com with SMTPS id 1574700605757378.3492872203785; Mon, 25 Nov 2019 08:50:05 -0800 (PST) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-338-Ak-5xfhIP5yAlBKp4mH8bA-1; Mon, 25 Nov 2019 11:50:00 -0500 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id CA573477; Mon, 25 Nov 2019 16:49:54 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.20]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 6ECEF5D9CA; Mon, 25 Nov 2019 16:49:51 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 2294218095FF; Mon, 25 Nov 2019 16:49:46 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id xAPGnjZZ025882 for ; Mon, 25 Nov 2019 11:49:45 -0500 Received: by smtp.corp.redhat.com (Postfix) id 53AFC10016EB; Mon, 25 Nov 2019 16:49:45 +0000 (UTC) Received: from moe.redhat.com (unknown [10.43.2.30]) by smtp.corp.redhat.com (Postfix) with ESMTP id A33ED10016E8; Mon, 25 Nov 2019 16:49:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1574700602; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=x6G7fakVX0WMaigRZYcKu2ka6Ft9Lc6UyQsqWKZUzZ8=; b=LygZ6uu3O1psrM0qa4ezMNcBut3buC8xRVQfIPVs8pSj3SIsWvgZTHHNgGCJkQ9XG9X3s1 3zRFZMjYa3w6y6tbdc63sqPhbjJs5/yYFiq+vlqJrCMSvoSPu/XY0eWVe4rNRnEhI8wR/a Lenjf8cKl5hZkBdzqXPeFEJNDn28chs= From: Michal Privoznik To: libvir-list@redhat.com Date: Mon, 25 Nov 2019 17:49:35 +0100 Message-Id: <90d7646ff3db9f479408f4e14e8259954e76d475.1574700385.git.mprivozn@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-loop: libvir-list@redhat.com Cc: liu.lance.89@gmail.com Subject: [libvirt] [PATCH] remote_daemon_stream: Hold an extra reference to stream in daemonStreamFilter X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-MC-Unique: Ak-5xfhIP5yAlBKp4mH8bA-1 X-Mimecast-Spam-Score: 0 Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) Content-Type: text/plain; charset="utf-8" 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 =3D filter->next" line will read from a memory that was just freed (freeing a stream also unregisters its filter). Signed-off-by: Michal Privoznik Reviewed-by: J=C3=A1n Tomko --- 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(+) diff --git a/src/remote/remote_daemon_stream.c b/src/remote/remote_daemon_s= tream.c index 82cadb67ac..73e4d7befb 100644 --- a/src/remote/remote_daemon_stream.c +++ b/src/remote/remote_daemon_stream.c @@ -293,10 +293,25 @@ daemonStreamFilter(virNetServerClientPtr client, daemonClientStream *stream =3D opaque; int ret =3D 0; =20 + /* We must honour lock ordering here. Client private data lock must + * be acquired before client lock. Bu we are already called with + * client locked. To avoid stream disappearing while we unlock + * everything, let's increase its refcounter. This has some + * implications though. */ + stream->refs++; virObjectUnlock(client); virMutexLock(&stream->priv->lock); virObjectLock(client); =20 + if (stream->refs =3D=3D 1) { + /* So we are the only ones holding the reference to the stream. + * Return 1 to signal to the caller that we've processed the + * message. And to "process" means free. */ + virNetMessageFree(msg); + ret =3D 1; + goto cleanup; + } + if (msg->header.type !=3D VIR_NET_STREAM && msg->header.type !=3D VIR_NET_STREAM_HOLE) goto cleanup; @@ -318,6 +333,10 @@ daemonStreamFilter(virNetServerClientPtr client, =20 cleanup: virMutexUnlock(&stream->priv->lock); + /* Don't pass client here, because client is locked here and this + * function might try to lock it again which would result in a + * deadlock. */ + daemonFreeClientStream(NULL, stream); return ret; } =20 --=20 2.23.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list