From nobody Mon Feb 9 17:34:39 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 205.139.110.120 as permitted sender) client-ip=205.139.110.120; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-1.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 205.139.110.120 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=1575562213; cv=none; d=zohomail.com; s=zohoarc; b=elj5H/IIWFFIC1/JPhEPQRQN9gETOC78T5bty9egK3DYBWeBjbu2mgp4J4e6KXZf0Gl+cF7Qu7h/9WieZXdOw266+IEYz/6WmvuPldvludbQILJcOrVW5TRMnpyfw4shi73zFIS5xBCGcsrafobflqq0Bc5CS4wSCkTtdZ4q6zc= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1575562213; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=SF9VqGT5GaINpjtvqFgIj2AGJm5AtsTGD5e7jju/ipI=; b=lzghrkagPqHq0P0z6DWG+OeOm7LKGuluCzb3CzDdp8WSDSKkIPmDes1xAFiaLNz25E594oDv7Rrb2S7BoUGN+vGMk+GT/pRWCMh3mcgGLriQuwLCWZd06a8h8Jz/l45fjXMs89Zm0RsMEJiIIkPZLgSA2dIQa6pcTBH6RxTz0C4= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 205.139.110.120 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-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by mx.zohomail.com with SMTPS id 1575562213516441.6852048421914; Thu, 5 Dec 2019 08:10:13 -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-363-p8O17BvkO2KuZ9grzv5SYA-1; Thu, 05 Dec 2019 11:09:26 -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 418468E692C; Thu, 5 Dec 2019 16:09:16 +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 147595DA60; Thu, 5 Dec 2019 16:09:16 +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 C3C37180880C; Thu, 5 Dec 2019 16:09:15 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id xB5G9576024100 for ; Thu, 5 Dec 2019 11:09:05 -0500 Received: by smtp.corp.redhat.com (Postfix) id D065D600D1; Thu, 5 Dec 2019 16:09:05 +0000 (UTC) Received: from himantopus.redhat.com (ovpn-116-111.phx2.redhat.com [10.3.116.111]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 9D90760132 for ; Thu, 5 Dec 2019 16:09:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1575562212; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=SF9VqGT5GaINpjtvqFgIj2AGJm5AtsTGD5e7jju/ipI=; b=GuDt61CVrKueH3QYieSGnfdO6NV93u4gl3a9gLzEMfoqj8JY5dJ8FszVDxJFhW+sb89llg aFm0WAvYDTlW3DQAtvN+0YyhjHao+T0VOIdAi5kRhvGpqJ8KlNEpar0964K1xc2UXCrPlw xJ5sNTgb5M1QlDouO9NsPgqGImHvgzM= From: Jonathon Jongsma To: libvir-list@redhat.com Date: Thu, 5 Dec 2019 10:08:57 -0600 Message-Id: <20191205160857.30182-9-jjongsma@redhat.com> In-Reply-To: <20191205160857.30182-1-jjongsma@redhat.com> References: <20191205160857.30182-1-jjongsma@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-loop: libvir-list@redhat.com Subject: [libvirt] [PATCH 8/8] qemu: remove qemuDomainObjBegin/EndJobWithAgent() 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: p8O17BvkO2KuZ9grzv5SYA-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" This function potentially grabs both a monitor job and an agent job at the same time. This is problematic because it means that a malicious (or just buggy) guest agent can cause a denial of service on the host. The presence of this function makes it easy to do the wrong thing and hold both jobs at the same time. All existing uses have already been removed by previous commits. Signed-off-by: Jonathon Jongsma --- src/qemu/THREADS.txt | 58 +++++------------------------------------- src/qemu/qemu_domain.c | 56 ++++------------------------------------ src/qemu/qemu_domain.h | 7 ----- 3 files changed, 11 insertions(+), 110 deletions(-) diff --git a/src/qemu/THREADS.txt b/src/qemu/THREADS.txt index aa428fda6a..a7d8709a43 100644 --- a/src/qemu/THREADS.txt +++ b/src/qemu/THREADS.txt @@ -61,11 +61,12 @@ There are a number of locks on various objects =20 Agent job condition is then used when thread wishes to talk to qemu agent monitor. It is possible to acquire just agent job - (qemuDomainObjBeginAgentJob), or only normal job - (qemuDomainObjBeginJob) or both at the same time - (qemuDomainObjBeginJobWithAgent). Which type of job to grab depends - whether caller wishes to communicate only with agent socket, or only - with qemu monitor socket or both, respectively. + (qemuDomainObjBeginAgentJob), or only normal job (qemuDomainObjBeginJo= b) + but not both at the same time. Holding an agent job and a normal job w= ould + allow an unresponsive or malicious agent to block normal libvirt API a= nd + potentially result in a denial of service. Which type of job to grab + depends whether caller wishes to communicate only with agent socket, or + only with qemu monitor socket. =20 Immediately after acquiring the virDomainObjPtr lock, any method which intends to update state must acquire asynchronous, normal or @@ -141,18 +142,6 @@ To acquire the agent job condition =20 =20 =20 -To acquire both normal and agent job condition - - qemuDomainObjBeginJobWithAgent() - - Waits until there is no normal and no agent job set - - Sets both job.active and job.agentActive with required job types - - qemuDomainObjEndJobWithAgent() - - Sets both job.active and job.agentActive to 0 - - Signals on job.cond condition - - - To acquire the asynchronous job condition =20 qemuDomainObjBeginAsyncJob() @@ -292,41 +281,6 @@ Design patterns virDomainObjEndAPI(&obj); =20 =20 - * Invoking both monitor and agent commands on a virDomainObjPtr - - virDomainObjPtr obj; - qemuAgentPtr agent; - - obj =3D qemuDomObjFromDomain(dom); - - qemuDomainObjBeginJobWithAgent(obj, QEMU_JOB_TYPE, QEMU_AGENT_JOB_TYP= E); - - if (!virDomainObjIsActive(dom)) - goto cleanup; - - ...do prep work... - - if (!qemuDomainAgentAvailable(obj, true)) - goto cleanup; - - agent =3D qemuDomainObjEnterAgent(obj); - qemuAgentXXXX(agent, ..); - qemuDomainObjExitAgent(obj, agent); - - ... - - qemuDomainObjEnterMonitor(obj); - qemuMonitorXXXX(priv->mon); - qemuDomainObjExitMonitor(obj); - - /* Alternatively, talk to the monitor first and then talk to the agen= t. */ - - ...do final work... - - qemuDomainObjEndJobWithAgent(obj); - virDomainObjEndAPI(&obj); - - * Running asynchronous job =20 virDomainObjPtr obj; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 470d342afc..97cf6b2255 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8569,26 +8569,6 @@ qemuDomainObjBeginAgentJob(virQEMUDriverPtr driver, QEMU_ASYNC_JOB_NONE, false); } =20 -/** - * qemuDomainObjBeginJobWithAgent: - * - * Grabs both monitor and agent types of job. Use if caller talks to - * both monitor and guest agent. However, if @job (or @agentJob) is - * QEMU_JOB_NONE (or QEMU_AGENT_JOB_NONE) only agent job is acquired (or - * monitor job). - * - * To end job call qemuDomainObjEndJobWithAgent. - */ -int -qemuDomainObjBeginJobWithAgent(virQEMUDriverPtr driver, - virDomainObjPtr obj, - qemuDomainJob job, - qemuDomainAgentJob agentJob) -{ - return qemuDomainObjBeginJobInternal(driver, obj, job, agentJob, - QEMU_ASYNC_JOB_NONE, false); -} - int qemuDomainObjBeginAsyncJob(virQEMUDriverPtr driver, virDomainObjPtr obj, qemuDomainAsyncJob asyncJob, @@ -8703,31 +8683,6 @@ qemuDomainObjEndAgentJob(virDomainObjPtr obj) virCondBroadcast(&priv->job.cond); } =20 -void -qemuDomainObjEndJobWithAgent(virQEMUDriverPtr driver, - virDomainObjPtr obj) -{ - qemuDomainObjPrivatePtr priv =3D obj->privateData; - qemuDomainJob job =3D priv->job.active; - qemuDomainAgentJob agentJob =3D priv->job.agentActive; - - priv->jobs_queued--; - - VIR_DEBUG("Stopping both jobs: %s %s (async=3D%s vm=3D%p name=3D%s)", - qemuDomainJobTypeToString(job), - qemuDomainAgentJobTypeToString(agentJob), - qemuDomainAsyncJobTypeToString(priv->job.asyncJob), - obj, obj->def->name); - - qemuDomainObjResetJob(priv); - qemuDomainObjResetAgentJob(priv); - if (qemuDomainTrackJob(job)) - qemuDomainObjSaveStatus(driver, obj); - /* We indeed need to wake up ALL threads waiting because - * grabbing a job requires checking more variables. */ - virCondBroadcast(&priv->job.cond); -} - void qemuDomainObjEndAsyncJob(virQEMUDriverPtr driver, virDomainObjPtr obj) { @@ -8761,9 +8716,9 @@ qemuDomainObjAbortAsyncJob(virDomainObjPtr obj) * obj must be locked before calling * * To be called immediately before any QEMU monitor API call - * Must have already either called qemuDomainObjBeginJob() or - * qemuDomainObjBeginJobWithAgent() and checked that the VM is - * still active; may not be used for nested async jobs. + * Must have already called qemuDomainObjBeginJob() and checked + * that the VM is still active; may not be used for nested async + * jobs. * * To be followed with qemuDomainObjExitMonitor() once complete */ @@ -8885,9 +8840,8 @@ qemuDomainObjEnterMonitorAsync(virQEMUDriverPtr drive= r, * obj must be locked before calling * * To be called immediately before any QEMU agent API call. - * Must have already called qemuDomainObjBeginAgentJob() or - * qemuDomainObjBeginJobWithAgent() and checked that the VM is - * still active. + * Must have already called qemuDomainObjBeginAgentJob() and + * checked that the VM is still active. * * To be followed with qemuDomainObjExitAgent() once complete */ diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index f626d3a54c..fb4c9e0467 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -622,11 +622,6 @@ int qemuDomainObjBeginAgentJob(virQEMUDriverPtr driver, virDomainObjPtr obj, qemuDomainAgentJob agentJob) G_GNUC_WARN_UNUSED_RESULT; -int qemuDomainObjBeginJobWithAgent(virQEMUDriverPtr driver, - virDomainObjPtr obj, - qemuDomainJob job, - qemuDomainAgentJob agentJob) - G_GNUC_WARN_UNUSED_RESULT; int qemuDomainObjBeginAsyncJob(virQEMUDriverPtr driver, virDomainObjPtr obj, qemuDomainAsyncJob asyncJob, @@ -645,8 +640,6 @@ int qemuDomainObjBeginJobNowait(virQEMUDriverPtr driver, void qemuDomainObjEndJob(virQEMUDriverPtr driver, virDomainObjPtr obj); void qemuDomainObjEndAgentJob(virDomainObjPtr obj); -void qemuDomainObjEndJobWithAgent(virQEMUDriverPtr driver, - virDomainObjPtr obj); void qemuDomainObjEndAsyncJob(virQEMUDriverPtr driver, virDomainObjPtr obj); void qemuDomainObjAbortAsyncJob(virDomainObjPtr obj); --=20 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list