1
1
mirror of https://github.com/systemd/systemd-stable.git synced 2024-12-24 21:34:08 +03:00

resolved: disable event sources before unreffing them

We generally operate on the assumption that a source is "gone" as soon
as we unref it. This is generally true because we have the only reference.
But if something else holds the reference, our unref doesn't really stop
the source and it could fire again.

In particular, on_query_timeout() is called with DnsQuery* as userdata, and
it calls dns_query_stop() which invalidates that pointer. If it was ever
called again, we'd be accessing already-freed memory.

I don't see what would hold the reference. sd-event takes a temporary reference,
but on the sd_event object, not on the individual sources. And our sources
are non-floating, so there is no reference from the sd_event object to the
sources.

For #18427.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek 2021-03-01 23:10:06 +01:00
parent f2ec080ef2
commit 9793530228
9 changed files with 22 additions and 22 deletions

View File

@ -326,7 +326,7 @@ static void dns_query_stop(DnsQuery *q) {
assert(q);
q->timeout_event_source = sd_event_source_unref(q->timeout_event_source);
q->timeout_event_source = sd_event_source_disable_unref(q->timeout_event_source);
LIST_FOREACH(candidates_by_query, c, q->candidates)
dns_query_candidate_stop(c);

View File

@ -111,9 +111,9 @@ DnsScope* dns_scope_free(DnsScope *s) {
hashmap_free(s->transactions_by_key);
ordered_hashmap_free_with_destructor(s->conflict_queue, dns_resource_record_unref);
sd_event_source_unref(s->conflict_event_source);
sd_event_source_disable_unref(s->conflict_event_source);
sd_event_source_unref(s->announce_event_source);
sd_event_source_disable_unref(s->announce_event_source);
dns_cache_flush(&s->cache);
dns_zone_flush(&s->zone);
@ -1147,7 +1147,7 @@ static int on_conflict_dispatch(sd_event_source *es, usec_t usec, void *userdata
assert(es);
assert(scope);
scope->conflict_event_source = sd_event_source_unref(scope->conflict_event_source);
scope->conflict_event_source = sd_event_source_disable_unref(scope->conflict_event_source);
for (;;) {
_cleanup_(dns_resource_key_unrefp) DnsResourceKey *key = NULL;
@ -1358,7 +1358,7 @@ static int on_announcement_timeout(sd_event_source *s, usec_t usec, void *userda
assert(s);
scope->announce_event_source = sd_event_source_unref(scope->announce_event_source);
scope->announce_event_source = sd_event_source_disable_unref(scope->announce_event_source);
(void) dns_scope_announce(scope, false);
return 0;

View File

@ -18,8 +18,8 @@
static void dns_stream_stop(DnsStream *s) {
assert(s);
s->io_event_source = sd_event_source_unref(s->io_event_source);
s->timeout_event_source = sd_event_source_unref(s->timeout_event_source);
s->io_event_source = sd_event_source_disable_unref(s->io_event_source);
s->timeout_event_source = sd_event_source_disable_unref(s->timeout_event_source);
s->fd = safe_close(s->fd);
/* Disconnect us from the server object if we are now not usable anymore */

View File

@ -81,8 +81,8 @@ DnsStubListenerExtra *dns_stub_listener_extra_free(DnsStubListenerExtra *p) {
if (!p)
return NULL;
p->udp_event_source = sd_event_source_unref(p->udp_event_source);
p->tcp_event_source = sd_event_source_unref(p->tcp_event_source);
p->udp_event_source = sd_event_source_disable_unref(p->udp_event_source);
p->tcp_event_source = sd_event_source_disable_unref(p->tcp_event_source);
hashmap_free(p->queries_by_packet);
@ -1257,12 +1257,12 @@ int manager_dns_stub_start(Manager *m) {
void manager_dns_stub_stop(Manager *m) {
assert(m);
m->dns_stub_udp_event_source = sd_event_source_unref(m->dns_stub_udp_event_source);
m->dns_stub_tcp_event_source = sd_event_source_unref(m->dns_stub_tcp_event_source);
m->dns_stub_udp_event_source = sd_event_source_disable_unref(m->dns_stub_udp_event_source);
m->dns_stub_tcp_event_source = sd_event_source_disable_unref(m->dns_stub_tcp_event_source);
}
static const char* const dns_stub_listener_mode_table[_DNS_STUB_LISTENER_MODE_MAX] = {
[DNS_STUB_LISTENER_NO] = "no",
[DNS_STUB_LISTENER_NO] = "no",
[DNS_STUB_LISTENER_UDP] = "udp",
[DNS_STUB_LISTENER_TCP] = "tcp",
[DNS_STUB_LISTENER_YES] = "yes",

View File

@ -66,7 +66,7 @@ static void dns_transaction_close_connection(
t->stream = dns_stream_unref(t->stream);
}
t->dns_udp_event_source = sd_event_source_unref(t->dns_udp_event_source);
t->dns_udp_event_source = sd_event_source_disable_unref(t->dns_udp_event_source);
/* If we have an UDP socket where we sent a packet, but never received one, then add it to the socket
* graveyard, instead of closing it right away. That way it will stick around for a moment longer,
@ -87,7 +87,7 @@ static void dns_transaction_close_connection(
static void dns_transaction_stop_timeout(DnsTransaction *t) {
assert(t);
t->timeout_event_source = sd_event_source_unref(t->timeout_event_source);
t->timeout_event_source = sd_event_source_disable_unref(t->timeout_event_source);
}
DnsTransaction* dns_transaction_free(DnsTransaction *t) {

View File

@ -12,16 +12,16 @@
void manager_llmnr_stop(Manager *m) {
assert(m);
m->llmnr_ipv4_udp_event_source = sd_event_source_unref(m->llmnr_ipv4_udp_event_source);
m->llmnr_ipv4_udp_event_source = sd_event_source_disable_unref(m->llmnr_ipv4_udp_event_source);
m->llmnr_ipv4_udp_fd = safe_close(m->llmnr_ipv4_udp_fd);
m->llmnr_ipv6_udp_event_source = sd_event_source_unref(m->llmnr_ipv6_udp_event_source);
m->llmnr_ipv6_udp_event_source = sd_event_source_disable_unref(m->llmnr_ipv6_udp_event_source);
m->llmnr_ipv6_udp_fd = safe_close(m->llmnr_ipv6_udp_fd);
m->llmnr_ipv4_tcp_event_source = sd_event_source_unref(m->llmnr_ipv4_tcp_event_source);
m->llmnr_ipv4_tcp_event_source = sd_event_source_disable_unref(m->llmnr_ipv4_tcp_event_source);
m->llmnr_ipv4_tcp_fd = safe_close(m->llmnr_ipv4_tcp_fd);
m->llmnr_ipv6_tcp_event_source = sd_event_source_unref(m->llmnr_ipv6_tcp_event_source);
m->llmnr_ipv6_tcp_event_source = sd_event_source_disable_unref(m->llmnr_ipv6_tcp_event_source);
m->llmnr_ipv6_tcp_fd = safe_close(m->llmnr_ipv6_tcp_fd);
}

View File

@ -342,7 +342,7 @@ static int manager_clock_change_listen(Manager *m) {
assert(m);
m->clock_change_event_source = sd_event_source_unref(m->clock_change_event_source);
m->clock_change_event_source = sd_event_source_disable_unref(m->clock_change_event_source);
fd = time_change_fd();
if (fd < 0)

View File

@ -15,10 +15,10 @@
void manager_mdns_stop(Manager *m) {
assert(m);
m->mdns_ipv4_event_source = sd_event_source_unref(m->mdns_ipv4_event_source);
m->mdns_ipv4_event_source = sd_event_source_disable_unref(m->mdns_ipv4_event_source);
m->mdns_ipv4_fd = safe_close(m->mdns_ipv4_fd);
m->mdns_ipv6_event_source = sd_event_source_unref(m->mdns_ipv6_event_source);
m->mdns_ipv6_event_source = sd_event_source_disable_unref(m->mdns_ipv6_event_source);
m->mdns_ipv6_fd = safe_close(m->mdns_ipv6_fd);
}

View File

@ -36,7 +36,7 @@ static SocketGraveyard* socket_graveyard_free(SocketGraveyard *g) {
if (g->io_event_source) {
log_debug("Closing graveyard socket fd %i", sd_event_source_get_io_fd(g->io_event_source));
sd_event_source_unref(g->io_event_source);
sd_event_source_disable_unref(g->io_event_source);
}
return mfree(g);