packages/systemd
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

!105 journald: enforce longer line length limit during "setup" phase of stream protocol

Browse Source 
From: @overweight
Reviewed-by: @openeuler-basic
Signed-off-by: @openeuler-basic
This commit is contained in:
openeuler-ci-bot 2021-05-31 21:30:39 +08:00 committed by Gitee
commit 0cd9615637
5 changed files with 573 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

148
0081-journal-refresh-cached-credentials-of-stdout-streams.patch Normal file
View File

@ -0,0 +1,148 @@
From 09d0b46ab61bebafe5bdc1be95ee153dfb13d6bc Mon Sep 17 00:00:00 2001
From: Lorenz Bauer <lmb@cloudflare.com>
Date: Mon, 4 Nov 2019 16:35:46 +0000
Subject: [PATCH] journal: refresh cached credentials of stdout streams
journald assumes that getsockopt(SO_PEERCRED) correctly identifies the
process on the remote end of the socket. However, this is incorrect
according to man 7 socket:
The returned credentials are those that were in effect at the
time of the call to connect(2) or socketpair(2).
This becomes a problem when a new process inherits the stdout stream
from a parent. First, log messages from the child process will
be attributed to the parent. Second, the struct ucred used by journald
becomes invalid as soon as the parent exits. Further sendmsg calls then
fail with ENOENT. Logs for the child process then vanish from the journal.
Fix this by using recvmsg on the stdout stream, and refreshing the cached
struct ucred if SCM_CREDENTIALS indicate a new process.
Fixes #13708
---
src/journal/journald-stream.c | 49 ++++++++++++++++++++++++++++++++++--
test/TEST-04-JOURNAL/test-journal.sh | 16 ++++++++++++
2 files changed, 63 insertions(+), 2 deletions(-)
diff --git a/src/journal/journald-stream.c b/src/journal/journald-stream.c
index afebade..22a70ce 100644
--- a/src/journal/journald-stream.c
+++ b/src/journal/journald-stream.c
@@ -487,11 +487,22 @@ static int stdout_stream_scan(StdoutStream *s, bool force_flush) {
}
static int stdout_stream_process(sd_event_source *es, int fd, uint32_t revents, void *userdata) {
+ uint8_t buf[CMSG_SPACE(sizeof(struct ucred))];
StdoutStream *s = userdata;
+ struct ucred *ucred = NULL;
+ struct cmsghdr *cmsg;
+ struct iovec iovec;
size_t limit;
ssize_t l;
int r;
+ struct msghdr msghdr = {
+ .msg_iov = &iovec,
+ .msg_iovlen = 1,
+ .msg_control = buf,
+ .msg_controllen = sizeof(buf),
+ };
+
assert(s);
if ((revents|EPOLLIN|EPOLLHUP) != (EPOLLIN|EPOLLHUP)) {
@@ -511,20 +522,50 @@ static int stdout_stream_process(sd_event_source *es, int fd, uint32_t revents,
* always leave room for a terminating NUL we might need to add. */
limit = MIN(s->allocated - 1, s->server->line_max);
- l = read(s->fd, s->buffer + s->length, limit - s->length);
+ iovec = IOVEC_MAKE(s->buffer + s->length, limit - s->length);
+
+ l = recvmsg(s->fd, &msghdr, MSG_DONTWAIT|MSG_CMSG_CLOEXEC);
if (l < 0) {
- if (errno == EAGAIN)
+ if (IN_SET(errno, EINTR, EAGAIN))
return 0;
log_warning_errno(errno, "Failed to read from stream: %m");
goto terminate;
}
+ cmsg_close_all(&msghdr);
if (l == 0) {
stdout_stream_scan(s, true);
goto terminate;
}
+ CMSG_FOREACH(cmsg, &msghdr)
+ if (cmsg->cmsg_level == SOL_SOCKET &&
+ cmsg->cmsg_type == SCM_CREDENTIALS &&
+ cmsg->cmsg_len == CMSG_LEN(sizeof(struct ucred))) {
+ ucred = (struct ucred *)CMSG_DATA(cmsg);
+ break;
+ }
+
+ /* Invalidate the context if the pid of the sender changed.
+ * This happens when a forked process inherits stdout / stderr
+ * from a parent. In this case getpeercred returns the ucred
+ * of the parent, which can be invalid if the parent has exited
+ * in the meantime.
+ */
+ if (ucred && ucred->pid != s->ucred.pid) {
+ /* force out any previously half-written lines from a
+ * different process, before we switch to the new ucred
+ * structure for everything we just added */
+ r = stdout_stream_scan(s, true);
+ if (r < 0)
+ goto terminate;
+
+ s->ucred = *ucred;
+ client_context_release(s->server, s->context);
+ s->context = NULL;
+ }
+
s->length += l;
r = stdout_stream_scan(s, false);
if (r < 0)
@@ -562,6 +603,10 @@ int stdout_stream_install(Server *s, int fd, StdoutStream **ret) {
if (r < 0)
return log_error_errno(r, "Failed to determine peer credentials: %m");
+ r = setsockopt_int(fd, SOL_SOCKET, SO_PASSCRED, true);
+ if (r < 0)
+ return log_error_errno(r, "SO_PASSCRED failed: %m");
+
if (mac_selinux_use()) {
r = getpeersec(fd, &stream->label);
if (r < 0 && r != -EOPNOTSUPP)
diff --git a/test/TEST-04-JOURNAL/test-journal.sh b/test/TEST-04-JOURNAL/test-journal.sh
index 4e539aa..de27eb0 100755
--- a/test/TEST-04-JOURNAL/test-journal.sh
+++ b/test/TEST-04-JOURNAL/test-journal.sh
@@ -74,6 +74,22 @@ cmp /expected /output
{ journalctl -ball -b -m 2>&1 || :; } | head -1 > /output
cmp /expected /output
+# https://github.com/systemd/systemd/issues/13708
+ID=$(systemd-id128 new)
+systemd-cat -t "$ID" bash -c 'echo parent; (echo child) & wait' &
+PID=$!
+wait %%
+journalctl --sync
+# We can drop this grep when https://github.com/systemd/systemd/issues/13937
+# has a fix.
+journalctl -b -o export -t "$ID" --output-fields=_PID | grep '^_PID=' >/output
+[[ `grep -c . /output` -eq 2 ]]
+grep -q "^_PID=$PID" /output
+grep -vq "^_PID=$PID" /output
+
+# Add new tests before here, the journald restarts below
+# may make tests flappy.
+
# Don't lose streams on restart
systemctl start forever-print-hola
sleep 3
--
1.8.3.1

77
0082-journald-rework-end-of-line-marker-handling-to-use-a.patch Normal file
View File

@ -0,0 +1,77 @@
From 549b7379ba404c33fd448d2bca46a57f6529b00b Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 12 May 2020 18:53:35 +0200
Subject: [PATCH] journald: rework end of line marker handling to use a field
table
---
src/journal/journald-stream.c | 29 ++++++++++++++++++++---------
1 file changed, 20 insertions(+), 9 deletions(-)
diff --git a/src/journal/journald-stream.c b/src/journal/journald-stream.c
index 22a70ce..b86ed78 100644
--- a/src/journal/journald-stream.c
+++ b/src/journal/journald-stream.c
@@ -57,6 +57,8 @@ typedef enum LineBreak {
LINE_BREAK_NUL,
LINE_BREAK_LINE_MAX,
LINE_BREAK_EOF,
+ _LINE_BREAK_MAX,
+ _LINE_BREAK_INVALID = -1,
} LineBreak;
struct StdoutStream {
@@ -236,7 +238,11 @@ fail:
return log_error_errno(r, "Failed to save stream data %s: %m", s->state_file);
}
-static int stdout_stream_log(StdoutStream *s, const char *p, LineBreak line_break) {
+static int stdout_stream_log(
+ StdoutStream *s,
+ const char *p,
+ LineBreak line_break) {
+
struct iovec *iovec;
int priority;
char syslog_priority[] = "PRIORITY=\0";
@@ -248,6 +254,9 @@ static int stdout_stream_log(StdoutStream *s, const char *p, LineBreak line_brea
assert(s);
assert(p);
+ assert(line_break >= 0);
+ assert(line_break < _LINE_BREAK_MAX);
+
if (s->context)
(void) client_context_maybe_refresh(s->server, s->context, NULL, NULL, 0, NULL, USEC_INFINITY);
else if (pid_is_valid(s->ucred.pid)) {
@@ -299,17 +308,19 @@ static int stdout_stream_log(StdoutStream *s, const char *p, LineBreak line_brea
iovec[n++] = IOVEC_MAKE_STRING(syslog_identifier);
}
- if (line_break != LINE_BREAK_NEWLINE) {
- const char *c;
+ static const char * const line_break_field_table[_LINE_BREAK_MAX] = {
+ [LINE_BREAK_NEWLINE] = NULL, /* Do not add field if traditional newline */
+ [LINE_BREAK_NUL] = "_LINE_BREAK=nul",
+ [LINE_BREAK_LINE_MAX] = "_LINE_BREAK=line-max",
+ [LINE_BREAK_EOF] = "_LINE_BREAK=eof",
+ };
- /* If this log message was generated due to an uncommon line break then mention this in the log
- * entry */
+ const char *c = line_break_field_table[line_break];
- c = line_break == LINE_BREAK_NUL ? "_LINE_BREAK=nul" :
- line_break == LINE_BREAK_LINE_MAX ? "_LINE_BREAK=line-max" :
- "_LINE_BREAK=eof";
+ /* If this log message was generated due to an uncommon line break then mention this in the log
+ * entry */
+ if (c)
iovec[n++] = IOVEC_MAKE_STRING(c);
- }
message = strjoin("MESSAGE=", p);
if (message)
--
1.8.3.1

230
0083-journald-rework-pid-change-handling.patch Normal file
View File

@ -0,0 +1,230 @@
From 45ba1ea5e9264d385fa565328fe957ef1d78caa1 Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 12 May 2020 18:56:34 +0200
Subject: [PATCH] journald: rework pid change handling
Let's introduce an explicit line ending marker for line endings due to
pid change.
Let's also make sure we don't get confused with buffer management.
Fixes: #15654
---
src/journal/journald-stream.c | 108 +++++++++++++++++++++++++++---------------
1 file changed, 69 insertions(+), 39 deletions(-)
diff --git a/src/journal/journald-stream.c b/src/journal/journald-stream.c
index b86ed78..3219b14 100644
--- a/src/journal/journald-stream.c
+++ b/src/journal/journald-stream.c
@@ -57,6 +57,7 @@ typedef enum LineBreak {
LINE_BREAK_NUL,
LINE_BREAK_LINE_MAX,
LINE_BREAK_EOF,
+ LINE_BREAK_PID_CHANGE,
_LINE_BREAK_MAX,
_LINE_BREAK_INVALID = -1,
} LineBreak;
@@ -313,6 +314,7 @@ static int stdout_stream_log(
[LINE_BREAK_NUL] = "_LINE_BREAK=nul",
[LINE_BREAK_LINE_MAX] = "_LINE_BREAK=line-max",
[LINE_BREAK_EOF] = "_LINE_BREAK=eof",
+ [LINE_BREAK_PID_CHANGE] = "_LINE_BREAK=pid-change",
};
const char *c = line_break_field_table[line_break];
@@ -434,21 +436,43 @@ static int stdout_stream_line(StdoutStream *s, char *p, LineBreak line_break) {
assert_not_reached("Unknown stream state");
}
-static int stdout_stream_scan(StdoutStream *s, bool force_flush) {
- char *p;
- size_t remaining;
+static int stdout_stream_found(
+ StdoutStream *s,
+ char *p,
+ size_t l,
+ LineBreak line_break) {
+
+ char saved;
int r;
assert(s);
+ assert(p);
+
+ /* Let's NUL terminate the specified buffer for this call, and revert back afterwards */
+ saved = p[l];
+ p[l] = 0;
+ r = stdout_stream_line(s, p, line_break);
+ p[l] = saved;
- p = s->buffer;
- remaining = s->length;
+ return r;
+}
+
+static int stdout_stream_scan(
+ StdoutStream *s,
+ char *p,
+ size_t remaining,
+ LineBreak force_flush,
+ size_t *ret_consumed) {
- /* XXX: This function does nothing if (s->length == 0) */
+ size_t consumed = 0;
+ int r;
+
+ assert(s);
+ assert(p);
for (;;) {
LineBreak line_break;
- size_t skip;
+ size_t skip, found;
char *end1, *end2;
end1 = memchr(p, '\n', remaining);
@@ -456,43 +480,40 @@ static int stdout_stream_scan(StdoutStream *s, bool force_flush) {
if (end2) {
/* We found a NUL terminator */
- skip = end2 - p + 1;
+ found = end2 - p;
+ skip = found + 1;
line_break = LINE_BREAK_NUL;
} else if (end1) {
/* We found a \n terminator */
- *end1 = 0;
- skip = end1 - p + 1;
+ found = end1 - p;
+ skip = found + 1;
line_break = LINE_BREAK_NEWLINE;
} else if (remaining >= s->server->line_max) {
/* Force a line break after the maximum line length */
- *(p + s->server->line_max) = 0;
- skip = remaining;
+ found = skip = s->server->line_max;
line_break = LINE_BREAK_LINE_MAX;
} else
break;
- r = stdout_stream_line(s, p, line_break);
+ r = stdout_stream_found(s, p, found, line_break);
if (r < 0)
return r;
- remaining -= skip;
p += skip;
+ consumed += skip;
+ remaining -= skip;
}
- if (force_flush && remaining > 0) {
- p[remaining] = 0;
- r = stdout_stream_line(s, p, LINE_BREAK_EOF);
+ if (force_flush >= 0 && remaining > 0) {
+ r = stdout_stream_found(s, p, remaining, force_flush);
if (r < 0)
return r;
- p += remaining;
- remaining = 0;
+ consumed += remaining;
}
- if (p > s->buffer) {
- memmove(s->buffer, p, remaining);
- s->length = remaining;
- }
+ if (ret_consumed)
+ *ret_consumed = consumed;
return 0;
}
@@ -500,11 +521,12 @@ static int stdout_stream_scan(StdoutStream *s, bool force_flush) {
static int stdout_stream_process(sd_event_source *es, int fd, uint32_t revents, void *userdata) {
uint8_t buf[CMSG_SPACE(sizeof(struct ucred))];
StdoutStream *s = userdata;
+ size_t limit, consumed;
struct ucred *ucred = NULL;
struct cmsghdr *cmsg;
struct iovec iovec;
- size_t limit;
ssize_t l;
+ char *p;
int r;
struct msghdr msghdr = {
@@ -532,7 +554,7 @@ static int stdout_stream_process(sd_event_source *es, int fd, uint32_t revents,
/* Try to make use of the allocated buffer in full, but never read more than the configured line size. Also,
* always leave room for a terminating NUL we might need to add. */
limit = MIN(s->allocated - 1, s->server->line_max);
-
+ assert(s->length <= limit);
iovec = IOVEC_MAKE(s->buffer + s->length, limit - s->length);
l = recvmsg(s->fd, &msghdr, MSG_DONTWAIT|MSG_CMSG_CLOEXEC);
@@ -546,7 +568,7 @@ static int stdout_stream_process(sd_event_source *es, int fd, uint32_t revents,
cmsg_close_all(&msghdr);
if (l == 0) {
- stdout_stream_scan(s, true);
+ (void) stdout_stream_scan(s, s->buffer, s->length, /* force_flush = */ LINE_BREAK_EOF, NULL);
goto terminate;
}
@@ -558,30 +580,38 @@ static int stdout_stream_process(sd_event_source *es, int fd, uint32_t revents,
break;
}
- /* Invalidate the context if the pid of the sender changed.
- * This happens when a forked process inherits stdout / stderr
- * from a parent. In this case getpeercred returns the ucred
- * of the parent, which can be invalid if the parent has exited
- * in the meantime.
- */
+ /* Invalidate the context if the PID of the sender changed. This happens when a forked process
+ * inherits stdout/stderr from a parent. In this case getpeercred() returns the ucred of the parent,
+ * which can be invalid if the parent has exited in the meantime. */
if (ucred && ucred->pid != s->ucred.pid) {
- /* force out any previously half-written lines from a
+ /* Force out any previously half-written lines from a
* different process, before we switch to the new ucred
* structure for everything we just added */
- r = stdout_stream_scan(s, true);
+ r = stdout_stream_scan(s, s->buffer, s->length, /* force_flush = */ LINE_BREAK_PID_CHANGE, NULL);
if (r < 0)
goto terminate;
- s->ucred = *ucred;
- client_context_release(s->server, s->context);
- s->context = NULL;
+ s->context = client_context_release(s->server, s->context);
+
+ p = s->buffer + s->length;
+ } else {
+ p = s->buffer;
+ l += s->length;
}
- s->length += l;
- r = stdout_stream_scan(s, false);
+ /* Always copy in the new credentials */
+ if (ucred)
+ s->ucred = *ucred;
+
+ r = stdout_stream_scan(s, p, l, _LINE_BREAK_INVALID, &consumed);
if (r < 0)
goto terminate;
+ /* Move what wasn't consumed to the front of the buffer */
+ assert(consumed <= (size_t) l);
+ s->length = l - consumed;
+ memmove(s->buffer, p + consumed, s->length);
+
return 1;
terminate:
--
1.8.3.1

104
0084-journald-enforce-longer-line-length-limit-during-set.patch Normal file
View File

@ -0,0 +1,104 @@
From 4e071b5240a29842bc8acd0d7eb0b797f2812b8b Mon Sep 17 00:00:00 2001
From: rpm-build <rpm-build>
Date: Fri, 21 May 2021 17:55:38 +0800
Subject: [PATCH] change
---
src/journal/journald-stream.c | 35 ++++++++++++++++++++++++++++-------
1 file changed, 28 insertions(+), 7 deletions(-)
diff --git a/src/journal/journald-stream.c b/src/journal/journald-stream.c
index 3219b14..fda75fb 100644
--- a/src/journal/journald-stream.c
+++ b/src/journal/journald-stream.c
@@ -38,6 +38,12 @@
#define STDOUT_STREAMS_MAX 4096
+/* During the "setup" protocol phase of the stream logic let's define a different maximum line length than
+ * during the actual operational phase. We want to allow users to specify very short line lengths after all,
+ * but the unit name we embed in the setup protocol might be longer than that. Hence, during the setup phase
+ * let's enforce a line length matching the maximum unit name length (255) */
+#define STDOUT_STREAM_SETUP_PROTOCOL_LINE_MAX (UNIT_NAME_MAX-1U)
+
typedef enum StdoutStreamState {
STDOUT_STREAM_IDENTIFIER,
STDOUT_STREAM_UNIT_ID,
@@ -46,7 +52,7 @@ typedef enum StdoutStreamState {
STDOUT_STREAM_FORWARD_TO_SYSLOG,
STDOUT_STREAM_FORWARD_TO_KMSG,
STDOUT_STREAM_FORWARD_TO_CONSOLE,
- STDOUT_STREAM_RUNNING
+ STDOUT_STREAM_RUNNING,
} StdoutStreamState;
/* The different types of log record terminators: a real \n was read, a NUL character was read, the maximum line length
@@ -457,6 +463,18 @@ static int stdout_stream_found(
return r;
}
+static size_t stdout_stream_line_max(StdoutStream *s) {
+ assert(s);
+
+ /* During the "setup" phase of our protocol, let's ensure we use a line length where a full unit name
+ * can fit in */
+ if (s->state != STDOUT_STREAM_RUNNING)
+ return STDOUT_STREAM_SETUP_PROTOCOL_LINE_MAX;
+
+ /* After the protocol's "setup" phase is complete, let's use whatever the user configured */
+ return s->server->line_max;
+}
+
static int stdout_stream_scan(
StdoutStream *s,
char *p,
@@ -464,19 +482,22 @@ static int stdout_stream_scan(
LineBreak force_flush,
size_t *ret_consumed) {
- size_t consumed = 0;
+ size_t consumed = 0, line_max;
int r;
assert(s);
assert(p);
+ line_max = stdout_stream_line_max(s);
+
for (;;) {
LineBreak line_break;
size_t skip, found;
char *end1, *end2;
+ size_t tmp_remaining = MIN(remaining, line_max);
- end1 = memchr(p, '\n', remaining);
- end2 = memchr(p, 0, end1 ? (size_t) (end1 - p) : remaining);
+ end1 = memchr(p, '\n', tmp_remaining);
+ end2 = memchr(p, 0, end1 ? (size_t) (end1 - p) : tmp_remaining);
if (end2) {
/* We found a NUL terminator */
@@ -488,9 +509,9 @@ static int stdout_stream_scan(
found = end1 - p;
skip = found + 1;
line_break = LINE_BREAK_NEWLINE;
- } else if (remaining >= s->server->line_max) {
+ } else if (remaining >= line_max) {
/* Force a line break after the maximum line length */
- found = skip = s->server->line_max;
+ found = skip = line_max;
line_break = LINE_BREAK_LINE_MAX;
} else
break;
@@ -553,7 +574,7 @@ static int stdout_stream_process(sd_event_source *es, int fd, uint32_t revents,
/* Try to make use of the allocated buffer in full, but never read more than the configured line size. Also,
* always leave room for a terminating NUL we might need to add. */
- limit = MIN(s->allocated - 1, s->server->line_max);
+ limit = MIN(s->allocated - 1, MAX(s->server->line_max, STDOUT_STREAM_SETUP_PROTOCOL_LINE_MAX));
assert(s->length <= limit);
iovec = IOVEC_MAKE(s->buffer + s->length, limit - s->length);
--
1.8.3.1

20
systemd.spec
View File

@ -16,7 +16,7 @@
Name: systemd
Url: https://www.freedesktop.org/wiki/Software/systemd
Version: 243
Release: 36
Release: 37
License: MIT and LGPLv2+ and GPLv2+
Summary: System and Service Manager
@ -125,6 +125,11 @@ Patch0078: 0078-backport-varlink-make-userdata-pointer-inheritance-from-var
Patch0079: 0079-backport-udev-net_id-parse-_SUN-ACPI-index-as-a-signed-intege.patch
Patch0080: 0080-backport-udev-net_id-don-t-generate-slot-based-names-if-multi.patch
Patch0081: 0081-journal-refresh-cached-credentials-of-stdout-streams.patch
Patch0082: 0082-journald-rework-end-of-line-marker-handling-to-use-a.patch
Patch0083: 0083-journald-rework-pid-change-handling.patch
Patch0084: 0084-journald-enforce-longer-line-length-limit-during-set.patch
#openEuler
Patch9002: 1509-fix-journal-file-descriptors-leak-problems.patch
Patch9003: 1602-activation-service-must-be-restarted-when-reactivated.patch
@ -1509,28 +1514,31 @@ fi
%exclude /usr/share/man/man3/*
%changelog
* Mon May 31 2021 overweight <hexiaowen@huawei.com> - 246-36
* Mon May 31 2021 overweight <hexiaowen@huawei.com> - 243-37
- fix journald: enforce longer line length limit during "setup" phase of stream protocol
* Mon May 31 2021 overweight <hexiaowen@huawei.com> - 243-36
- fix patches name and patches num
* Thu May 27 2021 shenyangyang <shenyangyang4@huawei.com> - 246-35
* Thu May 27 2021 shenyangyang <shenyangyang4@huawei.com> - 243-35
- Type:bugfix
- ID:NA
- SUG:NA
- DESC:change requires to openssl-libs as post scripts systemctl requires libssl.so.1.1
* Mon May 10 2021 shenyangyang <shenyangyang4@huawei.com> - 246-34
* Mon May 10 2021 shenyangyang <shenyangyang4@huawei.com> - 243-34
- Type:bugfix
- ID:NA
- SUG:NA
- DESC:backport from upstream to solve the problem when devices claim the same slot
* Fri Apr 02 2021 fangxiuning <fangxiuning@huawei.com> - 246-33
* Fri Apr 02 2021 fangxiuning <fangxiuning@huawei.com> - 243-33
- Type:bugfix
- ID:NA
- SUG:NA
- DESC:fix userdate double free
* Fri Jan 29 2021 overweight <hexiaowen@huawei.com> - 246-32
* Fri Jan 29 2021 overweight <hexiaowen@huawei.com> - 243-32
- Type:cve
- ID:CVE-2018-21029
- SUG:NA