From fd506cbf424ba12738ea10ce7bcf2c26d01221f4 Mon Sep 17 00:00:00 2001 From: stickz Date: Fri, 19 Jul 2024 10:31:11 -0400 Subject: [PATCH] libtorrent: Fix crash with null TrackerUdp object (#37) This commit sends a pointer for TrackerUdp object (which holds the callback for the UDNS query) to libudns then back again. On arrival back to libtorrent, it checks if the pointer to the TrackerUdp object is valid before calling the UDNS callback. This avoids a crash by not trying to invoke a function which is no longer valid. --- libtorrent/src/torrent/connection_manager.cc | 4 ++-- libtorrent/src/torrent/connection_manager.h | 6 ++++++ libtorrent/src/tracker/tracker_udp.cc | 5 +++++ libtorrent/src/utils/udnsevent.cc | 14 ++++++++++---- libtorrent/src/utils/udnsevent.h | 5 ++++- 5 files changed, 27 insertions(+), 7 deletions(-) diff --git a/libtorrent/src/torrent/connection_manager.cc b/libtorrent/src/torrent/connection_manager.cc index f1a5778a..f3d0097e 100644 --- a/libtorrent/src/torrent/connection_manager.cc +++ b/libtorrent/src/torrent/connection_manager.cc @@ -61,8 +61,8 @@ class UdnsAsyncResolver : public AsyncResolver { public: UdnsAsyncResolver(ConnectionManager *cm) : AsyncResolver(cm) {} - void *enqueue(const char *name, int family, resolver_callback *cbck) { - return m_udnsevent.enqueue_resolve(name, family, cbck); + void *enqueue(const char *name, int family, resolver_callback *cbck, TrackerUdp *pointer) { + return m_udnsevent.enqueue_resolve(name, family, cbck, pointer); } void flush() { diff --git a/libtorrent/src/torrent/connection_manager.h b/libtorrent/src/torrent/connection_manager.h index bed441ca..50f4c145 100644 --- a/libtorrent/src/torrent/connection_manager.h +++ b/libtorrent/src/torrent/connection_manager.h @@ -59,6 +59,8 @@ typedef std::pair ThrottlePair; // and the int holds the error code. typedef std::function resolver_callback; +class TrackerUdp; + // Encapsulates whether we do genuine async resolution or fall back to sync. // In a build with USE_UDNS, these do genuine asynchronous DNS resolution. // In a build without it, they're stubbed out to use a synchronous getaddrinfo(3) @@ -70,7 +72,11 @@ class LIBTORRENT_EXPORT AsyncResolver { // this queues a DNS resolve but doesn't send it. it doesn't execute any callbacks // and returns control immediately. the return value is an opaque identifier that // can be used to cancel the query (as long as the callback hasn't been executed yet): +#ifdef USE_UDNS + virtual void* enqueue(const char *name, int family, resolver_callback *cbck, TrackerUdp *pointer) = 0; +#else virtual void* enqueue(const char *name, int family, resolver_callback *cbck) = 0; +#endif // this sends any queued resolves. it can execute arbitrary callbacks // before returning control: virtual void flush() = 0; diff --git a/libtorrent/src/tracker/tracker_udp.cc b/libtorrent/src/tracker/tracker_udp.cc index e50b51bd..e6aac90c 100644 --- a/libtorrent/src/tracker/tracker_udp.cc +++ b/libtorrent/src/tracker/tracker_udp.cc @@ -101,7 +101,12 @@ TrackerUdp::send_state(int state) { m_resolver_query = manager->connection_manager()->async_resolver().enqueue( m_hostname.c_str(), AF_UNSPEC, +#ifdef USE_UDNS + &m_resolver_callback, + this +#else &m_resolver_callback +#endif ); manager->connection_manager()->async_resolver().flush(); } diff --git a/libtorrent/src/utils/udnsevent.cc b/libtorrent/src/utils/udnsevent.cc index c92aac2a..56c9294b 100644 --- a/libtorrent/src/utils/udnsevent.cc +++ b/libtorrent/src/utils/udnsevent.cc @@ -42,8 +42,11 @@ void a4_callback_wrapper(struct ::dns_ctx *ctx, ::dns_rr_a4 *result, void *data) udns_query *query = static_cast(data); // udns will free the a4_query after this callback exits query->a4_query = NULL; + + if (query->pointer == NULL) + delete query; - if (result == NULL || result->dnsa4_nrr == 0) { + else if (result == NULL || result->dnsa4_nrr == 0) { if (query->a6_query == NULL) { // nothing more to do: call the callback with a failure status if (*query->callback) { (*(query->callback))(NULL, udnserror_to_gaierror(::dns_status(ctx))); } @@ -67,8 +70,11 @@ void a6_callback_wrapper(struct ::dns_ctx *ctx, ::dns_rr_a6 *result, void *data) udns_query *query = static_cast(data); // udns will free the a6_query after this callback exits query->a6_query = NULL; + + if (query->pointer == NULL) + delete query; - if (result == NULL || result->dnsa6_nrr == 0) { + else if (result == NULL || result->dnsa6_nrr == 0) { if (query->a4_query == NULL) { // nothing more to do: call the callback with a failure status (*(query->callback))(NULL, udnserror_to_gaierror(::dns_status(ctx))); @@ -121,8 +127,8 @@ void UdnsEvent::event_write() { void UdnsEvent::event_error() { } -struct udns_query *UdnsEvent::enqueue_resolve(const char *name, int family, resolver_callback *callback) { - struct udns_query *query = new udns_query { NULL, NULL, callback, 0 }; +struct udns_query *UdnsEvent::enqueue_resolve(const char *name, int family, resolver_callback *callback, TrackerUdp *pointer) { + struct udns_query *query = new udns_query { NULL, NULL, callback, pointer, 0 }; if (family == AF_INET || family == AF_UNSPEC) { query->a4_query = ::dns_submit_a4(m_ctx, name, 0, a4_callback_wrapper, query); diff --git a/libtorrent/src/utils/udnsevent.h b/libtorrent/src/utils/udnsevent.h index f214814e..fce93f9d 100644 --- a/libtorrent/src/utils/udnsevent.h +++ b/libtorrent/src/utils/udnsevent.h @@ -15,10 +15,13 @@ struct dns_query; namespace torrent { +class TrackerUdp; + struct udns_query { ::dns_query *a4_query; ::dns_query *a6_query; resolver_callback *callback; + TrackerUdp *pointer; int error; }; @@ -37,7 +40,7 @@ class UdnsEvent : public Event { // wraps udns's dns_submit_a[46] functions. they and it return control immediately, // without either sending outgoing UDP packets or executing callbacks: - udns_query* enqueue_resolve(const char *name, int family, resolver_callback *callback); + udns_query* enqueue_resolve(const char *name, int family, resolver_callback *callback, TrackerUdp *pointer); // wraps the dns_timeouts function. it sends packets and can execute arbitrary // callbacks: void flush_resolves();