[PATCH 1/1] qemu_tpm: Get swtpm pid without binary validation

[Vasily Ulyanov]

Hi Michal,

On 11/01/2022 11:00, Michal Prívozník wrote:
> 
> Firstly, this fixes the problem only for swtpm case, but there is one
> other place where the same problem can happen (qemuVhostUserGPUGetPid()).
> 

Sure, I can also try to fix the vhost gpu case. Was mostly focused on tpm hence
did not look at other places.

> From conceptual POV, what may now happen is that when PID wrapped we may
> wrongly detect another process as swtpm.
> 

Right, valid point...

> 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.
> 

Unfortunately swtpm does not lock the pid file at all :(

> But this could all be mitigated by using virCommandSetPidFile() when
> spawning the command instead of passing arbitrary --pid arguments.
> 

So I can drop --pid and --daemon args from swtpm cmd and add

    virCommandSetPidFile(cmd, pidfile);
    virCommandDaemonize(cmd);

Then in virPidFileReadPathIfAlive() I can add a lock check. Hm... Nice, that
seems like a good way to go.


-- 
Vasily Ulyanov <vulyanov at suse.de>
Software Engineer, SUSE Labs Core