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

!85 fix userdate double free

Browse Source 
From: @fangxiuning
Reviewed-by: 
Signed-off-by:
This commit is contained in:
openeuler-ci-bot 2021-04-01 10:33:59 +08:00 committed by Gitee
commit 7639e0b50d
3 changed files with 276 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

107
backport-process-util-dont-allocate-max-length-to-read-proc-P.patch Normal file
View File

@ -0,0 +1,107 @@
From 7b7a060e83d6c7de8705904d71978ba4664f0a65 Mon Sep 17 00:00:00 2001
From: Anita Zhang <the.anitazha@gmail.com>
Date: Tue, 23 Mar 2021 00:49:28 -0700
Subject: [PATCH] process-util: dont allocate max length to read
/proc/PID/cmdline
Alternative title: Replace get_process_cmdline()'s fopen()/fread() with
read_full_virtual_file().
When RLIMIT_STACK is set to infinity:infinity, _SC_ARG_MAX will
return 4611686018427387903 (depending on the system, but definitely
something larger than most systems have). It's impractical to allocate this
in one go when most cmdlines are much shorter than that.
Instead use read_full_virtual_file() which seems to increase the buffer
depending on the size of the contents.
---
src/basic/process-util.c | 28 +++-------------------------
src/test/test-process-util.c | 5 +++++
2 files changed, 8 insertions(+), 25 deletions(-)
diff --git a/src/basic/process-util.c b/src/basic/process-util.c
index 264ecc276b..7d4301eadb 100644
--- a/src/basic/process-util.c
+++ b/src/basic/process-util.c
@@ -124,14 +124,10 @@ int get_process_comm(pid_t pid, char **ret) {
}
int get_process_cmdline(pid_t pid, size_t max_columns, ProcessCmdlineFlags flags, char **line) {
- _cleanup_fclose_ FILE *f = NULL;
_cleanup_free_ char *t = NULL, *ans = NULL;
const char *p;
- int r;
size_t k;
-
- /* This is supposed to be a safety guard against runaway command lines. */
- size_t max_length = sc_arg_max();
+ int r;
assert(line);
assert(pid >= 0);
@@ -147,36 +143,18 @@ int get_process_cmdline(pid_t pid, size_t max_columns, ProcessCmdlineFlags flags
* comm_fallback is false). Returns 0 and sets *line otherwise. */
p = procfs_file_alloca(pid, "cmdline");
- r = fopen_unlocked(p, "re", &f);
+ r = read_full_virtual_file(p, &t, &k);
if (r == -ENOENT)
return -ESRCH;
if (r < 0)
return r;
- /* We assume that each four-byte character uses one or two columns. If we ever check for combining
- * characters, this assumption will need to be adjusted. */
- if ((size_t) 4 * max_columns + 1 < max_columns)
- max_length = MIN(max_length, (size_t) 4 * max_columns + 1);
-
- t = new(char, max_length);
- if (!t)
- return -ENOMEM;
-
- k = fread(t, 1, max_length, f);
if (k > 0) {
/* Arguments are separated by NULs. Let's replace those with spaces. */
for (size_t i = 0; i < k - 1; i++)
if (t[i] == '\0')
t[i] = ' ';
-
- t[k] = '\0'; /* Normally, t[k] is already NUL, so this is just a guard in case of short read */
} else {
- /* We only treat getting nothing as an error. We *could* also get an error after reading some
- * data, but we ignore that case, as such an error is rather unlikely and we prefer to get
- * some data rather than none. */
- if (ferror(f))
- return -errno;
-
if (!(flags & PROCESS_CMDLINE_COMM_FALLBACK))
return -ENOENT;
@@ -187,7 +165,7 @@ int get_process_cmdline(pid_t pid, size_t max_columns, ProcessCmdlineFlags flags
if (r < 0)
return r;
- mfree(t);
+ free(t);
t = strjoin("[", t2, "]");
if (!t)
return -ENOMEM;
diff --git a/src/test/test-process-util.c b/src/test/test-process-util.c
index 0ebef530a0..5ca654e193 100644
--- a/src/test/test-process-util.c
+++ b/src/test/test-process-util.c
@@ -236,6 +236,11 @@ static void test_get_process_cmdline_harder(void) {
return;
}
+ /* Set RLIMIT_STACK to infinity to test we don't try to allocate unncessarily large values to read
+ * the cmdline. */
+ if (setrlimit(RLIMIT_STACK, &RLIMIT_MAKE_CONST(RLIM_INFINITY)) < 0)
+ log_warning("Testing without RLIMIT_STACK=infinity");
+
assert_se(unlink(path) >= 0);
assert_se(prctl(PR_SET_NAME, "testa") >= 0);
--
2.23.0

160
backport-varlink-make-userdata-pointer-inheritance-from-varli.patch Normal file
View File

@ -0,0 +1,160 @@
From 9807fdc1da8e037ddedfa4e2c6d2728b6e60051e Mon Sep 17 00:00:00 2001
From: Lennart Poettering <lennart@poettering.net>
Date: Wed, 20 Jan 2021 19:15:55 +0100
Subject: [PATCH] varlink: make 'userdata' pointer inheritance from varlink
server to connection optional
@keszybz's right on
https://github.com/systemd/systemd/pull/18248#issuecomment-760798473:
swapping out the userdata pointer of a live varlink connection is iffy.
Let's fix this by making the userdata inheritance from VarlinkServer
object to the Varlink connection object optional: we want it for most
cases, but not all, i.e. all those cases where the calls implemented as
varlink methods are stateless and can be answered synchronously. For the
other cases (i.e. where we want per-connection objects that wrap the
asynchronous operation as it goes on) let's not do such inheritance but
initialize the userdata pointer only once we have it. THis means the
original manager object must be manually retrieved from the
VarlinkServer object, which in turn needs to be requested from the
Varlink connection object.
The userdata inheritance is now controlled by the
VARLINK_INHERIT_USERDATA flag passed at VarlinkServer construction.
Alternative-to: #18248
---
src/core/core-varlink.c | 2 +-
src/home/homed-manager.c | 2 +-
src/journal/journald-server.c | 2 +-
src/machine/machined-varlink.c | 2 +-
src/resolve/resolved-varlink.c | 8 ++++++--
src/shared/varlink.c | 4 +++-
src/shared/varlink.h | 9 +++++----
7 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/src/core/core-varlink.c b/src/core/core-varlink.c
index dd6c11ab4d..d695106658 100644
--- a/src/core/core-varlink.c
+++ b/src/core/core-varlink.c
@@ -432,7 +432,7 @@ int manager_varlink_init(Manager *m) {
if (!MANAGER_IS_SYSTEM(m))
return 0;
- r = varlink_server_new(&s, VARLINK_SERVER_ACCOUNT_UID);
+ r = varlink_server_new(&s, VARLINK_SERVER_ACCOUNT_UID|VARLINK_SERVER_INHERIT_USERDATA);
if (r < 0)
return log_error_errno(r, "Failed to allocate varlink server object: %m");
diff --git a/src/home/homed-manager.c b/src/home/homed-manager.c
index 365ea4d234..91cabd5cef 100644
--- a/src/home/homed-manager.c
+++ b/src/home/homed-manager.c
@@ -956,7 +956,7 @@ static int manager_bind_varlink(Manager *m) {
assert(m);
assert(!m->varlink_server);
- r = varlink_server_new(&m->varlink_server, VARLINK_SERVER_ACCOUNT_UID);
+ r = varlink_server_new(&m->varlink_server, VARLINK_SERVER_ACCOUNT_UID|VARLINK_SERVER_INHERIT_USERDATA);
if (r < 0)
return log_error_errno(r, "Failed to allocate varlink server object: %m");
diff --git a/src/journal/journald-server.c b/src/journal/journald-server.c
index 10ebc3e22e..5cad374083 100644
--- a/src/journal/journald-server.c
+++ b/src/journal/journald-server.c
@@ -2033,7 +2033,7 @@ static int server_open_varlink(Server *s, const char *socket, int fd) {
assert(s);
- r = varlink_server_new(&s->varlink_server, VARLINK_SERVER_ROOT_ONLY);
+ r = varlink_server_new(&s->varlink_server, VARLINK_SERVER_ROOT_ONLY|VARLINK_SERVER_INHERIT_USERDATA);
if (r < 0)
return r;
diff --git a/src/machine/machined-varlink.c b/src/machine/machined-varlink.c
index 2d6c1991a4..009d283acc 100644
--- a/src/machine/machined-varlink.c
+++ b/src/machine/machined-varlink.c
@@ -388,7 +388,7 @@ int manager_varlink_init(Manager *m) {
if (m->varlink_server)
return 0;
- r = varlink_server_new(&s, VARLINK_SERVER_ACCOUNT_UID);
+ r = varlink_server_new(&s, VARLINK_SERVER_ACCOUNT_UID|VARLINK_SERVER_INHERIT_USERDATA);
if (r < 0)
return log_error_errno(r, "Failed to allocate varlink server object: %m");
diff --git a/src/resolve/resolved-varlink.c b/src/resolve/resolved-varlink.c
index 70d6f9056d..2fb9d38dfa 100644
--- a/src/resolve/resolved-varlink.c
+++ b/src/resolve/resolved-varlink.c
@@ -269,11 +269,13 @@ static int vl_method_resolve_hostname(Varlink *link, JsonVariant *parameters, Va
_cleanup_(lookup_parameters_destroy) LookupParameters p = {
.family = AF_UNSPEC,
};
- Manager *m = userdata;
DnsQuery *q;
+ Manager *m;
int r;
assert(link);
+
+ m = varlink_server_get_userdata(varlink_get_server(link));
assert(m);
if (FLAGS_SET(flags, VARLINK_METHOD_ONEWAY))
@@ -447,11 +449,13 @@ static int vl_method_resolve_address(Varlink *link, JsonVariant *parameters, Var
_cleanup_(lookup_parameters_destroy) LookupParameters p = {
.family = AF_UNSPEC,
};
- Manager *m = userdata;
DnsQuery *q;
+ Manager *m;
int r;
assert(link);
+
+ m = varlink_server_get_userdata(varlink_get_server(link));
assert(m);
if (FLAGS_SET(flags, VARLINK_METHOD_ONEWAY))
diff --git a/src/shared/varlink.c b/src/shared/varlink.c
index 274709abd5..24865fa07e 100644
--- a/src/shared/varlink.c
+++ b/src/shared/varlink.c
@@ -2137,7 +2137,9 @@ int varlink_server_add_connection(VarlinkServer *server, int fd, Varlink **ret)
return r;
v->fd = fd;
- v->userdata = server->userdata;
+ if (server->flags & VARLINK_SERVER_INHERIT_USERDATA)
+ v->userdata = server->userdata;
+
if (ucred_acquired) {
v->ucred = ucred;
v->ucred_acquired = true;
diff --git a/src/shared/varlink.h b/src/shared/varlink.h
index 7ea1f9113f..66a1ff630e 100644
--- a/src/shared/varlink.h
+++ b/src/shared/varlink.h
@@ -41,11 +41,12 @@ typedef enum VarlinkMethodFlags {
} VarlinkMethodFlags;
typedef enum VarlinkServerFlags {
- VARLINK_SERVER_ROOT_ONLY = 1 << 0, /* Only accessible by root */
- VARLINK_SERVER_MYSELF_ONLY = 1 << 1, /* Only accessible by our own UID */
- VARLINK_SERVER_ACCOUNT_UID = 1 << 2, /* Do per user accounting */
+ VARLINK_SERVER_ROOT_ONLY = 1 << 0, /* Only accessible by root */
+ VARLINK_SERVER_MYSELF_ONLY = 1 << 1, /* Only accessible by our own UID */
+ VARLINK_SERVER_ACCOUNT_UID = 1 << 2, /* Do per user accounting */
+ VARLINK_SERVER_INHERIT_USERDATA = 1 << 3, /* Initialize Varlink connection userdata from VarlinkServer userdata */
- _VARLINK_SERVER_FLAGS_ALL = (1 << 3) - 1,
+ _VARLINK_SERVER_FLAGS_ALL = (1 << 4) - 1,
} VarlinkServerFlags;
typedef int (*VarlinkMethod)(Varlink *link, JsonVariant *parameters, VarlinkMethodFlags flags, void *userdata);
--
2.23.0

10
systemd.spec
View File

@ -20,7 +20,7 @@
Name: systemd
Url: https://www.freedesktop.org/wiki/Software/systemd
Version: 246
Release: 14
Release: 15
License: MIT and LGPLv2+ and GPLv2+
Summary: System and Service Manager
@ -73,6 +73,8 @@ Patch0020: scope-on-unified-make-sure-to-unwatch-all-PIDs-once-.patch
Patch6000: backport-xdg-autostart-Lower-most-info-messages-to-debug-leve.patch
Patch6001: backport-RFC-Make-user-instance-aware-of-delegated-cgroup-controllers.patch
Patch6002: backport-core-Make-user-instance-aware-of-delegated-cgroup.patch
Patch6003: backport-varlink-make-userdata-pointer-inheritance-from-varli.patch
Patch6004: backport-process-util-dont-allocate-max-length-to-read-proc-P.patch
BuildRequires: gcc, gcc-c++
BuildRequires: libcap-devel, libmount-devel, pam-devel, libselinux-devel
@ -1488,6 +1490,12 @@ fi
%exclude /usr/share/man/man3/*
%changelog
* Wed Mar 31 2021 fangxiuning <fangxiuning@huawei.com> - 246-15
- Type:bugfix
- ID:NA
- SUG:NA
- DESC:fix userdata double free
* Wed Mar 3 2021 shenyangyang <shenyangyang4@huawei.com> - 246-14
- Type:bugfix
- ID:NA