From a422446d7c2cd106007eececcc4a5aec09a3c590 Mon Sep 17 00:00:00 2001 From: fangxiuning Date: Wed, 31 Mar 2021 16:06:18 +0800 Subject: [PATCH] modify --- ...t-allocate-max-length-to-read-proc-P.patch | 107 ++++++++++++ ...rdata-pointer-inheritance-from-varli.patch | 160 ++++++++++++++++++ systemd.spec | 10 +- 3 files changed, 276 insertions(+), 1 deletion(-) create mode 100644 backport-process-util-dont-allocate-max-length-to-read-proc-P.patch create mode 100644 backport-varlink-make-userdata-pointer-inheritance-from-varli.patch diff --git a/backport-process-util-dont-allocate-max-length-to-read-proc-P.patch b/backport-process-util-dont-allocate-max-length-to-read-proc-P.patch new file mode 100644 index 00000000..a910eb0d --- /dev/null +++ b/backport-process-util-dont-allocate-max-length-to-read-proc-P.patch @@ -0,0 +1,107 @@ +From 7b7a060e83d6c7de8705904d71978ba4664f0a65 Mon Sep 17 00:00:00 2001 +From: Anita Zhang +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 + diff --git a/backport-varlink-make-userdata-pointer-inheritance-from-varli.patch b/backport-varlink-make-userdata-pointer-inheritance-from-varli.patch new file mode 100644 index 00000000..fd0c489b --- /dev/null +++ b/backport-varlink-make-userdata-pointer-inheritance-from-varli.patch @@ -0,0 +1,160 @@ +From 9807fdc1da8e037ddedfa4e2c6d2728b6e60051e Mon Sep 17 00:00:00 2001 +From: Lennart Poettering +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 + diff --git a/systemd.spec b/systemd.spec index c04d1e17..14dbd08f 100644 --- a/systemd.spec +++ b/systemd.spec @@ -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 - 246-15 +- Type:bugfix +- ID:NA +- SUG:NA +- DESC:fix userdata double free + * Wed Mar 3 2021 shenyangyang - 246-14 - Type:bugfix - ID:NA -- Gitee