[PATCH v3 2/3] qemu: tpm: Get swtpm pid without binary validation

[Vasiliy Ulyanov <vulyanov@xxxxxxx>]

Access to /proc/[pid]/exe may be restricted in certain environments (e.g.
in containers) and any attempt to stat(2) or readlink(2) the file will
result in 'permission denied' error if the calling process does not have
CAP_SYS_PTRACE capability. According to proc(5) manpage:

Permission to dereference or read (readlink(2)) this symbolic link is
governed by a ptrace access mode PTRACE_MODE_READ_FSCREDS check; see
ptrace(2).

The binary validation in virPidFileReadPathIfAlive may fail with EACCES.
Therefore instead do only the check that the pidfile is locked by the
correct process. To ensure this is always the case the daemonization and
pidfile handling of the swtpm command is now controlled by libvirt.

Signed-off-by: Vasiliy Ulyanov <vulyanov@xxxxxxx>
---
 src/qemu/qemu_tpm.c | 40 +++++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 7e7b01768e..47c7891a4f 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -258,13 +258,13 @@ qemuTPMEmulatorGetPid(const char *swtpmStateDir,
                       const char *shortName,
                       pid_t *pid)
 {
-    g_autofree char *swtpm = virTPMGetSwtpm();
     g_autofree char *pidfile = qemuTPMEmulatorCreatePidFilename(swtpmStateDir,
                                                                 shortName);
+
     if (!pidfile)
         return -1;
 
-    if (virPidFileReadPathIfAlive(pidfile, pid, swtpm) < 0)
+    if (virPidFileReadPathIfLocked(pidfile, pid) < 0)
         return -1;
 
     return 0;
@@ -721,7 +721,7 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
 
     virCommandClearCaps(cmd);
 
-    virCommandAddArgList(cmd, "socket", "--daemon", "--ctrl", NULL);
+    virCommandAddArgList(cmd, "socket", "--ctrl", NULL);
     virCommandAddArgFormat(cmd, "type=unixio,path=%s,mode=0600",
                            tpm->data.emulator.source->data.nix.path);
 
@@ -751,8 +751,8 @@ qemuTPMEmulatorBuildCommand(virDomainTPMDef *tpm,
     if (!(pidfile = qemuTPMEmulatorCreatePidFilename(swtpmStateDir, shortName)))
         goto error;
 
-    virCommandAddArg(cmd, "--pid");
-    virCommandAddArgFormat(cmd, "file=%s", pidfile);
+    virCommandSetPidFile(cmd, pidfile);
+    virCommandDaemonize(cmd);
 
     if (tpm->data.emulator.hassecretuuid) {
         if (!virTPMSwtpmCapsGet(VIR_TPM_SWTPM_FEATURE_CMDARG_PWD_FD)) {
@@ -905,7 +905,7 @@ qemuExtTPMStartEmulator(virQEMUDriver *driver,
 {
     g_autoptr(virCommand) cmd = NULL;
     int exitstatus = 0;
-    g_autofree char *errbuf = NULL;
+    VIR_AUTOCLOSE errfd = -1;
     g_autoptr(virQEMUDriverConfig) cfg = NULL;
     g_autofree char *shortName = virDomainDefGetShortName(vm->def);
     int cmdret = 0, timeout, rc;
@@ -930,7 +930,7 @@ qemuExtTPMStartEmulator(virQEMUDriver *driver,
     if (qemuExtDeviceLogCommand(driver, vm, cmd, "TPM Emulator") < 0)
         return -1;
 
-    virCommandSetErrorBuffer(cmd, &errbuf);
+    virCommandSetErrorFD(cmd, &errfd);
 
     if (qemuSecurityStartTPMEmulator(driver, vm, cmd,
                                      cfg->swtpm_user, cfg->swtpm_group,
@@ -938,23 +938,33 @@ qemuExtTPMStartEmulator(virQEMUDriver *driver,
         return -1;
 
     if (cmdret < 0 || exitstatus != 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Could not start 'swtpm'. exitstatus: %d, "
-                         "error: %s"), exitstatus, errbuf);
+        char errbuf[1024] = { 0 };
+
+        if (saferead(errfd, errbuf, sizeof(errbuf) - 1) < 0) {
+            virReportSystemError(errno,
+                                 _("Could not start 'swtpm'. exitstatus: %d"),
+                                 exitstatus);
+        } else {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("Could not start 'swtpm'. exitstatus: %d, "
+                             "error: %s"), exitstatus, errbuf);
+        }
+
         return -1;
     }
 
-    /* check that the swtpm has written its pid into the file */
+    /* check that the swtpm has written its pid into the file and the control
+     * socket has been created. */
+    rc = qemuTPMEmulatorGetPid(cfg->swtpmStateDir, shortName, &pid);
+    if ((rc == 0 && pid == (pid_t)-1) || rc < 0)
+        goto error;
     timeout = 1000; /* ms */
     while (timeout > 0) {
-        rc = qemuTPMEmulatorGetPid(cfg->swtpmStateDir, shortName, &pid);
-        if (rc < 0) {
+        if (!virFileExists(tpm->data.emulator.source->data.nix.path)) {
             timeout -= 50;
             g_usleep(50 * 1000);
             continue;
         }
-        if (rc == 0 && pid == (pid_t)-1)
-            goto error;
         break;
     }
     if (timeout <= 0)
-- 
2.34.1