1
0
mirror of https://github.com/systemd/systemd synced 2026-03-15 09:34:47 +01:00

Compare commits

..

2 Commits

Author SHA1 Message Date
Zbigniew Jędrzejewski-Szmek
c856ef0457 resolved: fix braino with reference counting and linked lists
In 0e0fd08fc832b8f42e567d722d388eba086da5ff I added reference counts to keep
track of the DnsQueryCandidate objects. Unfortunately, dns_query_unref_candidates()
was written as

     while (q->candidates)
           dns_query_candidate_unref(q->candidates);

i.e. it would keep dropping the reference count as many times as needed for it
to hit 0, making the patch less than fully effective.

dns_query_unref_candidates() is renamed to dns_query_detach_candidates() and
changed to drop exactly one reference from each of the linked candidates.

Example failure:
==463== Invalid read of size 8
==463==    at 0x419C93: dns_query_candidate_go (resolved-dns-query.c:159)
==463==    by 0x41A143: dns_query_candidate_notify (resolved-dns-query.c:304)
==463==    by 0x434BD6: dns_transaction_complete (resolved-dns-transaction.c:437)
==463==    by 0x436A0F: dns_transaction_process_dnssec (resolved-dns-transaction.c:976)
==463==    by 0x4378C1: dns_transaction_process_reply (resolved-dns-transaction.c:1387)
==463==    by 0x437CE9: on_dns_packet (resolved-dns-transaction.c:1444)
==463==    by 0x4B2DC9B: source_dispatch (sd-event.c:3512)
==463==    by 0x4B2FB1F: sd_event_dispatch (sd-event.c:4077)
==463==    by 0x4B2FFFA: sd_event_run (sd-event.c:4138)
==463==    by 0x4B301D6: sd_event_loop (sd-event.c:4159)
==463==    by 0x464A24: run (resolved.c:92)
==463==    by 0x464B3C: main (resolved.c:99)
==463==  Address 0x5f409d0 is 32 bytes inside a block of size 72 free'd
==463==    at 0x48410E4: free (vg_replace_malloc.c:755)
==463==    by 0x418EDF: mfree (alloc-util.h:48)
==463==    by 0x4197E8: dns_query_candidate_free (resolved-dns-query.c:67)
==463==    by 0x4198B7: dns_query_candidate_unref (resolved-dns-query.c:70)
==463==    by 0x41A2E3: dns_query_unref_candidates (resolved-dns-query.c:337)
==463==    by 0x41C5FE: dns_query_cname_redirect (resolved-dns-query.c:1028)
==463==    by 0x41CA04: dns_query_process_cname_one (resolved-dns-query.c:1128)
==463==    by 0x41CA80: dns_query_process_cname_many (resolved-dns-query.c:1157)
==463==    by 0x40C0BD: bus_method_resolve_hostname_complete (resolved-bus.c:198)
==463==    by 0x41B312: dns_query_complete (resolved-dns-query.c:562)
==463==    by 0x41C1AC: dns_query_accept (resolved-dns-query.c:922)
==463==    by 0x41C2C4: dns_query_ready (resolved-dns-query.c:955)
==463==    by 0x41A162: dns_query_candidate_notify (resolved-dns-query.c:314)
==463==    by 0x434BD6: dns_transaction_complete (resolved-dns-transaction.c:437)
==463==    by 0x438995: dns_transaction_prepare (resolved-dns-transaction.c:1728)
==463==    by 0x43921D: dns_transaction_go (resolved-dns-transaction.c:1928)
==463==    by 0x419C7C: dns_query_candidate_go (resolved-dns-query.c:163)
==463==    by 0x41A143: dns_query_candidate_notify (resolved-dns-query.c:304)
==463==    by 0x434BD6: dns_transaction_complete (resolved-dns-transaction.c:437)
==463==    by 0x436A0F: dns_transaction_process_dnssec (resolved-dns-transaction.c:976)
==463==    by 0x4378C1: dns_transaction_process_reply (resolved-dns-transaction.c:1387)
==463==    by 0x437CE9: on_dns_packet (resolved-dns-transaction.c:1444)
==463==    by 0x4B2DC9B: source_dispatch (sd-event.c:3512)
==463==    by 0x4B2FB1F: sd_event_dispatch (sd-event.c:4077)
==463==    by 0x4B2FFFA: sd_event_run (sd-event.c:4138)
==463==    by 0x4B301D6: sd_event_loop (sd-event.c:4159)
==463==    by 0x464A24: run (resolved.c:92)
==463==    by 0x464B3C: main (resolved.c:99)
==463==  Block was alloc'd at
==463==    at 0x483E86F: malloc (vg_replace_malloc.c:380)
==463==    by 0x418F81: malloc_multiply (alloc-util.h:96)
==463==    by 0x419378: dns_query_candidate_new (resolved-dns-query.c:23)
==463==    by 0x41B42C: dns_query_add_candidate (resolved-dns-query.c:582)
==463==    by 0x41BB7A: dns_query_go (resolved-dns-query.c:762)
==463==    by 0x40CE3A: bus_method_resolve_hostname (resolved-bus.c:464)
==463==    by 0x4A84B86: method_callbacks_run (bus-objects.c:414)
==463==    by 0x4A87961: object_find_and_run (bus-objects.c:1323)
==463==    by 0x4A87FEE: bus_process_object (bus-objects.c:1443)
==463==    by 0x4AA3434: process_message (sd-bus.c:2964)
==463==    by 0x4AA3623: process_running (sd-bus.c:3006)
==463==    by 0x4AA4110: bus_process_internal (sd-bus.c:3226)
==463==    by 0x4AA41EF: sd_bus_process (sd-bus.c:3253)
==463==    by 0x4AA5343: io_callback (sd-bus.c:3604)
==463==    by 0x4B2DC9B: source_dispatch (sd-event.c:3512)
==463==    by 0x4B2FB1F: sd_event_dispatch (sd-event.c:4077)
==463==    by 0x4B2FFFA: sd_event_run (sd-event.c:4138)
==463==    by 0x4B301D6: sd_event_loop (sd-event.c:4159)
==463==    by 0x464A24: run (resolved.c:92)
==463==    by 0x464B3C: main (resolved.c:99)

Fixes #19376.
2021-05-14 23:18:10 +01:00
Zbigniew Jędrzejewski-Szmek
b8f1045fe7 Revert "tools/make-directive-index: parallelize"
This reverts commit a2031de849da52aa85b7e4326c0112ed7e5b5672.

The patch itself seems OK, but it exposes a bug in lxml or libxml2-2.9.12 which
was just released. This is being resolved in
https://gitlab.gnome.org/GNOME/libxml2/-/issues/255, but it might be while. So
let's revert this for now to unbreak our CI.

Fixes #19601.
2021-05-14 23:16:24 +01:00
2 changed files with 53 additions and 52 deletions

View File

@ -42,6 +42,8 @@ static void dns_query_candidate_stop(DnsQueryCandidate *c) {
assert(c);
/* Detach all the DnsTransactions attached to this query */
while ((t = set_steal_first(c->transactions))) {
set_remove(t->notify_query_candidates, c);
set_remove(t->notify_query_candidates_done, c);
@ -49,21 +51,34 @@ static void dns_query_candidate_stop(DnsQueryCandidate *c) {
}
}
static DnsQueryCandidate* dns_query_candidate_unlink(DnsQueryCandidate *c) {
assert(c);
/* Detach this DnsQueryCandidate from the Query and Scope objects */
if (c->query) {
LIST_REMOVE(candidates_by_query, c->query->candidates, c);
c->query = NULL;
}
if (c->scope) {
LIST_REMOVE(candidates_by_scope, c->scope->query_candidates, c);
c->scope = NULL;
}
return c;
}
static DnsQueryCandidate* dns_query_candidate_free(DnsQueryCandidate *c) {
if (!c)
return NULL;
dns_query_candidate_stop(c);
dns_query_candidate_unlink(c);
set_free(c->transactions);
dns_search_domain_unref(c->search_domain);
if (c->query)
LIST_REMOVE(candidates_by_query, c->query->candidates, c);
if (c->scope)
LIST_REMOVE(candidates_by_scope, c->scope->query_candidates, c);
return mfree(c);
}
@ -105,6 +120,7 @@ static int dns_query_candidate_add_transaction(
int r;
assert(c);
assert(c->query); /* We shan't add transactions to a candidate that has been detached already */
if (key) {
/* Regular lookup with a resource key */
@ -223,6 +239,7 @@ static int dns_query_candidate_setup_transactions(DnsQueryCandidate *c) {
int n = 0, r;
assert(c);
assert(c->query); /* We shan't add transactions to a candidate that has been detached already */
dns_query_candidate_stop(c);
@ -280,6 +297,9 @@ void dns_query_candidate_notify(DnsQueryCandidate *c) {
assert(c);
if (!c->query) /* This candidate has been abandoned, do nothing. */
return;
state = dns_query_candidate_state(c);
if (DNS_TRANSACTION_IS_LIVE(state))
@ -330,11 +350,13 @@ static void dns_query_stop(DnsQuery *q) {
dns_query_candidate_stop(c);
}
static void dns_query_unref_candidates(DnsQuery *q) {
static void dns_query_unlink_candidates(DnsQuery *q) {
assert(q);
while (q->candidates)
dns_query_candidate_unref(q->candidates);
/* Here we drop *our* references to each of the candidates. If we had the only reference, the
* DnsQueryCandidate object will be freed. */
dns_query_candidate_unref(dns_query_candidate_unlink(q->candidates));
}
static void dns_query_reset_answer(DnsQuery *q) {
@ -364,7 +386,7 @@ DnsQuery *dns_query_free(DnsQuery *q) {
LIST_REMOVE(auxiliary_queries, q->auxiliary_for->auxiliary_queries, q);
}
dns_query_unref_candidates(q);
dns_query_unlink_candidates(q);
dns_question_unref(q->question_idna);
dns_question_unref(q->question_utf8);
@ -1025,7 +1047,7 @@ static int dns_query_cname_redirect(DnsQuery *q, const DnsResourceRecord *cname)
dns_question_unref(q->question_utf8);
q->question_utf8 = TAKE_PTR(nq_utf8);
dns_query_unref_candidates(q);
dns_query_unlink_candidates(q);
/* Note that we do *not* reset the answer here, because the answer we previously got might already
* include everything we need, let's check that first */

View File

@ -4,21 +4,18 @@
import sys
import collections
import re
import concurrent.futures
from xml_helper import xml_parse, xml_print, tree
from copy import deepcopy
COLOPHON = '''\
This index contains {count} entries in {sections} sections,
referring to {pages} individual manual pages.
'''
def _extract_directives(page, names):
directive_groups = {name:collections.defaultdict(set) for name in names}
def _extract_directives(directive_groups, formatting, page):
t = xml_parse(page)
section = t.find('./refmeta/manvolnum').text
pagename = t.find('./refmeta/refentrytitle').text
formatting = {}
storopt = directive_groups['options']
for variablelist in t.iterfind('.//variablelist'):
@ -34,7 +31,7 @@ def _extract_directives(page, names):
if text.startswith('-'):
# for options, merge options with and without mandatory arg
text = text.partition('=')[0]
stor[text].add((pagename, section))
stor[text].append((pagename, section))
if text not in formatting:
# use element as formatted display
if name.text[-1] in "= '":
@ -45,7 +42,7 @@ def _extract_directives(page, names):
formatting[text] = name
extra = variablelist.attrib.get('extra-ref')
if extra:
stor[extra].add((pagename, section))
stor[extra].append((pagename, section))
if extra not in formatting:
elt = tree.Element("varname")
elt.text= extra
@ -71,13 +68,13 @@ def _extract_directives(page, names):
name.text = text
if text.endswith('/'):
text = text[:-1]
storfile[text].add((pagename, section))
storfile[text].append((pagename, section))
if text not in formatting:
# use element as formatted display
formatting[text] = name
else:
text = ' '.join(name.itertext())
storfile[text].add((pagename, section))
storfile[text].append((pagename, section))
formatting[text] = name
storfile = directive_groups['constants']
@ -87,7 +84,7 @@ def _extract_directives(page, names):
name.tail = ''
if name.text.startswith('('): # a cast, strip it
name.text = name.text.partition(' ')[2]
storfile[name.text].add((pagename, section))
storfile[name.text].append((pagename, section))
formatting[name.text] = name
storfile = directive_groups['specifiers']
@ -96,30 +93,18 @@ def _extract_directives(page, names):
continue
if name.attrib.get('index') == 'false':
continue
storfile[name.text].add((pagename, section))
storfile[name.text].append((pagename, section))
formatting[name.text] = name
for name in t.iterfind(".//literal[@class='specifiers']"):
storfile[name.text].add((pagename, section))
storfile[name.text].append((pagename, section))
formatting[name.text] = name
# Serialize to allow pickling
formatting = {name:xml_print(value) for name, value in formatting.items()}
return directive_groups, formatting
def extract_directives(arg):
page, names = arg
try:
return _extract_directives(page, names)
except Exception:
raise ValueError("Failed to process {}".format(page))
def _make_section(template, name, directives, formatting):
varlist = template.find(".//*[@id='{}']".format(name))
for varname, manpages in sorted(directives.items()):
entry = tree.SubElement(varlist, 'varlistentry')
term = tree.SubElement(entry, 'term')
display = tree.fromstring(formatting[varname])
display = deepcopy(formatting[varname])
term.append(display)
para = tree.SubElement(tree.SubElement(entry, 'listitem'), 'para')
@ -169,26 +154,20 @@ def make_page(template_path, xml_files):
"Extract directives from xml_files and return XML index tree."
template = xml_parse(template_path)
names = [vl.get('id') for vl in template.iterfind('.//variablelist')]
with concurrent.futures.ProcessPoolExecutor() as pool:
args = ((xml_file, names) for xml_file in xml_files)
results = list(pool.map(extract_directives, args))
directive_groups = {name:collections.defaultdict(set) for name in names}
directive_groups = {name:collections.defaultdict(list)
for name in names}
formatting = {}
for d_g, f in reversed(results):
for group, mapping in d_g.items():
for name, value in mapping.items():
directive_groups[group][name].update(value)
formatting.update(f)
for page in xml_files:
try:
_extract_directives(directive_groups, formatting, page)
except Exception:
raise ValueError("failed to process " + page)
return _make_page(template, directive_groups, formatting)
def main(output, template_path, *xml_files):
with open(output, 'wb') as f:
if __name__ == '__main__':
with open(sys.argv[1], 'wb') as f:
template_path = sys.argv[2]
xml_files = sys.argv[3:]
xml = make_page(template_path, xml_files)
f.write(xml_print(xml))
if __name__ == '__main__':
main(*sys.argv[1:])