From 4b2af1ee0fdafc52b16570c52910edf3e7abd7f9 Mon Sep 17 00:00:00 2001 From: Zbigniew Jędrzejewski-Szmek Date: Mar 11 2021 11:37:25 +0000 Subject: Backport one patch for beta freeze exception --- diff --git a/18892.patch b/18892.patch index e503d26..675aaf1 100644 --- a/18892.patch +++ b/18892.patch @@ -1,4 +1,4 @@ -From 670a1ebe9aa0b6da5a3ae62bf5ad927721fc812b Mon Sep 17 00:00:00 2001 +From e0ae456a554d0fce250f9a009c561b97f20c41f8 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 5 Mar 2021 17:47:45 +0100 Subject: [PATCH 1/6] dns-query: export CNAME_MAX, so that we can use it in @@ -47,7 +47,7 @@ index ea296167b61..5d12171b0a1 100644 + +#define CNAME_REDIRECT_MAX 16 -From 6fe1d507354710c62d735e5fbd48e014b547a76e Mon Sep 17 00:00:00 2001 +From d29958261a3df80f5cf0e98b1cd307790a92b13b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 5 Mar 2021 17:48:43 +0100 Subject: [PATCH 2/6] resolved: tighten checks in @@ -95,15 +95,15 @@ index 823117e5c92..7e76e0c6cc0 100644 r = dns_name_equal(dns_resource_key_name(key), dns_resource_key_name(cname->key)); -From 8fb7a20db536b992135a2654f08af0007f268b48 Mon Sep 17 00:00:00 2001 +From 4838dc4f2be1d29da9ce9a930c48717a4491d70e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 5 Mar 2021 17:53:31 +0100 Subject: [PATCH 3/6] resolved: handle multiple CNAME redirects in a single reply from upstream www.netflix.com responds with a chain of CNAMEs in the same packet. -Let's handle that properly (so far we only followed CNAMEs on step when -in the same packet) +Let's handle that properly (so far we only followed CNAMEs a single step +when in the same packet) Fixes: #18819 --- @@ -111,7 +111,7 @@ Fixes: #18819 1 file changed, 57 insertions(+), 48 deletions(-) diff --git a/src/resolve/resolved-dns-stub.c b/src/resolve/resolved-dns-stub.c -index c2734e57b9b..098a86fca3f 100644 +index c2734e57b9b..c3a28d390a4 100644 --- a/src/resolve/resolved-dns-stub.c +++ b/src/resolve/resolved-dns-stub.c @@ -162,79 +162,88 @@ static int dns_stub_collect_answer_by_question( @@ -126,7 +126,7 @@ index c2734e57b9b..098a86fca3f 100644 - /* Copies all RRs from 'answer' into 'reply', if they match 'question'. */ + /* Copies all RRs from 'answer' into 'reply', if they match 'question'. There might be direct and -+ * indirect matches (i.e. via CNAME/DNAME). If the have an indirect one, remember where we need to ++ * indirect matches (i.e. via CNAME/DNAME). If they have an indirect one, remember where we need to + * go, and restart the loop */ + + for (;;) { @@ -178,7 +178,7 @@ index c2734e57b9b..098a86fca3f 100644 - dns_resource_key_unref(redirected_key); + /* Oh, wow, this is a redirect. Let's remember where this points, and store + * it in 'next_redirected_key'. Once we finished iterating through the rest -+ * of the RR's we'll start again, with the redirected RR. */ ++ * of the RR's we'll start again, with the redirected RR key. */ + + n_cname_redirects++; + if (n_cname_redirects > CNAME_REDIRECT_MAX) /* don't loop forever */ @@ -252,7 +252,7 @@ index c2734e57b9b..098a86fca3f 100644 return 0; -From b54de8231bd35f08af46b76dae1028397a19a31e Mon Sep 17 00:00:00 2001 +From 39005e187095062718621880e5d8ad707ac8fe8f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 5 Mar 2021 18:01:27 +0100 Subject: [PATCH 4/6] resolved: split out helper that checks whether we shall @@ -264,7 +264,7 @@ Just some refactoring, no actual code changes. 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/resolve/resolved-dns-stub.c b/src/resolve/resolved-dns-stub.c -index 098a86fca3f..67e38bea6ea 100644 +index c3a28d390a4..b4df5837aad 100644 --- a/src/resolve/resolved-dns-stub.c +++ b/src/resolve/resolved-dns-stub.c @@ -561,6 +561,19 @@ static int dns_stub_send( @@ -299,12 +299,12 @@ index 098a86fca3f..67e38bea6ea 100644 - (q->answer_dnssec_result >= 0 || /* we did proper DNSSEC validation … */ - dns_query_fully_authenticated(q) || /* … or we considered it authentic otherwise … */ - DNS_PACKET_CD(q->request_packet)); /* … or client set CD */ -+ edns0_do = dns_stub_reply_with_edns0_do(q); /* let' check if we shall reply with EDNS0 DO? */ ++ edns0_do = dns_stub_reply_with_edns0_do(q); /* let's check if we shall reply with EDNS0 DO? */ r = dns_stub_assign_sections( q, -From fd67ea0d9804b8aaea4bda7527afe287060d14db Mon Sep 17 00:00:00 2001 +From b97fc57178932689bdcb9030e1e2bf299d49ce0b Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 5 Mar 2021 16:50:04 +0100 Subject: [PATCH 5/6] resolved: fully follow CNAMEs in the DNS stub after all @@ -322,12 +322,12 @@ like we do for the D-Bus/Varlink case, and collect replies as we go. 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/src/resolve/resolved-dns-stub.c b/src/resolve/resolved-dns-stub.c -index 67e38bea6ea..486b8146acf 100644 +index b4df5837aad..85c4eda469c 100644 --- a/src/resolve/resolved-dns-stub.c +++ b/src/resolve/resolved-dns-stub.c @@ -586,13 +586,6 @@ static int dns_stub_send_reply( - edns0_do = dns_stub_reply_with_edns0_do(q); /* let' check if we shall reply with EDNS0 DO? */ + edns0_do = dns_stub_reply_with_edns0_do(q); /* let's check if we shall reply with EDNS0 DO? */ - r = dns_stub_assign_sections( - q, @@ -389,7 +389,7 @@ index 67e38bea6ea..486b8146acf 100644 SD_RESOLVED_CLAMP_TTL); if (r < 0) { -From 42a0086a3e8939ab58cc81409f54ac64a2358923 Mon Sep 17 00:00:00 2001 +From 5d7da51ee1d27e86a0487a4b2abc3cfb0ed44c23 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Fri, 5 Mar 2021 18:20:59 +0100 Subject: [PATCH 6/6] resolved: when synthesizing stub replies from multiple @@ -404,11 +404,11 @@ sections when they are already listed in a "higher-priority" section. --- src/resolve/resolved-dns-answer.c | 25 +++++++++++++++++++++++++ src/resolve/resolved-dns-answer.h | 1 + - src/resolve/resolved-dns-stub.c | 18 ++++++++++++++++++ - 3 files changed, 44 insertions(+) + src/resolve/resolved-dns-stub.c | 20 ++++++++++++++++++++ + 3 files changed, 46 insertions(+) diff --git a/src/resolve/resolved-dns-answer.c b/src/resolve/resolved-dns-answer.c -index ce3cbce308d..8db97dce567 100644 +index ce3cbce308d..a667ab5ede4 100644 --- a/src/resolve/resolved-dns-answer.c +++ b/src/resolve/resolved-dns-answer.c @@ -640,6 +640,31 @@ int dns_answer_remove_by_rr(DnsAnswer **a, DnsResourceRecord *rm) { @@ -431,8 +431,8 @@ index ce3cbce308d..8db97dce567 100644 + if (r < 0) + return r; + -+ /* Let's remember this entrie's RR key, to optimize the loop a bit: if we have an RRset with -+ * more than one item then we don#t need to remove the key multiple times */ ++ /* Let's remember this entry's RR key, to optimize the loop a bit: if we have an RRset with ++ * more than one item then we don't need to remove the key multiple times */ + dns_resource_key_unref(prev); + prev = dns_resource_key_ref(item->rr->key); + } @@ -456,23 +456,25 @@ index c2fd0c078f4..7d19eee4e2b 100644 int dns_answer_copy_by_key(DnsAnswer **a, DnsAnswer *source, const DnsResourceKey *key, DnsAnswerFlags or_flags, DnsResourceRecord *rrsig); int dns_answer_move_by_key(DnsAnswer **to, DnsAnswer **from, const DnsResourceKey *key, DnsAnswerFlags or_flags, DnsResourceRecord *rrsig); diff --git a/src/resolve/resolved-dns-stub.c b/src/resolve/resolved-dns-stub.c -index 486b8146acf..7fc8b4fdd4f 100644 +index 85c4eda469c..8e781dd7389 100644 --- a/src/resolve/resolved-dns-stub.c +++ b/src/resolve/resolved-dns-stub.c -@@ -574,6 +574,22 @@ static int dns_stub_reply_with_edns0_do(DnsQuery *q) { +@@ -574,6 +574,24 @@ static int dns_stub_reply_with_edns0_do(DnsQuery *q) { DNS_PACKET_CD(q->request_packet)); /* … or client set CD */ } +static void dns_stub_suppress_duplicate_section_rrs(DnsQuery *q) { + /* If we follow a CNAME/DNAME chain we might end up populating our sections with redundant RRs + * because we built up the sections from multiple reply packets (one from each CNAME/DNAME chain -+ * element). e.g. it could be that an RR that was included in the first reply's additional section ++ * element). E.g. it could be that an RR that was included in the first reply's additional section + * ends up being relevant as main answer in a subsequent reply in the chain. Let's clean this up, and -+ * remove everything from the "higher priority" sections from the "lower priority" sections if they -+ * exists in both. ++ * remove everything in the "higher priority" sections from the "lower priority" sections. + * + * Note that this removal matches by RR keys instead of the full RRs. This is because RRsets should -+ * always end up in one section fully or not at all, but never be split among sections. */ ++ * always end up in one section fully or not at all, but never be split among sections. ++ * ++ * Specifically: we remove ANSWER section RRs from the AUTHORITATIVE and ADDITIONAL sections, as well ++ * as AUTHORITATIVE section RRs from the ADDITIONAL section. */ + + dns_answer_remove_by_answer_keys(&q->reply_authoritative, q->reply_answer); + dns_answer_remove_by_answer_keys(&q->reply_additional, q->reply_answer); @@ -482,7 +484,7 @@ index 486b8146acf..7fc8b4fdd4f 100644 static int dns_stub_send_reply( DnsQuery *q, int rcode) { -@@ -594,6 +610,8 @@ static int dns_stub_send_reply( +@@ -594,6 +612,8 @@ static int dns_stub_send_reply( if (r < 0) return log_debug_errno(r, "Failed to build reply packet: %m"); diff --git a/18915.patch b/18915.patch new file mode 100644 index 0000000..534b5bf --- /dev/null +++ b/18915.patch @@ -0,0 +1,154 @@ +From 8b0f54c9290564e8c27c9c8ac464cdcc2c659ad5 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= +Date: Sat, 6 Mar 2021 19:06:08 +0100 +Subject: [PATCH 1/3] pid1: return varlink error on the right connection + +--- + src/core/core-varlink.c | 6 +++--- + 1 file changed, 3 insertions(+), 3 deletions(-) + +diff --git a/src/core/core-varlink.c b/src/core/core-varlink.c +index d695106658b..b3df8cd893c 100644 +--- a/src/core/core-varlink.c ++++ b/src/core/core-varlink.c +@@ -142,7 +142,7 @@ static int vl_method_subscribe_managed_oom_cgroups( + /* We only take one subscriber for this method so return an error if there's already an existing one. + * This shouldn't happen since systemd-oomd is the only client of this method. */ + if (FLAGS_SET(flags, VARLINK_METHOD_MORE) && m->managed_oom_varlink_request) +- return varlink_error(m->managed_oom_varlink_request, VARLINK_ERROR_SUBSCRIPTION_TAKEN, NULL); ++ return varlink_error(link, VARLINK_ERROR_SUBSCRIPTION_TAKEN, NULL); + + r = json_build(&arr, JSON_BUILD_EMPTY_ARRAY); + if (r < 0) +@@ -188,6 +188,7 @@ static int vl_method_subscribe_managed_oom_cgroups( + if (!FLAGS_SET(flags, VARLINK_METHOD_MORE)) + return varlink_reply(link, v); + ++ assert(!m->managed_oom_varlink_request); + m->managed_oom_varlink_request = varlink_ref(link); + return varlink_notify(m->managed_oom_varlink_request, v); + } +@@ -475,8 +476,7 @@ void manager_varlink_done(Manager *m) { + assert(m); + + /* Send the final message if we still have a subscribe request open. */ +- if (m->managed_oom_varlink_request) +- m->managed_oom_varlink_request = varlink_close_unref(m->managed_oom_varlink_request); ++ m->managed_oom_varlink_request = varlink_close_unref(m->managed_oom_varlink_request); + + m->varlink_server = varlink_server_unref(m->varlink_server); + } + +From 39ad3f1c092b5dffcbb4b1d12eb9ca407f010a3c Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= +Date: Sun, 7 Mar 2021 16:42:35 +0100 +Subject: [PATCH 2/3] varlink: avoid using dangling ref in + varlink_close_unref() + +Fixes #18025, https://bugzilla.redhat.com/show_bug.cgi?id=1931034. + +We drop the reference stored in Manager.managed_oom_varlink_request in two code paths: +vl_disconnect() which is installed as a disconnect callback, and in manager_varlink_done(). +But we also make a disconnect from manager_varlink_done(). So we end up with the following +call stack: + +(gdb) bt + 0 vl_disconnect (s=0x112c7b0, link=0xea0070, userdata=0xe9bcc0) at ../src/core/core-varlink.c:414 + 1 0x00007f1366e9d5ac in varlink_detach_server (v=0xea0070) at ../src/shared/varlink.c:1210 + 2 0x00007f1366e9d664 in varlink_close (v=0xea0070) at ../src/shared/varlink.c:1228 + 3 0x00007f1366e9d6b5 in varlink_close_unref (v=0xea0070) at ../src/shared/varlink.c:1240 + 4 0x0000000000524629 in manager_varlink_done (m=0xe9bcc0) at ../src/core/core-varlink.c:479 + 5 0x000000000048ef7b in manager_free (m=0xe9bcc0) at ../src/core/manager.c:1357 + 6 0x000000000042602c in main (argc=5, argv=0x7fff439c43d8) at ../src/core/main.c:2909 + +When we enter vl_disconnect(), m->managed_oom_varlink_request.n_ref==1. +When we exit from vl_discconect(), m->managed_oom_varlink_request==NULL. But +varlink_close_unref() has a copy of the pointer in *v. When we continue executing +varlink_close_unref(), this pointer is dangling, and the call to varlink_unref() +is done with an invalid pointer. +--- + src/shared/varlink.c | 33 +++++++++++++++++++++++++-------- + 1 file changed, 25 insertions(+), 8 deletions(-) + +diff --git a/src/shared/varlink.c b/src/shared/varlink.c +index 31128e02e06..6ed72075ba5 100644 +--- a/src/shared/varlink.c ++++ b/src/shared/varlink.c +@@ -1206,8 +1206,9 @@ int varlink_close(Varlink *v) { + + varlink_set_state(v, VARLINK_DISCONNECTED); + +- /* Let's take a reference first, since varlink_detach_server() might drop the final (dangling) ref +- * which would destroy us before we can call varlink_clear() */ ++ /* Let's take a reference first, since varlink_detach_server() might drop the final ref from the ++ * disconnect callback, which would invalidate the pointer we are holding before we can call ++ * varlink_clear(). */ + varlink_ref(v); + varlink_detach_server(v); + varlink_clear(v); +@@ -1220,17 +1221,33 @@ Varlink* varlink_close_unref(Varlink *v) { + if (!v) + return NULL; + +- (void) varlink_close(v); ++ /* A reference is given to us to be destroyed. But when calling varlink_close(), a callback might ++ * also drop a reference. We allow this, and will hold a temporary reference to the object to make ++ * sure that the object still exists when control returns to us. If there's just one reference ++ * remaining after varlink_close(), even though there were at least two right before, we'll handle ++ * that gracefully instead of crashing. ++ * ++ * In other words, this call drops the donated reference, but if the internal call to varlink_close() ++ * dropped a reference to, we don't drop the reference afain. This allows the caller to say: ++ * global_object->varlink = varlink_close_unref(global_object->varlink); ++ * even though there is some callback which has access to global_object and may drop the reference ++ * stored in global_object->varlink. Without this step, the same code would have to be written as: ++ * Varlink *t = TAKE_PTR(global_object->varlink); ++ * varlink_close_unref(t); ++ */ ++ /* n_ref >= 1 */ ++ varlink_ref(v); /* n_ref >= 2 */ ++ varlink_close(v); /* n_ref >= 1 */ ++ if (v->n_ref > 1) ++ v->n_ref--; /* n_ref >= 1 */ + return varlink_unref(v); + } + + Varlink* varlink_flush_close_unref(Varlink *v) { +- if (!v) +- return NULL; ++ if (v) ++ varlink_flush(v); + +- (void) varlink_flush(v); +- (void) varlink_close(v); +- return varlink_unref(v); ++ return varlink_close_unref(v); + } + + static int varlink_enqueue_json(Varlink *v, JsonVariant *m) { + +From a19c1a4baaa1dadc80885e3ad41f19a6c6c450fd Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= +Date: Mon, 8 Mar 2021 09:21:25 +0100 +Subject: [PATCH 3/3] oomd: "downgrade" level of message + +PID1 already logs about the service being started, so this line isn't necessary +in normal use. Also, by the time it is emitted, the service has already +signalled readiness, so let's not say "starting" but "started". +--- + src/oom/oomd.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/oom/oomd.c b/src/oom/oomd.c +index 674d53fdcfe..6e2a5889d1e 100644 +--- a/src/oom/oomd.c ++++ b/src/oom/oomd.c +@@ -170,7 +170,7 @@ static int run(int argc, char *argv[]) { + + notify_msg = notify_start(NOTIFY_READY, NOTIFY_STOPPING); + +- log_info("systemd-oomd starting%s!", arg_dry_run ? " in dry run mode" : ""); ++ log_debug("systemd-oomd started%s.", arg_dry_run ? " in dry run mode" : ""); + + r = sd_event_loop(m->event); + if (r < 0) diff --git a/systemd.spec b/systemd.spec index cde6ea9..5f12475 100644 --- a/systemd.spec +++ b/systemd.spec @@ -21,7 +21,7 @@ Name: systemd Url: https://www.freedesktop.org/wiki/Software/systemd Version: 248~rc2 -Release: 3%{?dist} +Release: 4%{?dist} # For a breakdown of the licensing, see README License: LGPLv2+ and MIT and GPLv2+ Summary: System and Service Manager @@ -70,10 +70,13 @@ i=1; for j in 00*patch; do printf "Patch%04d: %s\n" $i $j; i=$((i+1));done| GIT_DIR=../../src/systemd/.git git diffab -M v233..master@{2017-06-15} -- hwdb/[67]* hwdb/parse_hwdb.py > hwdb.patch %endif -# https://github.com/systemd/systemd/pull/18892 -# Fix stub resolver handling of CNAME chains +# Backports of patches from upstream (0000–0499) + # https://bugzilla.redhat.com/show_bug.cgi?id=1933433 -Patch0: 18892.patch +Patch0000: https://github.com/systemd/systemd/pull/18892.patch + +# https://bugzilla.redhat.com/show_bug.cgi?id=1931034 +Patch0001: https://github.com/systemd/systemd/pull/18915.patch # Downstream-only patches (5000–9999) # https://bugzilla.redhat.com/show_bug.cgi?id=1738828 @@ -937,8 +940,11 @@ getent passwd systemd-network &>/dev/null || useradd -r -u 192 -l -g systemd-net %files standalone-sysusers -f .file-list-standalone-sysusers %changelog +* Thu Mar 11 2021 Zbigniew Jędrzejewski-Szmek - 248~rc2-4 +- Fix crash in pid1 during daemon-reexec (#1931034) + * Fri Mar 05 2021 Adam Williamson - 248~rc2-3 -- Backport PR #18892 to fix stub resolver CNAME chain resolving (#1933433) +- Fix stub resolver CNAME chain resolving (#1933433) * Mon Mar 01 2021 Josh Boyer - 248~rc2-2 - Don't set the fallback hostname to Fedora on non-Fedora OSes