Merge pull request #15052 from jaankit/journal-send

journal-send: Fix the limitation of LINE_MAX

hwdb.d/60-keyboard.hwdb

@@ -586,6 +586,8 @@ evdev:atkbd:dmi:bvn*:bvr*:bd*:svnHewlett-Packard*:pnHDX9494NR:pvr*
 
 # HP EliteBook 725 G2
 evdev:atkbd:dmi:bvn*:bvr*:bd*:svnHewlett-Packard*:pnHPLicrice:pvr*
+# HP EliteBook 840 G1
+evdev:atkbd:dmi:bvn*:bvr*:bd*:svnHewlett-Packard*:pnHPEliteBook840G1:pvr*
 # HP ProBook 440 G2
 evdev:atkbd:dmi:bvn*:bvr*:bd*:svnHewlett-Packard*:pnHP440G2:pvr*
 # several HP ProBooks 4xx

man/sd-daemon.xml

@@ -70,10 +70,9 @@
 
     <para>These prefixes are intended to be used in conjunction with stderr-based logging (or stdout-based
     logging) as implemented by systemd. If a systemd service definition file is configured with
-    <varname>StandardError=journal</varname>, <varname>StandardError=syslog</varname> or
-    <varname>StandardError=kmsg</varname> (and similar with <varname>StandardOutput=</varname>), these
-    prefixes can be used to encode a log level in lines printed. This is similar to the kernel
-    <function>printk()</function>-style logging. See
+    <varname>StandardError=journal</varname> or <varname>StandardError=kmsg</varname> (and similar with
+    <varname>StandardOutput=</varname>), these prefixes can be used to encode a log level in lines
+    printed. This is similar to the kernel <function>printk()</function>-style logging. See
     <citerefentry><refentrytitle>klogctl</refentrytitle><manvolnum>2</manvolnum></citerefentry> for more
     information.</para>
 

man/systemd.exec.xml

@@ -66,9 +66,9 @@
       will also gain an automatic <varname>After=</varname> dependency on
       <citerefentry><refentrytitle>systemd-tmpfiles-setup.service</refentrytitle><manvolnum>8</manvolnum></citerefentry>.</para></listitem>
 
-      <listitem><para>Units whose standard output or error output is connected to <option>journal</option>,
-      <option>syslog</option> or <option>kmsg</option> (or their combinations with console output, see below)
-      automatically acquire dependencies of type <varname>After=</varname> on
+      <listitem><para>Units whose standard output or error output is connected to <option>journal</option> or
+      <option>kmsg</option> (or their combinations with console output, see below) automatically acquire
+      dependencies of type <varname>After=</varname> on
       <filename>systemd-journald.socket</filename>.</para></listitem>
 
       <listitem><para>Units using <varname>LogNamespace=</varname> will automatically gain ordering and

src/core/execute.c

@@ -219,17 +219,10 @@ static bool is_terminal_input(ExecInput i) {
 static bool is_terminal_output(ExecOutput o) {
         return IN_SET(o,
                       EXEC_OUTPUT_TTY,
-                      EXEC_OUTPUT_SYSLOG_AND_CONSOLE,
                       EXEC_OUTPUT_KMSG_AND_CONSOLE,
                       EXEC_OUTPUT_JOURNAL_AND_CONSOLE);
 }
 
-static bool is_syslog_output(ExecOutput o) {
-        return IN_SET(o,
-                      EXEC_OUTPUT_SYSLOG,
-                      EXEC_OUTPUT_SYSLOG_AND_CONSOLE);
-}
-
 static bool is_kmsg_output(ExecOutput o) {
         return IN_SET(o,
                       EXEC_OUTPUT_KMSG,
@@ -361,7 +354,7 @@ static int connect_logger_as(
                 params->flags & EXEC_PASS_LOG_UNIT ? unit->id : "",
                 context->syslog_priority,
                 !!context->syslog_level_prefix,
-                is_syslog_output(output),
+                false,
                 is_kmsg_output(output),
                 is_terminal_output(output)) < 0)
                 return -errno;
@@ -664,8 +657,6 @@ static int setup_output(
                 /* We don't reset the terminal if this is just about output */
                 return open_terminal_as(exec_context_tty_path(context), O_WRONLY, fileno);
 
-        case EXEC_OUTPUT_SYSLOG:
-        case EXEC_OUTPUT_SYSLOG_AND_CONSOLE:
         case EXEC_OUTPUT_KMSG:
         case EXEC_OUTPUT_KMSG_AND_CONSOLE:
         case EXEC_OUTPUT_JOURNAL:
@@ -4736,17 +4727,13 @@ void exec_context_dump(const ExecContext *c, FILE* f, const char *prefix) {
                         prefix, yes_no(c->tty_vt_disallocate));
 
         if (IN_SET(c->std_output,
-                   EXEC_OUTPUT_SYSLOG,
                    EXEC_OUTPUT_KMSG,
                    EXEC_OUTPUT_JOURNAL,
-                   EXEC_OUTPUT_SYSLOG_AND_CONSOLE,
                    EXEC_OUTPUT_KMSG_AND_CONSOLE,
                    EXEC_OUTPUT_JOURNAL_AND_CONSOLE) ||
             IN_SET(c->std_error,
-                   EXEC_OUTPUT_SYSLOG,
                    EXEC_OUTPUT_KMSG,
                    EXEC_OUTPUT_JOURNAL,
-                   EXEC_OUTPUT_SYSLOG_AND_CONSOLE,
                    EXEC_OUTPUT_KMSG_AND_CONSOLE,
                    EXEC_OUTPUT_JOURNAL_AND_CONSOLE)) {
 
@@ -5764,8 +5751,6 @@ static const char* const exec_output_table[_EXEC_OUTPUT_MAX] = {
         [EXEC_OUTPUT_INHERIT] = "inherit",
         [EXEC_OUTPUT_NULL] = "null",
         [EXEC_OUTPUT_TTY] = "tty",
-        [EXEC_OUTPUT_SYSLOG] = "syslog",
-        [EXEC_OUTPUT_SYSLOG_AND_CONSOLE] = "syslog+console",
         [EXEC_OUTPUT_KMSG] = "kmsg",
         [EXEC_OUTPUT_KMSG_AND_CONSOLE] = "kmsg+console",
         [EXEC_OUTPUT_JOURNAL] = "journal",

src/core/execute.h

@@ -52,8 +52,6 @@ typedef enum ExecOutput {
         EXEC_OUTPUT_INHERIT,
         EXEC_OUTPUT_NULL,
         EXEC_OUTPUT_TTY,
-        EXEC_OUTPUT_SYSLOG,
-        EXEC_OUTPUT_SYSLOG_AND_CONSOLE,
         EXEC_OUTPUT_KMSG,
         EXEC_OUTPUT_KMSG_AND_CONSOLE,
         EXEC_OUTPUT_JOURNAL,

src/core/load-fragment.c

@@ -1099,6 +1099,7 @@ int config_parse_exec_output(
         const char *n;
         ExecContext *c = data;
         const Unit *u = userdata;
+        bool obsolete = false;
         ExecOutput eo;
         int r;
 
@@ -1123,6 +1124,14 @@ int config_parse_exec_output(
 
                 eo = EXEC_OUTPUT_NAMED_FD;
 
+        } else if (streq(rvalue, "syslog")) {
+                eo = EXEC_OUTPUT_JOURNAL;
+                obsolete = true;
+
+        } else if (streq(rvalue, "syslog+console")) {
+                eo = EXEC_OUTPUT_JOURNAL_AND_CONSOLE;
+                obsolete = true;
+
         } else if ((n = startswith(rvalue, "file:"))) {
 
                 r = unit_full_printf(u, n, &resolved);
@@ -1154,6 +1163,11 @@ int config_parse_exec_output(
                 }
         }
 
+        if (obsolete)
+                log_syntax(unit, LOG_NOTICE, filename, line, 0,
+                           "Standard output type %s is obsolete, automatically updating to %s. Please update your unit file, and consider removing the setting altogether.",
+                           rvalue, exec_output_to_string(eo));
+
         if (streq(lvalue, "StandardOutput")) {
                 if (eo == EXEC_OUTPUT_NAMED_FD)
                         free_and_replace(c->stdio_fdname[STDOUT_FILENO], resolved);
@@ -5051,22 +5065,36 @@ int config_parse_output_restricted(
                 void *userdata) {
 
         ExecOutput t, *eo = data;
+        bool obsolete = false;
 
         assert(filename);
         assert(lvalue);
         assert(rvalue);
         assert(data);
 
-        t = exec_output_from_string(rvalue);
-        if (t < 0) {
-                log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to parse output type, ignoring: %s", rvalue);
-                return 0;
+        if (streq(rvalue, "syslog")) {
+                t = EXEC_OUTPUT_JOURNAL;
+                obsolete = true;
+        } else if (streq(rvalue, "syslog+console")) {
+                t = EXEC_OUTPUT_JOURNAL_AND_CONSOLE;
+                obsolete = true;
+        } else {
+                t = exec_output_from_string(rvalue);
+                if (t < 0) {
+                        log_syntax(unit, LOG_ERR, filename, line, 0, "Failed to parse output type, ignoring: %s", rvalue);
+                        return 0;
+                }
+
+                if (IN_SET(t, EXEC_OUTPUT_SOCKET, EXEC_OUTPUT_NAMED_FD, EXEC_OUTPUT_FILE, EXEC_OUTPUT_FILE_APPEND)) {
+                        log_syntax(unit, LOG_ERR, filename, line, 0, "Standard output types socket, fd:, file:, append: are not supported as defaults, ignoring: %s", rvalue);
+                        return 0;
+                }
         }
 
-        if (IN_SET(t, EXEC_OUTPUT_SOCKET, EXEC_OUTPUT_NAMED_FD, EXEC_OUTPUT_FILE, EXEC_OUTPUT_FILE_APPEND)) {
-                log_syntax(unit, LOG_ERR, filename, line, 0, "Standard output types socket, fd:, file:, append: are not supported as defaults, ignoring: %s", rvalue);
-                return 0;
-        }
+        if (obsolete)
+                log_syntax(unit, LOG_NOTICE, filename, line, 0,
+                           "Standard output type %s is obsolete, automatically updating to %s. Please update your configuration.",
+                           rvalue, exec_output_to_string(t));
 
         *eo = t;
         return 0;

src/core/unit.c

@@ -1123,12 +1123,10 @@ int unit_add_exec_dependencies(Unit *u, ExecContext *c) {
 
         if (!IN_SET(c->std_output,
                     EXEC_OUTPUT_JOURNAL, EXEC_OUTPUT_JOURNAL_AND_CONSOLE,
-                    EXEC_OUTPUT_KMSG, EXEC_OUTPUT_KMSG_AND_CONSOLE,
-                    EXEC_OUTPUT_SYSLOG, EXEC_OUTPUT_SYSLOG_AND_CONSOLE) &&
+                    EXEC_OUTPUT_KMSG, EXEC_OUTPUT_KMSG_AND_CONSOLE) &&
             !IN_SET(c->std_error,
                     EXEC_OUTPUT_JOURNAL, EXEC_OUTPUT_JOURNAL_AND_CONSOLE,
-                    EXEC_OUTPUT_KMSG, EXEC_OUTPUT_KMSG_AND_CONSOLE,
-                    EXEC_OUTPUT_SYSLOG, EXEC_OUTPUT_SYSLOG_AND_CONSOLE) &&
+                    EXEC_OUTPUT_KMSG, EXEC_OUTPUT_KMSG_AND_CONSOLE) &&
             !c->log_namespace)
                 return 0;
 

src/journal/journal-send.c

@@ -15,6 +15,7 @@
 #include "errno-util.h"
 #include "fd-util.h"
 #include "io-util.h"
+#include "fileio.h"
 #include "memfd-util.h"
 #include "socket-util.h"
 #include "stdio-util.h"
@@ -73,12 +74,12 @@ _public_ int sd_journal_print(int priority, const char *format, ...) {
 }
 
 _public_ int sd_journal_printv(int priority, const char *format, va_list ap) {
-
-        /* FIXME: Instead of limiting things to LINE_MAX we could do a
-           C99 variable-length array on the stack here in a loop. */
-
-        char buffer[8 + LINE_MAX], p[STRLEN("PRIORITY=") + DECIMAL_STR_MAX(int) + 1];
+        char p[STRLEN("PRIORITY=") + DECIMAL_STR_MAX(int) + 1];
+        char sbuf[LINE_MAX + 8] = "MESSAGE=";
         struct iovec iov[2];
+        int len;
+        va_list aq;
+        char *buffer = sbuf;
 
         assert_return(priority >= 0, -EINVAL);
         assert_return(priority <= 7, -EINVAL);
@@ -86,14 +87,27 @@ _public_ int sd_journal_printv(int priority, const char *format, va_list ap) {
 
         xsprintf(p, "PRIORITY=%i", priority & LOG_PRIMASK);
 
-        memcpy(buffer, "MESSAGE=", 8);
-        vsnprintf(buffer+8, sizeof(buffer) - 8, format, ap);
+        va_copy(aq, ap);
+        len = vsnprintf(buffer + 8, LINE_MAX, format, aq);
+        va_end(aq);
+
+        if (len >= (int)LONG_LINE_MAX - 8)
+                return -ENOBUFS;
+
+        /* Allocate large buffer to accomodate big message */
+        if (len >= LINE_MAX) {
+                int rlen;
+                buffer = alloca(len + 9);
+                memcpy(buffer, "MESSAGE=", 8);
+                rlen = vsnprintf(buffer + 8, len + 1, format, ap);
+                assert(len == rlen);
+        }
 
         /* Strip trailing whitespace, keep prefix whitespace. */
         (void) strstrip(buffer);
 
         /* Suppress empty lines */
-        if (isempty(buffer+8))
+        if (isempty(buffer + 8))
                 return 0;
 
         iov[0] = IOVEC_MAKE_STRING(buffer);
@@ -437,9 +451,13 @@ _public_ int sd_journal_print_with_location(int priority, const char *file, cons
 }
 
 _public_ int sd_journal_printv_with_location(int priority, const char *file, const char *line, const char *func, const char *format, va_list ap) {
-        char buffer[8 + LINE_MAX], p[STRLEN("PRIORITY=") + DECIMAL_STR_MAX(int) + 1];
+        char p[STRLEN("PRIORITY=") + DECIMAL_STR_MAX(int) + 1];
+        char sbuf[LINE_MAX + 8] = "MESSAGE=";
         struct iovec iov[5];
         char *f;
+        int len;
+        char *buffer = sbuf;
+        va_list aq;
 
         assert_return(priority >= 0, -EINVAL);
         assert_return(priority <= 7, -EINVAL);
@@ -447,14 +465,27 @@ _public_ int sd_journal_printv_with_location(int priority, const char *file, con
 
         xsprintf(p, "PRIORITY=%i", priority & LOG_PRIMASK);
 
-        memcpy(buffer, "MESSAGE=", 8);
-        vsnprintf(buffer+8, sizeof(buffer) - 8, format, ap);
+        va_copy(aq, ap);
+        len = vsnprintf(buffer + 8, LINE_MAX, format, aq);
+        va_end(aq);
+
+        if (len >= (int)LONG_LINE_MAX - 8)
+                return -ENOBUFS;
+
+        /* Allocate large buffer to accomodate big message */
+        if (len >= LINE_MAX) {
+                int rlen;
+                buffer = alloca(len + 9);
+                memcpy(buffer, "MESSAGE=", 8);
+                rlen = vsnprintf(buffer + 8, len + 1, format, ap);
+                assert(len == rlen);
+        }
 
         /* Strip trailing whitespace, keep prefixing whitespace */
         (void) strstrip(buffer);
 
         /* Suppress empty lines */
-        if (isempty(buffer+8))
+        if (isempty(buffer + 8))
                 return 0;
 
         /* func is initialized from __func__ which is not a macro, but

src/journal/test-journal-send.c

@@ -5,11 +5,19 @@
 #include <unistd.h>
 
 #include "sd-journal.h"
-
+#include "fileio.h"
 #include "macro.h"
 #include "memory-util.h"
 
-int main(int argc, char *argv[]) {
+static void test_journal_print(void) {
+        assert_se(sd_journal_print(LOG_INFO, "XXX") == 0);
+        assert_se(sd_journal_print(LOG_INFO, "%s", "YYY") == 0);
+        assert_se(sd_journal_print(LOG_INFO, "X%4094sY", "ZZZ") == 0);
+        assert_se(sd_journal_print(LOG_INFO, "X%*sY", LONG_LINE_MAX - 8 - 3, "ZZZ") == 0);
+        assert_se(sd_journal_print(LOG_INFO, "X%*sY", LONG_LINE_MAX - 8 - 2, "ZZZ") == -ENOBUFS);
+}
+
+static void test_journal_send(void) {
         _cleanup_free_ char *huge = NULL;
 
 #define HUGE_SIZE (4096*1024)
@@ -82,7 +90,13 @@ int main(int argc, char *argv[]) {
         assert_se(sd_journal_sendv(graph2, 1) == 0);
         assert_se(sd_journal_sendv(message1, 1) == 0);
         assert_se(sd_journal_sendv(message2, 1) == 0);
+}
 
+int main(int argc, char *argv[]) {
+        test_journal_print();
+        test_journal_send();
+
+        /* Sleep a bit to make it easy for journald to collect metadata. */
         sleep(1);
 
         return 0;