Skip to content

Commit

Permalink
libtorrent: Fix crash with null TrackerUdp object (#37)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
stickz authored Jul 19, 2024
1 parent fb19a89 commit fd506cb
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 7 deletions.
4 changes: 2 additions & 2 deletions libtorrent/src/torrent/connection_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
6 changes: 6 additions & 0 deletions libtorrent/src/torrent/connection_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ typedef std::pair<Throttle*, Throttle*> ThrottlePair;
// and the int holds the error code.
typedef std::function<void (const sockaddr*, int)> 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)
Expand All @@ -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;
Expand Down
5 changes: 5 additions & 0 deletions libtorrent/src/tracker/tracker_udp.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand Down
14 changes: 10 additions & 4 deletions libtorrent/src/utils/udnsevent.cc
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,11 @@ void a4_callback_wrapper(struct ::dns_ctx *ctx, ::dns_rr_a4 *result, void *data)
udns_query *query = static_cast<udns_query*>(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))); }
Expand All @@ -67,8 +70,11 @@ void a6_callback_wrapper(struct ::dns_ctx *ctx, ::dns_rr_a6 *result, void *data)
udns_query *query = static_cast<udns_query*>(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)));
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 4 additions & 1 deletion libtorrent/src/utils/udnsevent.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand All @@ -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();
Expand Down

0 comments on commit fd506cb

Please sign in to comment.