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. The size parameter is repurposed, and if it is negative, sd_j_sendv will not fall back to the async-signal-unsafe path when O_TMPFILE is not available, returning an error instead. https://bugzilla.gnome.org/show_bug.cgi?id=722889 --- man/sd_journal_print.xml | 65 +++++++++++++++++++++++++++++++++++++++++--- src/core/manager.c | 2 +- src/journal/journal-send.c | 31 +++++++++++++++++++-- src/journal/journal-verify.c | 6 ++-- src/shared/util.c | 5 +++- src/shared/util.h | 2 +- 6 files changed, 99 insertions(+), 12 deletions(-) diff --git a/man/sd_journal_print.xml b/man/sd_journal_print.xml index a716cc3..4c49fe8 100644 --- a/man/sd_journal_print.xml +++ b/man/sd_journal_print.xml @@ -152,9 +152,12 @@ <citerefentry><refentrytitle>readv</refentrytitle><manvolnum>3</manvolnum></citerefentry> for details) instead of the format string. Each structure should reference one field of the entry to - submit. The second argument specifies the number of - structures in the - array. <function>sd_journal_sendv()</function> is + submit. The absolute value of the second argument + specifies the number of structures in the array. If + <parameter>n</parameter> is negative, + <function>sd_journal_sendv()</function> will behave + in a "async signal safe" way, see below. + <function>sd_journal_sendv()</function> is particularly useful to submit binary objects to the journal where that is necessary.</para> @@ -221,6 +224,47 @@ sd_journal_send("MESSAGE=Hello World, this is PID %lu!", (unsigned long) getpid( </refsect1> <refsect1> + <title>Async signal safety</title> + <para><function>sd_journal_sendv()</function> is "async signal + safe" in the meaning of <citerefentry><refentrytitle>signal</refentrytitle><manvolnum>7</manvolnum></citerefentry>, + when any of the following conditions are true: + </para> + + <itemizedlist> + <listitem><para>The running kernel supports + <constant>O_TMPFILE</constant> (Linux v. 3.11 + or higher).</para></listitem> + + <listitem><para>The whole entry is small + enough to fit in one datagram. + <function>sd_journal_fd()</function> will + attempt to set the maximum size to a fairly + large value (currently 8 MiB) using + <constant>SO_SNDBUF</constant> and + <constant>SO_SNDBUFFORCE</constant> if + priviledges permit. However, an error to + increase this limit is ignored, so the maximum + size is dependent on the effective + capabilities and configuration of the system. + </para></listitem> + </itemizedlist> + + <para>When the conditions for async-signal-safe + execution are not met, behaviour of + <function>sd_journal_sendv()</function> depends on the + sign of the <parameter>n</parameter> parameter. If it + was positive, this function will fall back to a + non-async signal safe code path. If it was negative, + an error will be returned.</para> + + <para><function>sd_journal_print</function>, + <function>sd_journal_printv</function>, + <function>sd_journal_send</function>, and + <function>sd_journal_perror</function> are + not async signal safe.</para> + </refsect1> + + <refsect1> <title>Notes</title> <para>The <function>sd_journal_print()</function>, @@ -234,6 +278,17 @@ sd_journal_send("MESSAGE=Hello World, this is PID %lu!", (unsigned long) getpid( </refsect1> <refsect1> + <title>History</title> + + <para><function>sd_journal_sendv()</function> was + modified to accept negative length parameter and + guarantee async-signal-safety in systemd-209. Before + that, it would behave safely only when entry size was + small enough (the second of two conditions described + above).</para> + </refsect1> + + <refsect1> <title>See Also</title> <para> @@ -243,7 +298,9 @@ sd_journal_send("MESSAGE=Hello World, this is PID %lu!", (unsigned long) getpid( <citerefentry><refentrytitle>syslog</refentrytitle><manvolnum>3</manvolnum></citerefentry>, <citerefentry><refentrytitle>perror</refentrytitle><manvolnum>3</manvolnum></citerefentry>, <citerefentry><refentrytitle>errno</refentrytitle><manvolnum>3</manvolnum></citerefentry>, - <citerefentry><refentrytitle>systemd.journal-fields</refentrytitle><manvolnum>7</manvolnum></citerefentry> + <citerefentry><refentrytitle>systemd.journal-fields</refentrytitle><manvolnum>7</manvolnum></citerefentry>, + <citerefentry><refentrytitle>signal</refentrytitle><manvolnum>7</manvolnum></citerefentry>, + <citerefentry><refentrytitle>socket</refentrytitle><manvolnum>7</manvolnum></citerefentry> </para> </refsect1> diff --git a/src/core/manager.c b/src/core/manager.c index 7156c38..5155121 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1992,7 +1992,7 @@ int manager_open_serialization(Manager *m, FILE **_f) { assert(_f); path = m->running_as == SYSTEMD_SYSTEM ? "/run/systemd" : "/tmp"; - fd = open_tmpfile(path, O_RDWR|O_CLOEXEC); + fd = open_tmpfile(path, O_RDWR|O_CLOEXEC, false); if (fd < 0) return -errno; diff --git a/src/journal/journal-send.c b/src/journal/journal-send.c index ca9199f..42a3d98 100644 --- a/src/journal/journal-send.c +++ b/src/journal/journal-send.c @@ -196,6 +196,27 @@ finish: return r; } +static int writev_safe(int fd, const struct iovec *w, int j, bool async_signal_safe) { + if (!async_signal_safe) + return writev(fd, w, j); + + for (int i = 0; i < j; i++) { + size_t written = 0; + + while (written < w[i].iov_len) { + ssize_t r; + + r = write(fd, w[i].iov_base + written, w[i].iov_len - written); + if (r < 0 && errno != -EINTR) + return -errno; + + written += r; + } + } + + return 0; +} + _public_ int sd_journal_sendv(const struct iovec *iov, int n) { PROTECT_ERRNO; int fd, buffer_fd; @@ -217,6 +238,12 @@ _public_ int sd_journal_sendv(const struct iovec *iov, int n) { } control; struct cmsghdr *cmsg; bool have_syslog_identifier = false; + bool async_signal_safe = false; + + if (n < 0) { + async_signal_safe = true; + n = -n; + }; assert_return(iov, -EINVAL); assert_return(n > 0, -EINVAL); @@ -310,11 +337,11 @@ _public_ int sd_journal_sendv(const struct iovec *iov, int n) { * We use /dev/shm instead of /tmp here, since we want this to * be a tmpfs, and one that is available from early boot on * and where unprivileged users can create files. */ - buffer_fd = open_tmpfile("/dev/shm", O_RDWR | O_CLOEXEC); + buffer_fd = open_tmpfile("/dev/shm", O_RDWR | O_CLOEXEC, async_signal_safe); if (buffer_fd < 0) return buffer_fd; - n = writev(buffer_fd, w, j); + n = writev_safe(buffer_fd, w, j, async_signal_safe); if (n < 0) { close_nointr_nofail(buffer_fd); return -errno; diff --git a/src/journal/journal-verify.c b/src/journal/journal-verify.c index 9434cc9..f7e42ba 100644 --- a/src/journal/journal-verify.c +++ b/src/journal/journal-verify.c @@ -805,21 +805,21 @@ int journal_file_verify( } else if (f->seal) return -ENOKEY; - data_fd = open_tmpfile("/var/tmp", O_RDWR | O_CLOEXEC); + data_fd = open_tmpfile("/var/tmp", O_RDWR | O_CLOEXEC, false); if (data_fd < 0) { log_error("Failed to create data file: %m"); r = -errno; goto fail; } - entry_fd = open_tmpfile("/var/tmp", O_RDWR | O_CLOEXEC); + entry_fd = open_tmpfile("/var/tmp", O_RDWR | O_CLOEXEC, false); if (entry_fd < 0) { log_error("Failed to create entry file: %m"); r = -errno; goto fail; } - entry_array_fd = open_tmpfile("/var/tmp", O_RDWR | O_CLOEXEC); + entry_array_fd = open_tmpfile("/var/tmp", O_RDWR | O_CLOEXEC, false); if (entry_array_fd < 0) { log_error("Failed to create entry array file: %m"); r = -errno; diff --git a/src/shared/util.c b/src/shared/util.c index 34b3d96..48e94c4 100644 --- a/src/shared/util.c +++ b/src/shared/util.c @@ -6106,7 +6106,7 @@ int getpeersec(int fd, char **ret) { return 0; } -int open_tmpfile(const char *path, int flags) { +int open_tmpfile(const char *path, int flags, bool async_signal_safe) { int fd; char *p; @@ -6115,6 +6115,9 @@ int open_tmpfile(const char *path, int flags) { if (fd >= 0) return fd; #endif + if (async_signal_safe) + return -ENOTSUP; + p = strappenda(path, "/systemd-tmp-XXXXXX"); RUN_WITH_UMASK(0077) { diff --git a/src/shared/util.h b/src/shared/util.h index 630137a..6144b1c 100644 --- a/src/shared/util.h +++ b/src/shared/util.h @@ -850,4 +850,4 @@ bool pid_valid(pid_t pid); int getpeercred(int fd, struct ucred *ucred); int getpeersec(int fd, char **ret); -int open_tmpfile(const char *path, int flags); +int open_tmpfile(const char *path, int flags, bool async_signal_safe); -- 1.8.5.3 _______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
