MSVSphere Packaging Team
1 year ago
parent
b807117f5f
commit
d31b3e86ef
@ -0,0 +1,69 @@
|
||||
From ea011ddb65272c74d6378deae3f3a3365aadd77d Mon Sep 17 00:00:00 2001
|
||||
From: Romain Geissler <romain.geissler@amadeus.com>
|
||||
Date: Tue, 20 Jun 2023 16:06:31 +0000
|
||||
Subject: [PATCH] elf-util: discard PT_LOAD segment early based on the start
|
||||
address.
|
||||
|
||||
Indeed when iterating over all the PT_LOAD segment of the core dump
|
||||
while trying to look for the elf headers of a given module, we iterate
|
||||
over them all and try to use the first one for which we can parse a
|
||||
package metadata, but the start address is never taken into account,
|
||||
so absolutely nothing guarantees we actually parse the right ELF header
|
||||
of the right module we are currently iterating on.
|
||||
|
||||
This was tested like this:
|
||||
- Create a core dump using sleep on a fedora 37 container, with an
|
||||
explicit LD_PRELOAD of a library having a valid package metadata:
|
||||
|
||||
podman run -t -i --rm -v $(pwd):$(pwd) -w $(pwd) fedora:37 bash -x -c \
|
||||
'LD_PRELOAD=libreadline.so.8 sleep 1000 & SLEEP_PID="$!" && sleep 1 && kill -11 "${SLEEP_PID}" && mv "core.${SLEEP_PID}" the-core'
|
||||
|
||||
- Then from a fedora 38 container with systemd installed, the resulting
|
||||
core dump has been passed to systemd-coredump with and without this
|
||||
patch. Without this patch, we get:
|
||||
|
||||
Module /usr/bin/sleep from rpm bash-5.2.15-3.fc38.x86_64
|
||||
Module /usr/lib64/libtinfo.so.6.3 from rpm coreutils-9.1-8.fc37.x86_64
|
||||
Module /usr/lib64/libc.so.6 from rpm coreutils-9.1-8.fc37.x86_64
|
||||
Module /usr/lib64/libreadline.so.8.2 from rpm coreutils-9.1-8.fc37.x86_64
|
||||
Module /usr/lib64/ld-linux-x86-64.so.2 from rpm coreutils-9.1-8.fc37.x86_64
|
||||
|
||||
While with this patch we get:
|
||||
|
||||
Module /usr/bin/sleep from rpm bash-5.2.15-3.fc38.x86_64
|
||||
Module /usr/lib64/libtinfo.so.6.3 from rpm ncurses-6.3-5.20220501.fc37.x86_64
|
||||
Module /usr/lib64/libreadline.so.8.2 from rpm readline-8.2-2.fc37.x86_64
|
||||
|
||||
So the parsed package metadata reported by systemd-coredump when the module
|
||||
files are not found on the host (ie the case of crash inside a container) are
|
||||
now correct. The inconsistency of the first module in the above example
|
||||
(sleep is indeed not provided by the bash package) can be ignored as it
|
||||
is a consequence of how this was tested.
|
||||
|
||||
In addition to this, this also fixes the performance issue of
|
||||
systemd-coredump in case of the crashing process uses a large number of
|
||||
shared libraries and having no package metadata, as reported in
|
||||
https://sourceware.org/pipermail/elfutils-devel/2023q2/006225.html.
|
||||
|
||||
(cherry picked from commit 21a2c735e2bfdc3bfdc42f894d6e3d00f4a38dcd)
|
||||
|
||||
Resolves: #2222259
|
||||
---
|
||||
src/shared/elf-util.c | 4 ++++
|
||||
1 file changed, 4 insertions(+)
|
||||
|
||||
diff --git a/src/shared/elf-util.c b/src/shared/elf-util.c
|
||||
index 181735409d..d746f3ab3f 100644
|
||||
--- a/src/shared/elf-util.c
|
||||
+++ b/src/shared/elf-util.c
|
||||
@@ -538,6 +538,10 @@ static int module_callback(Dwfl_Module *mod, void **userdata, const char *name,
|
||||
if (!program_header || program_header->p_type != PT_LOAD)
|
||||
continue;
|
||||
|
||||
+ /* This PT_LOAD segment doesn't contain the start address, so it can't be the module we are looking for. */
|
||||
+ if (start < program_header->p_vaddr || start >= program_header->p_vaddr + program_header->p_memsz)
|
||||
+ continue;
|
||||
+
|
||||
/* Now get a usable Elf reference, and parse the notes from it. */
|
||||
data = sym_elf_getdata_rawchunk(elf,
|
||||
program_header->p_offset,
|
||||
@ -0,0 +1,40 @@
|
||||
From 2fcb340ec5faf51a8d9b0cb2ddd8386b4db6a33d Mon Sep 17 00:00:00 2001
|
||||
From: Romain Geissler <romain.geissler@amadeus.com>
|
||||
Date: Thu, 22 Jun 2023 16:05:18 +0000
|
||||
Subject: [PATCH] elf-util: check for overflow when computing end of core's
|
||||
PT_LOAD segments
|
||||
|
||||
(cherry picked from commit 3965f173eae4701a014113cfaf4a28a6bb63bed7)
|
||||
|
||||
Resolves: #2222259
|
||||
---
|
||||
src/shared/elf-util.c | 9 ++++++++-
|
||||
1 file changed, 8 insertions(+), 1 deletion(-)
|
||||
|
||||
diff --git a/src/shared/elf-util.c b/src/shared/elf-util.c
|
||||
index d746f3ab3f..bde5013b92 100644
|
||||
--- a/src/shared/elf-util.c
|
||||
+++ b/src/shared/elf-util.c
|
||||
@@ -532,14 +532,21 @@ static int module_callback(Dwfl_Module *mod, void **userdata, const char *name,
|
||||
for (size_t i = 0; i < n_program_headers; ++i) {
|
||||
GElf_Phdr mem, *program_header;
|
||||
Elf_Data *data;
|
||||
+ GElf_Addr end_of_segment;
|
||||
|
||||
/* The core file stores the ELF files in the PT_LOAD segment. */
|
||||
program_header = sym_gelf_getphdr(elf, i, &mem);
|
||||
if (!program_header || program_header->p_type != PT_LOAD)
|
||||
continue;
|
||||
|
||||
+ /* Check that the end of segment is a valid address. */
|
||||
+ if (__builtin_add_overflow(program_header->p_vaddr, program_header->p_memsz, &end_of_segment)) {
|
||||
+ log_error("Abort due to corrupted core dump, end of segment address %#zx + %#zx overflows", (size_t)program_header->p_vaddr, (size_t)program_header->p_memsz);
|
||||
+ return DWARF_CB_ABORT;
|
||||
+ }
|
||||
+
|
||||
/* This PT_LOAD segment doesn't contain the start address, so it can't be the module we are looking for. */
|
||||
- if (start < program_header->p_vaddr || start >= program_header->p_vaddr + program_header->p_memsz)
|
||||
+ if (start < program_header->p_vaddr || start >= end_of_segment)
|
||||
continue;
|
||||
|
||||
/* Now get a usable Elf reference, and parse the notes from it. */
|
||||
@ -0,0 +1,64 @@
|
||||
From 6c4090a8bdf54928e43c21d7443d2e6825ed6d1f Mon Sep 17 00:00:00 2001
|
||||
From: Jan Macku <jamacku@redhat.com>
|
||||
Date: Tue, 9 May 2023 13:15:06 +0200
|
||||
Subject: [PATCH] manager: don't taint the host if cgroups v1 is used
|
||||
|
||||
In upstream of systemd, cgroups v1 are not considered as supported.
|
||||
This is not true for RHEL, don't taint the host when cgroups v1 are enabled.
|
||||
|
||||
rhel-only
|
||||
|
||||
Resolves: #2196479
|
||||
---
|
||||
man/org.freedesktop.systemd1.xml | 6 ------
|
||||
src/core/manager.c | 3 ---
|
||||
src/test/test-manager.c | 5 -----
|
||||
3 files changed, 14 deletions(-)
|
||||
|
||||
diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml
|
||||
index 40510c43eb..13a84af747 100644
|
||||
--- a/man/org.freedesktop.systemd1.xml
|
||||
+++ b/man/org.freedesktop.systemd1.xml
|
||||
@@ -1589,12 +1589,6 @@ node /org/freedesktop/systemd1 {
|
||||
<listitem><para>Support for cgroups is unavailable.</para></listitem>
|
||||
</varlistentry>
|
||||
|
||||
- <varlistentry>
|
||||
- <term><literal>cgroupsv1</literal></term>
|
||||
-
|
||||
- <listitem><para>The system is using the old cgroup hierarchy.</para></listitem>
|
||||
- </varlistentry>
|
||||
-
|
||||
<varlistentry>
|
||||
<term><literal>local-hwclock</literal></term>
|
||||
|
||||
diff --git a/src/core/manager.c b/src/core/manager.c
|
||||
index a59afafb58..657263eb73 100644
|
||||
--- a/src/core/manager.c
|
||||
+++ b/src/core/manager.c
|
||||
@@ -4491,9 +4491,6 @@ char* manager_taint_string(const Manager *m) {
|
||||
if (access("/proc/cgroups", F_OK) < 0)
|
||||
stage[n++] = "cgroups-missing";
|
||||
|
||||
- if (cg_all_unified() == 0)
|
||||
- stage[n++] = "cgroupsv1";
|
||||
-
|
||||
if (clock_is_localtime(NULL) > 0)
|
||||
stage[n++] = "local-hwclock";
|
||||
|
||||
diff --git a/src/test/test-manager.c b/src/test/test-manager.c
|
||||
index 89f9277b28..2faf715d76 100644
|
||||
--- a/src/test/test-manager.c
|
||||
+++ b/src/test/test-manager.c
|
||||
@@ -14,11 +14,6 @@ TEST(manager_taint_string) {
|
||||
* to test for them. Let's do just one. */
|
||||
assert_se(!strstr(a, "split-usr"));
|
||||
|
||||
- if (cg_all_unified() == 0)
|
||||
- assert_se(strstr(a, "cgroupsv1"));
|
||||
- else
|
||||
- assert_se(!strstr(a, "cgroupsv1"));
|
||||
-
|
||||
m.taint_usr = true;
|
||||
_cleanup_free_ char *b = manager_taint_string(&m);
|
||||
assert_se(b);
|
||||
Loading…
Reference in new issue