From 9e7766754fa002c7c5107413b925cb3091dec070 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Tue, 31 Mar 2026 18:28:41 +0200 Subject: [PATCH 1/9] libcli/dns: Simplify dns over tcp requests, save 39 lines :-) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit tstream_readv_pdu_send() is overkill here, tstream_read_packet_send() is sufficient. The only downside is that dns_tcp_request_recv() does not return a talloc context on its own in reply anymore, but as the reply is immediately ndr-parsed, so we keep that around only a tiny bit longer. Signed-off-by: Volker Lendecke Reviewed-by: Pavel Filipenský (cherry picked from commit a265d18de746a0cc564c4ce64717660d611167fe) BUG: https://bugzilla.samba.org/show_bug.cgi?id=16225 --- libcli/dns/dns.c | 89 ++++++++++++---------------------------- libcli/dns/wscript_build | 2 +- 2 files changed, 28 insertions(+), 63 deletions(-) diff --git a/libcli/dns/dns.c b/libcli/dns/dns.c index 81144601bb89..4539a7713ba8 100644 --- a/libcli/dns/dns.c +++ b/libcli/dns/dns.c @@ -23,6 +23,7 @@ #include "system/network.h" #include #include "lib/tsocket/tsocket.h" +#include "source3/lib/util_tsock.h" #include "libcli/dns/libdns.h" #include "lib/util/tevent_unix.h" #include "lib/util/samba_util.h" @@ -193,11 +194,9 @@ struct dns_tcp_request_state { static void dns_tcp_request_connected(struct tevent_req *subreq); static void dns_tcp_request_sent(struct tevent_req *subreq); -static int dns_tcp_request_next_vector(struct tstream_context *stream, - void *private_data, - TALLOC_CTX *mem_ctx, - struct iovec **_vector, - size_t *_count); +static ssize_t dns_tcp_request_more(uint8_t *buf, + size_t buflen, + void *private_data); static void dns_tcp_request_received(struct tevent_req *subreq); static struct tevent_req *dns_tcp_request_send(TALLOC_CTX *mem_ctx, @@ -297,76 +296,37 @@ static void dns_tcp_request_sent(struct tevent_req *subreq) return; } - subreq = tstream_readv_pdu_send(state, state->ev, state->stream, - dns_tcp_request_next_vector, state); + subreq = tstream_read_packet_send(state, + state->ev, + state->stream, + 2, + dns_tcp_request_more, + NULL); if (tevent_req_nomem(subreq, req)) { return; } tevent_req_set_callback(subreq, dns_tcp_request_received, req); } -static int dns_tcp_request_next_vector(struct tstream_context *stream, - void *private_data, - TALLOC_CTX *mem_ctx, - struct iovec **_vector, - size_t *_count) +static ssize_t dns_tcp_request_more(uint8_t *buf, + size_t buflen, + void *private_data) { - struct dns_tcp_request_state *state = talloc_get_type_abort( - private_data, struct dns_tcp_request_state); - struct iovec *vector; - uint16_t msglen; - - if (state->nread == 0) { - vector = talloc_array(mem_ctx, struct iovec, 1); - if (vector == NULL) { - return -1; - } - vector[0] = (struct iovec) { - .iov_base = state->dns_msglen_hdr, - .iov_len = sizeof(state->dns_msglen_hdr) - }; - state->nread = sizeof(state->dns_msglen_hdr); - - *_vector = vector; - *_count = 1; - return 0; - } - - if (state->nread == sizeof(state->dns_msglen_hdr)) { - msglen = RSVAL(state->dns_msglen_hdr, 0); - - state->reply = talloc_array(state, uint8_t, msglen); - if (state->reply == NULL) { - return -1; - } - - vector = talloc_array(mem_ctx, struct iovec, 1); - if (vector == NULL) { - return -1; - } - vector[0] = (struct iovec) { - .iov_base = state->reply, - .iov_len = msglen - }; - state->nread += msglen; - - *_vector = vector; - *_count = 1; - return 0; + if (buflen > 2) { + return 0; /* We've been here, we're done */ } - - *_vector = NULL; - *_count = 0; - return 0; + return PULL_BE_U16(buf, 0); } static void dns_tcp_request_received(struct tevent_req *subreq) { struct tevent_req *req = tevent_req_callback_data( subreq, struct tevent_req); + struct dns_tcp_request_state *state = tevent_req_data( + req, struct dns_tcp_request_state); int ret, err; - ret = tstream_readv_pdu_recv(subreq, &err); + ret = tstream_read_packet_recv(subreq, state, &state->reply, &err); TALLOC_FREE(subreq); if (ret == -1) { tevent_req_error(req, err); @@ -390,8 +350,14 @@ static int dns_tcp_request_recv(struct tevent_req *req, return err; } - *reply_len = talloc_array_length(state->reply); - *reply = talloc_move(mem_ctx, &state->reply); + /* + * DNS over TCP prefixes each message with a 2-byte length + * header, skip it. + */ + *reply_len = talloc_array_length(state->reply) - 2; + *reply = state->reply + 2; + talloc_steal(mem_ctx, state->reply); + tevent_req_received(req); return 0; @@ -568,7 +534,6 @@ static void dns_cli_request_tcp_done(struct tevent_req *subreq) tevent_req_error(req, ndr_map_error2errno(ndr_err)); return; } - TALLOC_FREE(reply.data); if (state->reply->id != state->req_id) { DBG_DEBUG("Got id %"PRIu16", expected %"PRIu16"\n", diff --git a/libcli/dns/wscript_build b/libcli/dns/wscript_build index 2d90aa7ec0ff..e1e05429a96c 100644 --- a/libcli/dns/wscript_build +++ b/libcli/dns/wscript_build @@ -2,7 +2,7 @@ bld.SAMBA_LIBRARY('clidns', source='dns.c resolvconf.c', - public_deps='LIBTSOCKET tevent-util ndr-standard', + public_deps='LIBTSOCKET util_tsock tevent-util ndr-standard', private_library=True) bld.SAMBA_BINARY('resolvconftest', -- 2.43.0 From ce5ad2611eb73f3f92a26658247939cd03b7d38b Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 20 Apr 2026 15:40:02 +0200 Subject: [PATCH 2/9] libcli/dns: Avoid a local variable referenced only once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Volker Lendecke Reviewed-by: Pavel Filipenský (cherry picked from commit 06623bfd7aab829fcd085340c21afcfd984bd699) BUG: https://bugzilla.samba.org/show_bug.cgi?id=16225 --- libcli/dns/dns.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/libcli/dns/dns.c b/libcli/dns/dns.c index 4539a7713ba8..6fd3667ef382 100644 --- a/libcli/dns/dns.c +++ b/libcli/dns/dns.c @@ -55,7 +55,6 @@ static struct tevent_req *dns_udp_request_send(TALLOC_CTX *mem_ctx, struct tevent_req *req, *subreq; struct dns_udp_request_state *state; struct tsocket_address *local_addr, *server_addr; - struct tdgram_context *dgram; int ret; req = tevent_req_create(mem_ctx, &state, struct dns_udp_request_state); @@ -80,18 +79,21 @@ static struct tevent_req *dns_udp_request_send(TALLOC_CTX *mem_ctx, return tevent_req_post(req, ev); } - ret = tdgram_inet_udp_socket(local_addr, server_addr, state, &dgram); + ret = tdgram_inet_udp_socket(local_addr, + server_addr, + state, + &state->dgram); if (ret != 0) { tevent_req_error(req, errno); return tevent_req_post(req, ev); } - state->dgram = dgram; state->query_len = query_len; dump_data(10, query, query_len); - subreq = tdgram_sendto_send(state, ev, dgram, query, query_len, NULL); + subreq = tdgram_sendto_send( + state, ev, state->dgram, query, query_len, NULL); if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); } -- 2.43.0 From 40a80fbb7e50dd49fb8b7cc388a66b513b930af3 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 20 Apr 2026 15:56:34 +0200 Subject: [PATCH 3/9] libcli/dns: dns_udp_request_get_reply -> dns_udp_request_sent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit More in line with typical async code right now Signed-off-by: Volker Lendecke Reviewed-by: Pavel Filipenský (cherry picked from commit b9f9d5a0dc278499a69740348edc995606397b1b) BUG: https://bugzilla.samba.org/show_bug.cgi?id=16225 --- libcli/dns/dns.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libcli/dns/dns.c b/libcli/dns/dns.c index 6fd3667ef382..d59f483ab0a5 100644 --- a/libcli/dns/dns.c +++ b/libcli/dns/dns.c @@ -43,7 +43,7 @@ struct dns_udp_request_state { #define DNS_REQUEST_TIMEOUT 10 /* Declare callback functions used below. */ -static void dns_udp_request_get_reply(struct tevent_req *subreq); +static void dns_udp_request_sent(struct tevent_req *subreq); static void dns_udp_request_done(struct tevent_req *subreq); static struct tevent_req *dns_udp_request_send(TALLOC_CTX *mem_ctx, @@ -103,11 +103,11 @@ static struct tevent_req *dns_udp_request_send(TALLOC_CTX *mem_ctx, return tevent_req_post(req, ev); } - tevent_req_set_callback(subreq, dns_udp_request_get_reply, req); + tevent_req_set_callback(subreq, dns_udp_request_sent, req); return req; } -static void dns_udp_request_get_reply(struct tevent_req *subreq) +static void dns_udp_request_sent(struct tevent_req *subreq) { struct tevent_req *req = tevent_req_callback_data(subreq, struct tevent_req); -- 2.43.0 From 3865d8b16dd031fca4b90231126ef34827961253 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Wed, 22 Apr 2026 16:41:20 +0200 Subject: [PATCH 4/9] libcli/dns: Make SMB_ASSERT available MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SMB_ASSERT requires "lib/util/debug.h" first and then "lib/util/samba_util.h": 1. #include "lib/util/debug.h" * _SAMBA_DEBUG_H is now defined 2. #include "lib/util/samba_util.h" * samba_util.h includes fault.h * fault.h checks: #ifdef _SAMBA_DEBUG_H (only if yes, SMB_ASSERT gets defined) Signed-off-by: Volker Lendecke Reviewed-by: Pavel Filipenský (cherry picked from commit 8d04671f5ad90f09c46bfbe5a1c88e09ab0e45b7) BUG: https://bugzilla.samba.org/show_bug.cgi?id=16225 --- libcli/dns/dns.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcli/dns/dns.c b/libcli/dns/dns.c index d59f483ab0a5..0c5a3a44d996 100644 --- a/libcli/dns/dns.c +++ b/libcli/dns/dns.c @@ -26,8 +26,8 @@ #include "source3/lib/util_tsock.h" #include "libcli/dns/libdns.h" #include "lib/util/tevent_unix.h" -#include "lib/util/samba_util.h" #include "lib/util/debug.h" +#include "lib/util/samba_util.h" #include "libcli/util/error.h" #include "librpc/ndr/libndr.h" #include "librpc/gen_ndr/ndr_dns.h" -- 2.43.0 From 04e896562c3e435ed3a85a7a4c810dc110e32b35 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Mon, 20 Apr 2026 15:54:59 +0200 Subject: [PATCH 5/9] libcli/dns: Add TCP parallel fallback after 1 second UDP timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Comes with refactoring: Add EDNS0 only to the UDP request, try TCP on any UDP failure. This patch should probably be split up into several micro-commits, but the code is very simply structured, so I'd propose to put this in as is. For review, just look at the final code :-) Signed-off-by: Volker Lendecke Reviewed-by: Pavel Filipenský Autobuild-User(master): Volker Lendecke Autobuild-Date(master): Wed Apr 29 17:40:38 UTC 2026 on atb-devel-224 (cherry picked from commit a55c1c97ac4d015dfe30f58b79e724f28185b14a) BUG: https://bugzilla.samba.org/show_bug.cgi?id=16225 --- libcli/dns/dns.c | 347 +++++++++++++++++++++++++++++++---------------- 1 file changed, 227 insertions(+), 120 deletions(-) diff --git a/libcli/dns/dns.c b/libcli/dns/dns.c index 0c5a3a44d996..2482e1353e7a 100644 --- a/libcli/dns/dns.c +++ b/libcli/dns/dns.c @@ -34,6 +34,7 @@ struct dns_udp_request_state { struct tevent_context *ev; + const struct dns_name_packet *q; struct tdgram_context *dgram; size_t query_len; uint8_t *reply; @@ -46,14 +47,17 @@ struct dns_udp_request_state { static void dns_udp_request_sent(struct tevent_req *subreq); static void dns_udp_request_done(struct tevent_req *subreq); -static struct tevent_req *dns_udp_request_send(TALLOC_CTX *mem_ctx, - struct tevent_context *ev, - const char *server_addr_string, - const uint8_t *query, - size_t query_len) +static struct tevent_req *dns_udp_request_send( + TALLOC_CTX *mem_ctx, + struct tevent_context *ev, + const char *server_addr_string, + const struct dns_name_packet *_q) { struct tevent_req *req, *subreq; struct dns_udp_request_state *state; + struct dns_name_packet *udp_q = NULL; + DATA_BLOB blob = {}; + enum ndr_err_code ndr_err; struct tsocket_address *local_addr, *server_addr; int ret; @@ -63,6 +67,50 @@ static struct tevent_req *dns_udp_request_send(TALLOC_CTX *mem_ctx, } state->ev = ev; + state->q = _q; + + if (_q->arcount == UINT16_MAX) { + tevent_req_error(req, EMSGSIZE); + return tevent_req_post(req, ev); + } + + /* + * Add minimal EDNS0 OPT record to ADDITIONAL section when + * sending a DNS request out, indicating we can accept DNS + * packets up to 4Kb in size. + */ + udp_q = talloc_memdup(state, _q, sizeof(struct dns_name_packet)); + if (tevent_req_nomem(udp_q, req)) { + return tevent_req_post(req, ev); + } + udp_q->additional = talloc_array(udp_q, + struct dns_res_rec, + _q->arcount + 1); + if (tevent_req_nomem(udp_q->additional, req)) { + return tevent_req_post(req, ev); + } + memcpy(udp_q->additional, + _q->additional, + sizeof(struct dns_res_rec) * _q->arcount); + + udp_q->additional[_q->arcount] = (struct dns_res_rec){ + .name = "", + .rr_type = DNS_QTYPE_OPT, + .rr_class = 4096 /* 4096 bytes UDP buffer size */ + }; + udp_q->arcount += 1; + + ndr_err = ndr_push_struct_blob(&blob, + state, + udp_q, + (ndr_push_flags_fn_t) + ndr_push_dns_name_packet); + TALLOC_FREE(udp_q); + + if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { + tevent_req_error(req, ndr_map_error2errno(ndr_err)); + return tevent_req_post(req, ev); + } /* Use connected UDP sockets */ ret = tsocket_address_inet_from_strings(state, "ip", NULL, 0, @@ -88,12 +136,12 @@ static struct tevent_req *dns_udp_request_send(TALLOC_CTX *mem_ctx, return tevent_req_post(req, ev); } - state->query_len = query_len; + state->query_len = blob.length; - dump_data(10, query, query_len); + dump_data(10, blob.data, blob.length); subreq = tdgram_sendto_send( - state, ev, state->dgram, query, query_len, NULL); + state, ev, state->dgram, blob.data, blob.length, NULL); if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); } @@ -141,9 +189,8 @@ static void dns_udp_request_done(struct tevent_req *subreq) { struct tevent_req *req = tevent_req_callback_data(subreq, struct tevent_req); - struct dns_udp_request_state *state = tevent_req_data(req, - struct dns_udp_request_state); - + struct dns_udp_request_state *state = tevent_req_data( + req, struct dns_udp_request_state); ssize_t len; int err = 0; @@ -183,9 +230,9 @@ static int dns_udp_request_recv(struct tevent_req *req, struct dns_tcp_request_state { struct tevent_context *ev; + const struct dns_name_packet *q; struct tstream_context *stream; - const uint8_t *query; - size_t query_len; + DATA_BLOB query; uint8_t dns_msglen_hdr[2]; struct iovec iov[2]; @@ -204,11 +251,11 @@ static void dns_tcp_request_received(struct tevent_req *subreq); static struct tevent_req *dns_tcp_request_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev, const char *server_addr_string, - const uint8_t *query, - size_t query_len) + const struct dns_name_packet *q) { struct tevent_req *req, *subreq; struct dns_tcp_request_state *state; + enum ndr_err_code ndr_err; struct tsocket_address *local, *remote; int ret; @@ -218,10 +265,19 @@ static struct tevent_req *dns_tcp_request_send(TALLOC_CTX *mem_ctx, return NULL; } state->ev = ev; - state->query = query; - state->query_len = query_len; + state->q = q; + + ndr_err = ndr_push_struct_blob(&state->query, + state, + q, + (ndr_push_flags_fn_t) + ndr_push_dns_name_packet); + if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { + tevent_req_error(req, ndr_map_error2errno(ndr_err)); + return tevent_req_post(req, ev); + } - if (query_len > UINT16_MAX) { + if (state->query.length > UINT16_MAX) { tevent_req_error(req, EMSGSIZE); return tevent_req_post(req, ev); } @@ -265,14 +321,14 @@ static void dns_tcp_request_connected(struct tevent_req *subreq) return; } - RSSVAL(state->dns_msglen_hdr, 0, state->query_len); + RSSVAL(state->dns_msglen_hdr, 0, state->query.length); state->iov[0] = (struct iovec) { .iov_base = state->dns_msglen_hdr, .iov_len = sizeof(state->dns_msglen_hdr) }; - state->iov[1] = (struct iovec) { - .iov_base = discard_const_p(void, state->query), - .iov_len = state->query_len + state->iov[1] = (struct iovec){ + .iov_base = discard_const_p(void, state->query.data), + .iov_len = state->query.length, }; subreq = tstream_writev_send(state, state->ev, state->stream, @@ -369,14 +425,17 @@ struct dns_cli_request_state { struct tevent_context *ev; const char *nameserver; - uint16_t req_id; + struct tevent_req *udp_subreq; + struct tevent_req *tcp_subreq; - DATA_BLOB query; + struct dns_name_question question; + struct dns_name_packet out_packet; - struct dns_name_packet *reply; + DATA_BLOB reply; }; static void dns_cli_request_udp_done(struct tevent_req *subreq); +static void dns_cli_request_trigger_tcp(struct tevent_req *subreq); static void dns_cli_request_tcp_done(struct tevent_req *subreq); struct tevent_req *dns_cli_request_send(TALLOC_CTX *mem_ctx, @@ -386,12 +445,8 @@ struct tevent_req *dns_cli_request_send(TALLOC_CTX *mem_ctx, enum dns_qclass qclass, enum dns_qtype qtype) { - struct tevent_req *req, *subreq; + struct tevent_req *req = NULL, *wakeup_subreq = NULL; struct dns_cli_request_state *state; - struct dns_name_question question; - struct dns_name_packet out_packet; - struct dns_res_rec edns0_opt; - enum ndr_err_code ndr_err; req = tevent_req_create(mem_ctx, &state, struct dns_cli_request_state); @@ -404,43 +459,42 @@ struct tevent_req *dns_cli_request_send(TALLOC_CTX *mem_ctx, DBG_DEBUG("Asking %s for %s/%d/%d via UDP\n", nameserver, name, (int)qclass, (int)qtype); - generate_random_buffer((uint8_t *)&state->req_id, - sizeof(state->req_id)); - - question = (struct dns_name_question) { + state->question = (struct dns_name_question){ .name = discard_const_p(char, name), - .question_type = qtype, .question_class = qclass + .question_type = qtype, + .question_class = qclass, }; - edns0_opt = (struct dns_res_rec) { - .name = "", - .rr_type = DNS_QTYPE_OPT, - .rr_class = 4096 /* 4096 bytes UDP buffer size */ - }; - - out_packet = (struct dns_name_packet) { - .id = state->req_id, + state->out_packet = (struct dns_name_packet){ .operation = DNS_OPCODE_QUERY | DNS_FLAG_RECURSION_DESIRED, .qdcount = 1, - .questions = &question, - .arcount = 1, - .additional = &edns0_opt + .questions = &state->question, }; - ndr_err = ndr_push_struct_blob( - &state->query, state, &out_packet, - (ndr_push_flags_fn_t)ndr_push_dns_name_packet); - if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { - tevent_req_error(req, ndr_map_error2errno(ndr_err)); + generate_random_buffer((uint8_t *)&state->out_packet.id, + sizeof(state->out_packet.id)); + + state->udp_subreq = dns_udp_request_send(state, + state->ev, + state->nameserver, + &state->out_packet); + if (tevent_req_nomem(state->udp_subreq, req)) { return tevent_req_post(req, ev); } + tevent_req_set_callback(state->udp_subreq, + dns_cli_request_udp_done, + req); - subreq = dns_udp_request_send(state, state->ev, state->nameserver, - state->query.data, state->query.length); - if (tevent_req_nomem(subreq, req)) { + wakeup_subreq = tevent_wakeup_send(state, + state->ev, + tevent_timeval_current_ofs(1, 0)); + if (tevent_req_nomem(wakeup_subreq, req)) { return tevent_req_post(req, ev); } - tevent_req_set_callback(subreq, dns_cli_request_udp_done, req); + tevent_req_set_callback(wakeup_subreq, + dns_cli_request_trigger_tcp, + req); + return req; } @@ -450,118 +504,171 @@ static void dns_cli_request_udp_done(struct tevent_req *subreq) subreq, struct tevent_req); struct dns_cli_request_state *state = tevent_req_data( req, struct dns_cli_request_state); - DATA_BLOB reply; - enum ndr_err_code ndr_err; uint16_t reply_id, operation; int ret; - ret = dns_udp_request_recv(subreq, state, &reply.data, &reply.length); + SMB_ASSERT(subreq == state->udp_subreq); + + ret = dns_udp_request_recv(subreq, + state, + &state->reply.data, + &state->reply.length); TALLOC_FREE(subreq); - if (tevent_req_error(req, ret)) { - return; + state->udp_subreq = NULL; + + if (ret != 0) { + DBG_DEBUG("dns_udp_request_recv() returned %d (%s)\n", + ret, + strerror(ret)); + goto tcp_fallback; } - if (reply.length < 4) { - DBG_DEBUG("Short DNS packet: length=%zu\n", reply.length); - tevent_req_error(req, EINVAL); - return; + if (state->reply.length < 4) { + DBG_DEBUG("Short DNS packet: length=%zu\n", + state->reply.length); + goto tcp_fallback; } - reply_id = PULL_BE_U16(reply.data, 0); - if (reply_id != state->req_id) { - DBG_DEBUG("Got id %"PRIu16", expected %"PRIu16"\n", - state->reply->id, state->req_id); - tevent_req_error(req, ENOMSG); - return; + reply_id = PULL_BE_U16(state->reply.data, 0); + if (reply_id != state->out_packet.id) { + DBG_DEBUG("Got id %" PRIu16 ", expected %" PRIu16 "\n", + reply_id, + state->out_packet.id); + goto tcp_fallback; } - operation = PULL_BE_U16(reply.data, 2); + operation = PULL_BE_U16(state->reply.data, 2); if ((operation & DNS_FLAG_TRUNCATION) != 0) { - DBG_DEBUG("Reply was truncated, retrying TCP\n"); - subreq = dns_tcp_request_send( - state, - state->ev, - state->nameserver, - state->query.data, - state->query.length); - if (tevent_req_nomem(subreq, req)) { - return; - } - tevent_req_set_callback(subreq, dns_cli_request_tcp_done, req); - return; + DBG_DEBUG("Id %" PRIu16 ", truncated\n", state->out_packet.id); + goto tcp_fallback; } - state->reply = talloc(state, struct dns_name_packet); - if (tevent_req_nomem(state->reply, req)) { + tevent_req_done(req); + return; + +tcp_fallback: + if (state->tcp_subreq != NULL) { return; } - ndr_err = ndr_pull_struct_blob( - &reply, state->reply, state->reply, - (ndr_pull_flags_fn_t)ndr_pull_dns_name_packet); - if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { - tevent_req_error(req, ndr_map_error2errno(ndr_err)); + state->tcp_subreq = dns_tcp_request_send(state, + state->ev, + state->nameserver, + &state->out_packet); + if (tevent_req_nomem(state->tcp_subreq, req)) { return; } - TALLOC_FREE(reply.data); - - tevent_req_done(req); + tevent_req_set_callback(state->tcp_subreq, + dns_cli_request_tcp_done, + req); } -static void dns_cli_request_tcp_done(struct tevent_req *subreq) +static void dns_cli_request_trigger_tcp(struct tevent_req *subreq) { struct tevent_req *req = tevent_req_callback_data( subreq, struct tevent_req); struct dns_cli_request_state *state = tevent_req_data( req, struct dns_cli_request_state); - DATA_BLOB reply; - enum ndr_err_code ndr_err; - int ret; + bool ok; - ret = dns_tcp_request_recv(subreq, state, &reply.data, &reply.length); + ok = tevent_wakeup_recv(subreq); TALLOC_FREE(subreq); - if (tevent_req_error(req, ret)) { + if (!ok) { + tevent_req_oom(subreq); return; } - state->reply = talloc(state, struct dns_name_packet); - if (tevent_req_nomem(state->reply, req)) { + if (state->tcp_subreq != NULL) { return; } - ndr_err = ndr_pull_struct_blob( - &reply, state->reply, state->reply, - (ndr_pull_flags_fn_t)ndr_pull_dns_name_packet); - if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { - tevent_req_error(req, ndr_map_error2errno(ndr_err)); + state->tcp_subreq = dns_tcp_request_send(state, + state->ev, + state->nameserver, + &state->out_packet); + if (tevent_req_nomem(state->tcp_subreq, req)) { return; } + tevent_req_set_callback(state->tcp_subreq, + dns_cli_request_tcp_done, + req); +} + +static void dns_cli_request_tcp_done(struct tevent_req *subreq) +{ + struct tevent_req *req = tevent_req_callback_data(subreq, + struct tevent_req); + struct dns_cli_request_state *state = tevent_req_data( + req, struct dns_cli_request_state); + int ret; + + SMB_ASSERT(subreq == state->tcp_subreq); + + ret = dns_tcp_request_recv(subreq, + state, + &state->reply.data, + &state->reply.length); + TALLOC_FREE(subreq); + state->tcp_subreq = NULL; - if (state->reply->id != state->req_id) { - DBG_DEBUG("Got id %"PRIu16", expected %"PRIu16"\n", - state->reply->id, state->req_id); - tevent_req_error(req, ENOMSG); + if ((ret != 0) && (state->udp_subreq != NULL)) { + DBG_DEBUG("dns_tcp_request_recv() failed: (%s), " + "waiting for UDP\n", + strerror(ret)); return; } - DBG_DEBUG("Got op=%x %"PRIu16"/%"PRIu16"/%"PRIu16"/%"PRIu16 - " recs\n", (int)state->reply->operation, - state->reply->qdcount, state->reply->ancount, - state->reply->nscount, state->reply->nscount); - + if (tevent_req_error(req, ret)) { + return; + } tevent_req_done(req); } -int dns_cli_request_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, - struct dns_name_packet **reply) +int dns_cli_request_recv(struct tevent_req *req, + TALLOC_CTX *mem_ctx, + struct dns_name_packet **_reply) { struct dns_cli_request_state *state = tevent_req_data( req, struct dns_cli_request_state); - int err; + struct dns_name_packet *reply = NULL; + enum ndr_err_code ndr_err; + int ret, err; if (tevent_req_is_unix_error(req, &err)) { return err; } - *reply = talloc_move(mem_ctx, &state->reply); - return 0; + + reply = talloc(mem_ctx, struct dns_name_packet); + if (reply == NULL) { + ret = ENOMEM; + goto done; + } + + ndr_err = ndr_pull_struct_blob(&state->reply, + reply, + reply, + (ndr_pull_flags_fn_t) + ndr_pull_dns_name_packet); + if (!NDR_ERR_CODE_IS_SUCCESS(ndr_err)) { + TALLOC_FREE(reply); + ret = ndr_map_error2errno(ndr_err); + goto done; + } + + if (reply->id != state->out_packet.id) { + DBG_DEBUG("Got id %" PRIu16 ", expected %" PRIu16 "\n", + reply->id, + state->out_packet.id); + ret = ENOMSG; + goto done; + } + + *_reply = reply; + ret = 0; +done: + if (ret != 0) { + TALLOC_FREE(reply); + } + tevent_req_received(req); + return ret; } -- 2.43.0 From f79a212fb14cca5773ac8f02b9c51dc5d6a59d4c Mon Sep 17 00:00:00 2001 From: Shachar Sharon Date: Thu, 30 Apr 2026 14:11:49 +0300 Subject: [PATCH 6/9] libcli/dns: fix use-after-free oom case A failure to tevent_wakeup_recv(subreq) should trigger oom call on 'req' (instead on 'subreq', which has already been free). Signed-off-by: Shachar Sharon Reviewed-by: Noel Power Reviewed-by: Anoop C S Autobuild-User(master): Anoop C S Autobuild-Date(master): Sat May 2 14:51:58 UTC 2026 on atb-devel-224 (cherry picked from commit 1b762a4ea48635c534fe937d60b5e358ff6cab6c) BUG: https://bugzilla.samba.org/show_bug.cgi?id=16225 --- libcli/dns/dns.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcli/dns/dns.c b/libcli/dns/dns.c index 2482e1353e7a..782fdd0d730d 100644 --- a/libcli/dns/dns.c +++ b/libcli/dns/dns.c @@ -574,7 +574,7 @@ static void dns_cli_request_trigger_tcp(struct tevent_req *subreq) ok = tevent_wakeup_recv(subreq); TALLOC_FREE(subreq); if (!ok) { - tevent_req_oom(subreq); + tevent_req_oom(req); return; } -- 2.43.0 From 273fe0d5d01753afe4369d57384bdd6b07ac95bd Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Thu, 23 Apr 2026 10:53:19 +0200 Subject: [PATCH 7/9] libcli/dns: Make dns_cli_request_send() take a dns_name_packet() Factor out dns_cli_create_query(), allow dns_cli_request_send() to be used for other transactions than just DNS_OPCODE_QUERY. Signed-off-by: Volker Lendecke Reviewed-by: Stefan Metzmacher (cherry picked from commit ce1e9ed9b7f9a2258bdd0d97195e7826f28f9a21) BUG: https://bugzilla.samba.org/show_bug.cgi?id=16225 --- libcli/dns/dns.c | 89 +++++++++++++++++++++------------- libcli/dns/dns_lookup.c | 16 +++--- libcli/dns/libdns.h | 9 ++-- source4/dns_server/dns_query.c | 13 +++-- 4 files changed, 77 insertions(+), 50 deletions(-) diff --git a/libcli/dns/dns.c b/libcli/dns/dns.c index 782fdd0d730d..2795ad24ca26 100644 --- a/libcli/dns/dns.c +++ b/libcli/dns/dns.c @@ -428,8 +428,7 @@ struct dns_cli_request_state { struct tevent_req *udp_subreq; struct tevent_req *tcp_subreq; - struct dns_name_question question; - struct dns_name_packet out_packet; + const struct dns_name_packet *q; DATA_BLOB reply; }; @@ -441,9 +440,7 @@ static void dns_cli_request_tcp_done(struct tevent_req *subreq); struct tevent_req *dns_cli_request_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev, const char *nameserver, - const char *name, - enum dns_qclass qclass, - enum dns_qtype qtype) + const struct dns_name_packet *q) { struct tevent_req *req = NULL, *wakeup_subreq = NULL; struct dns_cli_request_state *state; @@ -455,29 +452,12 @@ struct tevent_req *dns_cli_request_send(TALLOC_CTX *mem_ctx, } state->ev = ev; state->nameserver = nameserver; - - DBG_DEBUG("Asking %s for %s/%d/%d via UDP\n", nameserver, - name, (int)qclass, (int)qtype); - - state->question = (struct dns_name_question){ - .name = discard_const_p(char, name), - .question_type = qtype, - .question_class = qclass, - }; - - state->out_packet = (struct dns_name_packet){ - .operation = DNS_OPCODE_QUERY | DNS_FLAG_RECURSION_DESIRED, - .qdcount = 1, - .questions = &state->question, - }; - - generate_random_buffer((uint8_t *)&state->out_packet.id, - sizeof(state->out_packet.id)); + state->q = q; state->udp_subreq = dns_udp_request_send(state, state->ev, state->nameserver, - &state->out_packet); + state->q); if (tevent_req_nomem(state->udp_subreq, req)) { return tevent_req_post(req, ev); } @@ -530,16 +510,16 @@ static void dns_cli_request_udp_done(struct tevent_req *subreq) } reply_id = PULL_BE_U16(state->reply.data, 0); - if (reply_id != state->out_packet.id) { + if (reply_id != state->q->id) { DBG_DEBUG("Got id %" PRIu16 ", expected %" PRIu16 "\n", reply_id, - state->out_packet.id); + state->q->id); goto tcp_fallback; } operation = PULL_BE_U16(state->reply.data, 2); if ((operation & DNS_FLAG_TRUNCATION) != 0) { - DBG_DEBUG("Id %" PRIu16 ", truncated\n", state->out_packet.id); + DBG_DEBUG("Id %" PRIu16 ", truncated\n", state->q->id); goto tcp_fallback; } @@ -554,7 +534,7 @@ tcp_fallback: state->tcp_subreq = dns_tcp_request_send(state, state->ev, state->nameserver, - &state->out_packet); + state->q); if (tevent_req_nomem(state->tcp_subreq, req)) { return; } @@ -585,7 +565,7 @@ static void dns_cli_request_trigger_tcp(struct tevent_req *subreq) state->tcp_subreq = dns_tcp_request_send(state, state->ev, state->nameserver, - &state->out_packet); + state->q); if (tevent_req_nomem(state->tcp_subreq, req)) { return; } @@ -655,20 +635,61 @@ int dns_cli_request_recv(struct tevent_req *req, goto done; } - if (reply->id != state->out_packet.id) { + if (reply->id != state->q->id) { DBG_DEBUG("Got id %" PRIu16 ", expected %" PRIu16 "\n", reply->id, - state->out_packet.id); + state->q->id); ret = ENOMSG; goto done; } *_reply = reply; + reply = NULL; + ret = 0; done: - if (ret != 0) { - TALLOC_FREE(reply); - } + TALLOC_FREE(reply); tevent_req_received(req); return ret; } + +struct dns_name_packet *dns_cli_create_query(TALLOC_CTX *mem_ctx, + const char *name, + enum dns_qclass qclass, + enum dns_qtype qtype) +{ + struct dns_name_packet *p = NULL; + struct dns_name_question *q = NULL; + + p = talloc(mem_ctx, struct dns_name_packet); + if (p == NULL) { + goto fail; + } + + q = talloc(p, struct dns_name_question); + if (q == NULL) { + goto fail; + } + + *q = (struct dns_name_question){ + .name = talloc_strdup(q, name), + .question_class = qclass, + .question_type = qtype, + }; + if (q->name == NULL) { + goto fail; + } + + *p = (struct dns_name_packet){ + .operation = DNS_OPCODE_QUERY | DNS_FLAG_RECURSION_DESIRED, + .qdcount = 1, + .questions = q, + }; + + generate_random_buffer((uint8_t *)&p->id, sizeof(p->id)); + + return p; +fail: + TALLOC_FREE(p); + return NULL; +} diff --git a/libcli/dns/dns_lookup.c b/libcli/dns/dns_lookup.c index 646d0a8b5942..d07905c489ef 100644 --- a/libcli/dns/dns_lookup.c +++ b/libcli/dns/dns_lookup.c @@ -27,9 +27,7 @@ struct dns_lookup_state { struct tevent_context *ev; - const char *name; - enum dns_qclass qclass; - enum dns_qtype qtype; + struct dns_name_packet *q; char **nameservers; size_t num_nameservers; @@ -63,9 +61,11 @@ struct tevent_req *dns_lookup_send(TALLOC_CTX *mem_ctx, return NULL; } state->ev = ev; - state->name = name; - state->qclass = qclass; - state->qtype = qtype; + + state->q = dns_cli_create_query(state, name, qclass, qtype); + if (tevent_req_nomem(state->q, req)) { + return tevent_req_post(req, ev); + } if (resolv_conf_fp == NULL) { const char *resolvconf = "/etc/resolv.conf"; @@ -140,9 +140,7 @@ static int dns_lookup_send_next(struct tevent_req *req) state->dns_subreqs, state->ev, state->nameservers[state->num_sent], - state->name, - state->qclass, - state->qtype); + state->q); if (state->dns_subreqs[state->num_sent] == NULL) { return ENOMEM; diff --git a/libcli/dns/libdns.h b/libcli/dns/libdns.h index 15ca257c0b38..8d00feb31384 100644 --- a/libcli/dns/libdns.h +++ b/libcli/dns/libdns.h @@ -33,11 +33,12 @@ struct tevent_req *dns_cli_request_send(TALLOC_CTX *mem_ctx, struct tevent_context *ev, const char *nameserver, - const char *name, - enum dns_qclass qclass, - enum dns_qtype qtype); + const struct dns_name_packet *q); int dns_cli_request_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, struct dns_name_packet **reply); - +struct dns_name_packet *dns_cli_create_query(TALLOC_CTX *mem_ctx, + const char *name, + enum dns_qclass qclass, + enum dns_qtype qtype); #endif /*__LIBDNS_H__*/ diff --git a/source4/dns_server/dns_query.c b/source4/dns_server/dns_query.c index 2998dbee0603..d0d733bb5b9a 100644 --- a/source4/dns_server/dns_query.c +++ b/source4/dns_server/dns_query.c @@ -282,15 +282,22 @@ static struct tevent_req *ask_forwarder_send( { struct tevent_req *req, *subreq; struct ask_forwarder_state *state; + struct dns_name_packet *q = NULL; req = tevent_req_create(mem_ctx, &state, struct ask_forwarder_state); if (req == NULL) { return NULL; } - subreq = dns_cli_request_send(state, ev, forwarder, - question->name, question->question_class, - question->question_type); + q = dns_cli_create_query(state, + question->name, + question->question_class, + question->question_type); + if (tevent_req_nomem(q, req)) { + return tevent_req_post(req, ev); + } + + subreq = dns_cli_request_send(state, ev, forwarder, q); if (tevent_req_nomem(subreq, req)) { return tevent_req_post(req, ev); } -- 2.43.0 From 68d287cfda9ae698d8ce2082609a8a06284b1558 Mon Sep 17 00:00:00 2001 From: Volker Lendecke Date: Thu, 23 Apr 2026 11:13:17 +0200 Subject: [PATCH 8/9] libcli/dns: Add dns_cli_request() sync wrapper Signed-off-by: Volker Lendecke Reviewed-by: Stefan Metzmacher (cherry picked from commit 562a203fba5de9b3d0fb45b9450fa1bc5d546597) BUG: https://bugzilla.samba.org/show_bug.cgi?id=16225 --- libcli/dns/dns.c | 27 +++++++++++++++++++++++++++ libcli/dns/libdns.h | 4 ++++ 2 files changed, 31 insertions(+) diff --git a/libcli/dns/dns.c b/libcli/dns/dns.c index 2795ad24ca26..717def985bc8 100644 --- a/libcli/dns/dns.c +++ b/libcli/dns/dns.c @@ -653,6 +653,33 @@ done: return ret; } +int dns_cli_request(TALLOC_CTX *mem_ctx, + const char *nameserver, + const struct dns_name_packet *q, + struct dns_name_packet **reply) +{ + TALLOC_CTX *frame = talloc_stackframe(); + struct tevent_context *ev = NULL; + struct tevent_req *req = NULL; + int ret = ENOMEM; + + ev = samba_tevent_context_init(frame); + if (ev == NULL) { + goto fail; + } + req = dns_cli_request_send(frame, ev, nameserver, q); + if (req == NULL) { + goto fail; + } + if (!tevent_req_poll_unix(req, ev, &ret)) { + goto fail; + } + ret = dns_cli_request_recv(req, mem_ctx, reply); +fail: + TALLOC_FREE(frame); + return ret; +} + struct dns_name_packet *dns_cli_create_query(TALLOC_CTX *mem_ctx, const char *name, enum dns_qclass qclass, diff --git a/libcli/dns/libdns.h b/libcli/dns/libdns.h index 8d00feb31384..af5ddd7b519e 100644 --- a/libcli/dns/libdns.h +++ b/libcli/dns/libdns.h @@ -36,6 +36,10 @@ struct tevent_req *dns_cli_request_send(TALLOC_CTX *mem_ctx, const struct dns_name_packet *q); int dns_cli_request_recv(struct tevent_req *req, TALLOC_CTX *mem_ctx, struct dns_name_packet **reply); +int dns_cli_request(TALLOC_CTX *mem_ctx, + const char *nameserver, + const struct dns_name_packet *q, + struct dns_name_packet **reply); struct dns_name_packet *dns_cli_create_query(TALLOC_CTX *mem_ctx, const char *name, enum dns_qclass qclass, -- 2.43.0 From 2029459e6f13849391ad62d58eda5b786a38c8a7 Mon Sep 17 00:00:00 2001 From: Jennifer Sutton Date: Thu, 2 Jul 2026 16:43:39 +1200 Subject: [PATCH 9/9] libcli/dns: Do not pass NULL pointer to memcpy() To do so is to invoke undefined behaviour. Signed-off-by: Jennifer Sutton Reviewed-by: Douglas Bagnall (cherry picked from commit 4e0d3953e9c6974c4c4fe3fe0ee76d86042d9c37) BUG: https://bugzilla.samba.org/show_bug.cgi?id=16225 --- libcli/dns/dns.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/libcli/dns/dns.c b/libcli/dns/dns.c index 717def985bc8..1d285f501339 100644 --- a/libcli/dns/dns.c +++ b/libcli/dns/dns.c @@ -89,9 +89,11 @@ static struct tevent_req *dns_udp_request_send( if (tevent_req_nomem(udp_q->additional, req)) { return tevent_req_post(req, ev); } - memcpy(udp_q->additional, - _q->additional, - sizeof(struct dns_res_rec) * _q->arcount); + if (_q->additional != NULL) { + memcpy(udp_q->additional, + _q->additional, + sizeof(struct dns_res_rec) * _q->arcount); + } udp_q->additional[_q->arcount] = (struct dns_res_rec){ .name = "", -- 2.43.0