From nobody Sun Feb 8 21:33:26 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 216.205.24.74 as permitted sender) client-ip=216.205.24.74; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-74.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.74 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=1584980036; cv=none; d=zohomail.com; s=zohoarc; b=OChhT7nSkldCMwLb+7ownnkqog8lobCVICCVzrq2il/yD6DyjyYA2snEWbRkkqN2myHRJiDtQY8ebqDl72EFjC4w/U7wDsci9HQg8KH5KRQtc/O+zFHb3WN5DsU8WGWoKrHJq95IiZ/GvBXAzksSwHv+9Tib3/Jh+btsKpRUOrA= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1584980036; 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=My+47DyV2u8xhbDBsS5tJ2vK1ehy0F3jX8T/yhlfI+E=; b=EeMTZpwRpYBFs1BPndD9tfEf06+slutERFZ685FH0AOnlZa6iVjDPfWCo7A9gQeF+t3uJqvWKVvgccVOafC9utJruIzH4KzKBoJPugBWR910tgoO/yu2I87PJ3ZpSXQTS1zZp+JabaA1rcThYeSN6/8l6KJ21HLscZ2RqG3MyMQ= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 216.205.24.74 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-74.mimecast.com (us-smtp-delivery-74.mimecast.com [216.205.24.74]) by mx.zohomail.com with SMTPS id 1584980036494641.9994906544371; Mon, 23 Mar 2020 09:13:56 -0700 (PDT) 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-423-vITlVegTNyy3o779PDrmsg-1; Mon, 23 Mar 2020 12:13:52 -0400 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 568E0149C2; Mon, 23 Mar 2020 16:13:46 +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 2C5019B921; Mon, 23 Mar 2020 16:13:46 +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 D1E0018089CD; Mon, 23 Mar 2020 16:13:45 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 02NGDeGQ005208 for ; Mon, 23 Mar 2020 12:13:40 -0400 Received: by smtp.corp.redhat.com (Postfix) id E272494B30; Mon, 23 Mar 2020 16:13:40 +0000 (UTC) Received: from localhost.localdomain (unknown [10.40.192.151]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5F34F9497F for ; Mon, 23 Mar 2020 16:13:40 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1584980035; 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=My+47DyV2u8xhbDBsS5tJ2vK1ehy0F3jX8T/yhlfI+E=; b=XWlChh5g7AJcWISVZpWhr24pGrlYfBJpnmUeyzll24PQl2nykEYs1D15O2P3R2xLxwE8A+ Jcjm2NfrDGQLYhPAg1pgwNVKa7LXX2nqhYdPj60sfPt++o5zqAXHnchlHXG6+iU1IiA5Ow 8JD7cydxyUM116W2jn2kWFUOA82NdwM= X-MC-Unique: vITlVegTNyy3o779PDrmsg-1 From: Michal Privoznik To: libvir-list@redhat.com Subject: [PATCH RESEND 1/5] virCommand: Actually acquire pidfile instead of just writing it Date: Mon, 23 Mar 2020 17:13:30 +0100 Message-Id: <6a6a7d380aecd1cf644cf04fd61a6622bc9b5d64.1584979958.git.mprivozn@redhat.com> In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 X-loop: libvir-list@redhat.com 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.11 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) Content-Type: text/plain; charset="utf-8" Our virCommand module allows us to set a pidfile for commands we want to spawn. The caller constructs the string of pidfile path and then uses virCommandSetPidFile() to tell the module to write the pidfile once the command is ran. This usually works, but has two flaws: 1) the child process does not hold the pidfile open & locked. Therefore, the caller (or anybody else) can't use our fancy virPidFileForceCleanupPath() function to kill the command afterwards. Also, for everybody else on the system it's needlessly harder to check if the pid from the pidfile is still alive or not. 2) if the caller ever makes a mistake and passes the same pidfile path for two different commands, the start of the second command will overwrite the pidfile even though the first command might still be running. Signed-off-by: Michal Privoznik Reviewed-by: Marc-Andr=C3=A9 Lureau --- docs/internals/command.html.in | 4 ++- src/util/vircommand.c | 56 +++++++++++++++++++++++++++++----- tests/commanddata/test4.log | 1 + 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/docs/internals/command.html.in b/docs/internals/command.html.in index 8a9061e70f..823d89cc71 100644 --- a/docs/internals/command.html.in +++ b/docs/internals/command.html.in @@ -226,7 +226,9 @@ virCommandSetPidFile(cmd, "/var/run/dnsmasq.pid"); =20

This PID file is guaranteed to be written before - the intermediate process exits. + the intermediate process exits. Moreover, the daemonized + process will inherit the FD of the opened and locked PID + file.

=20

Reducing command privileges

diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 9ffa0cf49b..77078d09fb 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -612,6 +612,7 @@ virExec(virCommandPtr cmd) int null =3D -1; int pipeout[2] =3D {-1, -1}; int pipeerr[2] =3D {-1, -1}; + int pipesync[2] =3D {-1, -1}; int childin =3D cmd->infd; int childout =3D -1; int childerr =3D -1; @@ -745,9 +746,15 @@ virExec(virCommandPtr cmd) /* Initialize full logging for a while */ virLogSetFromEnv(); =20 + if (cmd->pidfile && + virPipe(pipesync) < 0) + goto fork_error; + /* Daemonize as late as possible, so the parent process can detect * the above errors with wait* */ if (cmd->flags & VIR_EXEC_DAEMON) { + char c; + if (setsid() < 0) { virReportSystemError(errno, "%s", _("cannot become session leader")); @@ -768,12 +775,13 @@ virExec(virCommandPtr cmd) } =20 if (pid > 0) { - if (cmd->pidfile && (virPidFileWritePath(cmd->pidfile, pid) < = 0)) { - if (virProcessKillPainfully(pid, true) >=3D 0) - virReportSystemError(errno, - _("could not write pidfile %s for= %d"), - cmd->pidfile, pid); - goto fork_error; + /* The parent expect us to have written the pid file before + * exiting. Wait here for the child to write it and signal us.= */ + if (cmd->pidfile && + saferead(pipesync[0], &c, sizeof(c)) !=3D sizeof(c)) { + virReportSystemError(errno, "%s", + _("Unable to wait for child process")= ); + _exit(EXIT_FAILURE); } _exit(EXIT_SUCCESS); } @@ -788,6 +796,37 @@ virExec(virCommandPtr cmd) if (cmd->setMaxCore && virProcessSetMaxCoreSize(0, cmd->maxCore) < 0) goto fork_error; + if (cmd->pidfile) { + VIR_AUTOCLOSE pidfilefd =3D -1; + int newpidfilefd =3D -1; + char c; + + pidfilefd =3D virPidFileAcquirePath(cmd->pidfile, false, getpid()); + if (pidfilefd < 0) + goto fork_error; + if (virSetInherit(pidfilefd, true) < 0) { + virReportSystemError(errno, "%s", + _("Cannot disable close-on-exec flag")); + goto fork_error; + } + + c =3D '1'; + if (safewrite(pipesync[1], &c, sizeof(c)) !=3D sizeof(c)) { + virReportSystemError(errno, "%s", _("Unable to notify child pr= ocess")); + goto fork_error; + } + VIR_FORCE_CLOSE(pipesync[0]); + VIR_FORCE_CLOSE(pipesync[1]); + + /* This is here only to move the pidfilefd + * to the lowest possible number. */ + if ((newpidfilefd =3D dup(pidfilefd)) < 0) { + virReportSystemError(errno, "%s", _("Unable to dup FD")); + goto fork_error; + } + + /* newpidfilefd is intentionally leaked. */ + } =20 if (cmd->hook) { VIR_DEBUG("Run hook %p %p", cmd->hook, cmd->opaque); @@ -1080,8 +1119,9 @@ virCommandPassFDGetFDIndex(virCommandPtr cmd, int fd) * @cmd: the command to modify * @pidfile: filename to use * - * Save the child PID in a pidfile. The pidfile will be populated - * before the exec of the child. + * Save the child PID in a pidfile. The pidfile will be populated before t= he + * exec of the child and the child will inherit opened and locked FD to the + * pidfile. */ void virCommandSetPidFile(virCommandPtr cmd, const char *pidfile) diff --git a/tests/commanddata/test4.log b/tests/commanddata/test4.log index f170be204e..5820f28307 100644 --- a/tests/commanddata/test4.log +++ b/tests/commanddata/test4.log @@ -9,6 +9,7 @@ ENV:USER=3Dtest FD:0 FD:1 FD:2 +FD:3 DAEMON:yes CWD:/ UMASK:0022 --=20 2.24.1