[systemd-devel] [RFC PATCH 2/2] journal: add async-signal-safe mode for sd_journald_sendv

Zbigniew Jędrzejewski-Szmek zbyszek at in.waw.pl
Sat Jan 25 21:21:18 PST 2014


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



More information about the systemd-devel mailing list