Re: [Qemu-devel] [PATCH for-2.12] os: truncate pidfile on creation

[Eric Blake]

On 03/20/2018 11:02 AM, Daniel P. Berrangé wrote:
> On Tue, Mar 20, 2018 at 09:50:19AM -0500, Eric Blake wrote:
> > On 03/20/2018 04:31 AM, Florian Larysch wrote:
> > > qemu_create_pidfile does not truncate the pidfile when it creates it,
> > > but rather overwrites its contents with the new pid. This works fine as
> > > long as the length of the pid doesn't decrease, but this might happen in
> > > case of wraparounds, causing pidfiles to contain trailing garbage which
> > > breaks operations such as 'kill $(cat pidfile)'.
> >
> > Ouch.  Good analysis.
> >
> > However, I have to wonder if we have the opposite problem - when the file
> > exists (truncated) but has not yet been written, how often do we have a race
> > where someone can see the empty file?

> > This part is right, if we don't care about the race with someone reading an
> > empty file.
>
> No, it is unsafe - we rely on lockf() to get the mutual exclusion.

Oh, right, because we aren't using O_EXCL.

> If a QEMU is running with pidfile locked, and its pid written into
> it, then a 2nd QEMU comes along it will truncate the pidfile owned
> by the original QEMU because the truncation happens before it has
> tried to acquire the lock. The 2nd QEMU will still exit, but the
> original QEMU's pid has now been lost.
>
> We must call ftruncate() after lockf(), but before writing the new
> pid into the file. That ensures there is no window in which it is
> possible to see the new & old pids mixed together.

So the window of time where the file can be empty still exists, but it 
must be AFTER the lockf() (and the window where reading the file content 
can see the wrong pid is longer - from the open until the ftruncate - 
but at least the wrong data is limited to a previous pid or the empty 
string, rather than a completely random wrong value from squashing 
together two unrelated pids).

We'll need a v2.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org