diff options
author | Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> | 2014-01-25 23:35:28 -0500 |
---|---|---|
committer | Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> | 2014-01-27 23:17:02 -0500 |
commit | 65b3903ff576488eaabb51d3c4fbf9c73d867d7c (patch) | |
tree | 0343e9d79057f6ca67acb87d6cc06f45ab7a1384 /src | |
parent | 8e33886ec582336564ae11b80023abe93d7599c0 (diff) |
journal: guarantee async-signal-safety in sd_journald_sendv
signal(7) provides a list of functions which may be called from a
signal handler. Other functions, which only call those functions and
don't access global memory and are reentrant are also safe.
sd_j_sendv was mostly OK, but would call mkostemp and writev in a
fallback path, which are unsafe.
Being able to call sd_j_sendv in a async-signal-safe way is important
because it allows it be used in signal handlers.
Safety is achieved by replacing mkostemp with open(O_TMPFILE) and an
open-coded writev replacement which uses write. Unfortunately,
O_TMPFILE is only available on kernels >= 3.11. When O_TMPFILE is
unavailable, an open-coded mkostemp is used.
https://bugzilla.gnome.org/show_bug.cgi?id=722889
Diffstat (limited to 'src')
-rw-r--r-- | src/journal/journal-send.c | 2 | ||||
-rw-r--r-- | src/shared/def.h | 1 | ||||
-rw-r--r-- | src/shared/missing.h | 16 | ||||
-rw-r--r-- | src/shared/util.c | 54 | ||||
-rw-r--r-- | src/shared/util.h | 3 | ||||
-rw-r--r-- | src/test/test-tmpfiles.c | 51 | ||||
-rw-r--r-- | src/test/test-util.c | 26 |
7 files changed, 141 insertions, 12 deletions
diff --git a/src/journal/journal-send.c b/src/journal/journal-send.c index ca9199f718..960c5776b0 100644 --- a/src/journal/journal-send.c +++ b/src/journal/journal-send.c @@ -314,7 +314,7 @@ _public_ int sd_journal_sendv(const struct iovec *iov, int n) { if (buffer_fd < 0) return buffer_fd; - n = writev(buffer_fd, w, j); + n = writev_safe(buffer_fd, w, j); if (n < 0) { close_nointr_nofail(buffer_fd); return -errno; diff --git a/src/shared/def.h b/src/shared/def.h index a2304fddda..7777756aad 100644 --- a/src/shared/def.h +++ b/src/shared/def.h @@ -44,6 +44,7 @@ #define LOWERCASE_LETTERS "abcdefghijklmnopqrstuvwxyz" #define UPPERCASE_LETTERS "ABCDEFGHIJKLMNOPQRSTUVWXYZ" #define LETTERS LOWERCASE_LETTERS UPPERCASE_LETTERS +#define ALPHANUMERICAL LETTERS DIGITS #define REBOOT_PARAM_FILE "/run/systemd/reboot-param" diff --git a/src/shared/missing.h b/src/shared/missing.h index 6c038d9f08..4e62100030 100644 --- a/src/shared/missing.h +++ b/src/shared/missing.h @@ -301,25 +301,29 @@ static inline int name_to_handle_at(int fd, const char *name, struct file_handle #endif #ifndef CIFS_MAGIC_NUMBER -#define CIFS_MAGIC_NUMBER 0xFF534D42 +# define CIFS_MAGIC_NUMBER 0xFF534D42 #endif #ifndef TFD_TIMER_CANCEL_ON_SET -#define TFD_TIMER_CANCEL_ON_SET (1 << 1) +# define TFD_TIMER_CANCEL_ON_SET (1 << 1) #endif #ifndef SO_REUSEPORT -#define SO_REUSEPORT 15 +# define SO_REUSEPORT 15 #endif #ifndef EVIOCREVOKE -#define EVIOCREVOKE _IOW('E', 0x91, int) +# define EVIOCREVOKE _IOW('E', 0x91, int) #endif #ifndef DRM_IOCTL_SET_MASTER -#define DRM_IOCTL_SET_MASTER _IO('d', 0x1e) +# define DRM_IOCTL_SET_MASTER _IO('d', 0x1e) #endif #ifndef DRM_IOCTL_DROP_MASTER -#define DRM_IOCTL_DROP_MASTER _IO('d', 0x1f) +# define DRM_IOCTL_DROP_MASTER _IO('d', 0x1f) +#endif + +#ifndef TMP_MAX +# define TMP_MAX 238328 #endif diff --git a/src/shared/util.c b/src/shared/util.c index 27fc9594cd..a437d9b127 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -6109,6 +6109,53 @@ int getpeersec(int fd, char **ret) { return 0; } +int writev_safe(int fd, const struct iovec *w, int j) { + for (int i = 0; i < j; i++) { + size_t written = 0; + + while (written < w[i].iov_len) { + ssize_t r; + + r = write(fd, (char*) w[i].iov_base + written, w[i].iov_len - written); + if (r < 0 && errno != -EINTR) + return -errno; + + written += r; + } + } + + return 0; +} + +int mkostemp_safe(char *pattern, int flags) { + char *s = pattern + strlen(pattern) - 6; + uint64_t tries = TMP_MAX; + int randfd, fd, i; + + assert(streq(s, "XXXXXX")); + + randfd = open("/dev/urandom", O_RDONLY); + if (randfd < 0) + return -ENOSYS; + + while (tries--) { + fd = read(randfd, s, 6); + if (fd == 0) + return -ENOSYS; + + for (i = 0; i < 6; i++) + s[i] = ALPHANUMERICAL[(unsigned) s[i] % strlen(ALPHANUMERICAL)]; + + fd = open(pattern, flags|O_EXCL|O_CREAT, S_IRUSR|S_IWUSR); + if (fd >= 0) + return fd; + if (!IN_SET(errno, EEXIST, EINTR)) + return -errno; + } + + return -EEXIST; +} + int open_tmpfile(const char *path, int flags) { int fd; char *p; @@ -6120,12 +6167,9 @@ int open_tmpfile(const char *path, int flags) { #endif p = strappenda(path, "/systemd-tmp-XXXXXX"); - RUN_WITH_UMASK(0077) { - fd = mkostemp(p, O_RDWR|O_CLOEXEC); - } - + fd = mkostemp_safe(p, O_RDWR|O_CLOEXEC); if (fd < 0) - return -errno; + return fd; unlink(p); return fd; diff --git a/src/shared/util.h b/src/shared/util.h index 630137a53a..1169864c3a 100644 --- a/src/shared/util.h +++ b/src/shared/util.h @@ -850,4 +850,7 @@ bool pid_valid(pid_t pid); int getpeercred(int fd, struct ucred *ucred); int getpeersec(int fd, char **ret); +int writev_safe(int fd, const struct iovec *w, int j); + +int mkostemp_safe(char *pattern, int flags); int open_tmpfile(const char *path, int flags); diff --git a/src/test/test-tmpfiles.c b/src/test/test-tmpfiles.c new file mode 100644 index 0000000000..f25a0dca52 --- /dev/null +++ b/src/test/test-tmpfiles.c @@ -0,0 +1,51 @@ +/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/ + +/*** + This file is part of systemd. + + Copyright 2014 Zbigniew Jędrzejewski-Szmek + + systemd is free software; you can redistribute it and/or modify it + under the terms of the GNU Lesser General Public License as published by + the Free Software Foundation; either version 2.1 of the License, or + (at your option) any later version. + + systemd is distributed in the hope that it will be useful, but + WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public License + along with systemd; If not, see <http://www.gnu.org/licenses/>. +***/ + +#include <sys/types.h> +#include <sys/stat.h> +#include <fcntl.h> +#include <unistd.h> +#include <stdlib.h> +#include <stdio.h> + +#include "util.h" + +int main(int argc, char** argv) { + const char *p = argv[1] ?: "/tmp"; + char *pattern = strappenda(p, "/systemd-test-XXXXXX"); + _cleanup_close_ int fd, fd2; + _cleanup_free_ char *cmd, *cmd2; + + fd = open_tmpfile(p, O_RDWR); + assert(fd >= 0); + + assert_se(asprintf(&cmd, "ls -l /proc/"PID_FMT"/fd/%d", getpid(), fd) > 0); + system(cmd); + + fd2 = mkostemp_safe(pattern, O_RDWR); + assert(fd >= 0); + assert_se(unlink(pattern) == 0); + + assert_se(asprintf(&cmd2, "ls -l /proc/"PID_FMT"/fd/%d", getpid(), fd2) > 0); + system(cmd2); + + return 0; +} diff --git a/src/test/test-util.c b/src/test/test-util.c index 05b2294e24..f819589b52 100644 --- a/src/test/test-util.c +++ b/src/test/test-util.c @@ -28,6 +28,8 @@ #include "util.h" #include "strv.h" +#include "def.h" +#include "fileio.h" static void test_streq_ptr(void) { assert_se(streq_ptr(NULL, NULL)); @@ -578,6 +580,29 @@ static void test_in_set(void) { assert_se(!IN_SET(0, 1, 2, 3, 4)); } +static void test_writev_safe(void) { + char name[] = "/tmp/test-writev_safe.XXXXXX"; + _cleanup_free_ char *contents; + size_t size; + int fd, r; + + struct iovec iov[3]; + IOVEC_SET_STRING(iov[0], "abc\n"); + IOVEC_SET_STRING(iov[1], ALPHANUMERICAL "\n"); + IOVEC_SET_STRING(iov[2], ""); + + fd = mkstemp(name); + printf("test_writev_safe: %s", name); + + r = writev_safe(fd, iov, 3); + assert(r == 0); + + r = read_full_file(name, &contents, &size); + assert(r == 0); + printf("contents: %s", contents); + assert(streq(contents, "abc\n" ALPHANUMERICAL "\n")); +} + int main(int argc, char *argv[]) { test_streq_ptr(); test_first_word(); @@ -615,6 +640,7 @@ int main(int argc, char *argv[]) { test_fstab_node_to_udev_node(); test_get_files_in_directory(); test_in_set(); + test_writev_safe(); return 0; } |