Re: [PATCH 1/1] qemu_tpm: Get swtpm pid without binary validation
On 1/3/22 08:56, Vasiliy Ulyanov wrote:
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).
If the first call to virPidFileReadPathIfAlive fails with EACCES try to
call it one more time without specifyng swtpm binary path in order to
avoid dereferencing the symlink.
Signed-off-by: Vasiliy Ulyanov <vulyanov(a)suse.de>
---
src/qemu/qemu_tpm.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 7e7b01768e..9c80e15e9b 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -261,10 +261,17 @@ qemuTPMEmulatorGetPid(const char *swtpmStateDir,
g_autofree char *swtpm = virTPMGetSwtpm();
g_autofree char *pidfile = qemuTPMEmulatorCreatePidFilename(swtpmStateDir,
shortName);
+ int rc;
+
if (!pidfile)
return -1;
- if (virPidFileReadPathIfAlive(pidfile, pid, swtpm) < 0)
+ rc = virPidFileReadPathIfAlive(pidfile, pid, swtpm);
+ /* If access to /proc/[pid]/exe is restricted then skip the validation of
+ * swtpm binary. */
+ if (rc < 0 && virLastErrorIsSystemErrno(EACCES))
+ rc = virPidFileReadPathIfAlive(pidfile, pid, NULL);
+ if (rc < 0)
return -1;
return 0;
Firstly, this fixes the problem only for swtpm case, but there is one
other place where the same problem can happen (qemuVhostUserGPUGetPid()).
From conceptual POV, what may now happen is that when PID wrapped we
may
wrongly detect another process as swtpm.
I'm wondering whether we can ditch the @binPath argument of
virPidFileReadPathIfAlive() completely. A pid file should be locked by
the process owning it [1], therefore what we could do is read given file
and try to lock it. If we succeeded it means that the process is dead
and we read a stale PID. If locking fails then the process is still
running and the PID we read is valid.
1: This is how libvirt treats pidfiles (see virPidFileAcquirePath()).
Question is whether other tools which use their own pidfiles do the
same. For instance, swtpm is given '--pid file=/some/path' argument but
I haven't checked whether it behaves as expected. Also, to make things
worse there are at least two types of file locks on Linux and both are
independent of each other, so question then is what type of lock does
given tool use.
But this could all be mitigated by using virCommandSetPidFile() when
spawning the command instead of passing arbitrary --pid arguments.
Michal