On Thu, 17 Nov 2016 12:17:59 +0000 Daniel Stone <dani...@collabora.com> wrote:
> The X11 lock file was somewhat opaque. Into a sized array of 16 > characters, we previously read 11 bytes. 61beda653b fixed the parsing of > this input to ensure that we only considered the first 10 bytes: this > has the effect of culling a LF byte at the end of the string. > > This commit more explicitly NULLs the entire string before reading, and > trims trailing LF characters only. > > It also adds some documentation by way of resizing pid, an explicit size > check on snprintf's return, and comments. > > Verified manually that it emits lock files with a trailing \n, as Xorg > does. Also verified manually that it ignores misformatted lock files, > but accepts either \n or \0 in the trailing position. > > Related Mutter issue: https://bugzilla.gnome.org/show_bug.cgi?id=774613 > > Signed-off-by: Daniel Stone <dani...@collabora.com> > Cc: Pekka Paalanen <pekka.paala...@collabora.co.uk> > Cc: Eric Engestrom <e...@engestrom.ch> > --- > xwayland/launcher.c | 24 +++++++++++++++++------- > 1 file changed, 17 insertions(+), 7 deletions(-) > > diff --git a/xwayland/launcher.c b/xwayland/launcher.c > index 97d7c6e..c3114f5 100644 > --- a/xwayland/launcher.c > +++ b/xwayland/launcher.c > @@ -148,13 +148,19 @@ bind_to_unix_socket(int display) > static int > create_lockfile(int display, char *lockfile, size_t lsize) > { > - char pid[16]; > + /* 10 decimal characters, trailing LF and NUL byte; see comment > + * at end of function. */ > + char pid[11]; Hi, um, 11? No room for the terminator-for-sure? Otherwise looks good. Thanks, pq > int fd, size; > pid_t other; > > snprintf(lockfile, lsize, "/tmp/.X%d-lock", display); > fd = open(lockfile, O_WRONLY | O_CLOEXEC | O_CREAT | O_EXCL, 0444); > if (fd < 0 && errno == EEXIST) { > + /* Clear our input buffer so we always have a NUL-terminated > + * string. */ > + memset(pid, 0, sizeof(pid)); > + > fd = open(lockfile, O_CLOEXEC | O_RDONLY); > if (fd < 0 || read(fd, pid, 11) != 11) { > weston_log("can't read lock file %s: %s\n", > @@ -166,8 +172,10 @@ create_lockfile(int display, char *lockfile, size_t > lsize) > return -1; > } > > - /* Trim the newline, ensure terminated string. */ > - pid[10] = '\0'; > + /* If the last character is LF, trim it so safe_strtoint > + * doesn't explode. */ > + if (pid[10] == '\n') > + pid[10] = '\0'; > > if (!safe_strtoint(pid, &other)) { > weston_log("can't parse lock file %s\n", > @@ -199,10 +207,12 @@ create_lockfile(int display, char *lockfile, size_t > lsize) > return -1; > } > > - /* Subtle detail: we use the pid of the wayland > - * compositor, not the xserver in the lock file. */ > - size = snprintf(pid, sizeof pid, "%10d\n", getpid()); > - if (write(fd, pid, size) != size) { > + /* Subtle detail: we use the pid of the wayland compositor, not the > + * xserver in the lock file. > + * Also subtle is that we don't emit a trailing NUL to the file, so > + * our size here is 11 rather than 12. */ > + size = dprintf(fd, "%10d\n", getpid()); > + if (size != 11) { > unlink(lockfile); > close(fd); > return -1; _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
