Backport one patch for beta freeze exception

This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2021-03-11 12:37:25 +01:00
parent e5f70dadb2
commit 4b2af1ee0f
3 changed files with 194 additions and 32 deletions

View File

@ -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 <lennart@poettering.net>
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 <lennart@poettering.net>
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 <lennart@poettering.net>
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 <lennart@poettering.net>
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 <lennart@poettering.net>
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 <lennart@poettering.net>
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");

154
18915.patch Normal file
View File

@ -0,0 +1,154 @@
From 8b0f54c9290564e8c27c9c8ac464cdcc2c659ad5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>
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?= <zbyszek@in.waw.pl>
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?= <zbyszek@in.waw.pl>
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)

View File

@ -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 (00000499)
# 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 (50009999)
# 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 <zbyszek@in.waw.pl> - 248~rc2-4
- Fix crash in pid1 during daemon-reexec (#1931034)
* Fri Mar 05 2021 Adam Williamson <awilliam@redhat.com> - 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 <jwboyer@fedoraproject.org> - 248~rc2-2
- Don't set the fallback hostname to Fedora on non-Fedora OSes