| Message ID | 20180320165855.23258-1-fl@n621.de |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] os: truncate pidfile on creation | expand |
On 03/20/2018 11:58 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)'. > > Instead, always truncate the file before writing it. > > Note that the order is important here: We cannot simply use O_TRUNC in > the open() call because another qemu process might truncate the pidfile > of a process that is still running before reaching the lockf() barrier. > > The Windows version suffers from a similar problem, but as it does not > provide effective mutual exclusion anyway (because the file handle is > closed immediately after writing to it), adopting this behavior still > seems to be an improvement, as it at least prevents garbeled pidfiles. s/garbeled/garbled/ > > Signed-off-by: Florian Larysch <fl@n621.de> > --- > os-posix.c | 6 ++++++ > os-win32.c | 2 +- > 2 files changed, 7 insertions(+), 1 deletion(-) > Reviewed-by: Eric Blake <eblake@redhat.com>
diff --git a/os-posix.c b/os-posix.c index b9c2343b1e..f2318aef55 100644 --- a/os-posix.c +++ b/os-posix.c @@ -309,6 +309,12 @@ int qemu_create_pidfile(const char *filename) close(fd); return -1; } + + if (ftruncate(fd, 0)) { + close(fd); + return -1; + } + len = snprintf(buffer, sizeof(buffer), FMT_pid "\n", getpid()); if (write(fd, buffer, len) != len) { close(fd); diff --git a/os-win32.c b/os-win32.c index 586a7c7d49..85dbad7af8 100644 --- a/os-win32.c +++ b/os-win32.c @@ -108,7 +108,7 @@ int qemu_create_pidfile(const char *filename) memset(&overlap, 0, sizeof(overlap)); file = CreateFile(filename, GENERIC_WRITE, FILE_SHARE_READ, NULL, - OPEN_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); + CREATE_ALWAYS, FILE_ATTRIBUTE_NORMAL, NULL); if (file == INVALID_HANDLE_VALUE) { return -1;
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)'. Instead, always truncate the file before writing it. Note that the order is important here: We cannot simply use O_TRUNC in the open() call because another qemu process might truncate the pidfile of a process that is still running before reaching the lockf() barrier. The Windows version suffers from a similar problem, but as it does not provide effective mutual exclusion anyway (because the file handle is closed immediately after writing to it), adopting this behavior still seems to be an improvement, as it at least prevents garbeled pidfiles. Signed-off-by: Florian Larysch <fl@n621.de> --- os-posix.c | 6 ++++++ os-win32.c | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-)