From ddfe8a1bdcd170daaedde8eaec3ad88190e24915 Mon Sep 17 00:00:00 2001 From: Michal Nowak Date: Thu, 4 Jun 2026 12:09:26 +0000 Subject: [PATCH] Preserve the request buffer across async SIG(0) processing For SIG(0)-signed requests, view matching is offloaded and the request is finished asynchronously from ns_client_request_continue(), which passes client->inner.buffer to dns_dt_send(). That buffer aliases the network manager's receive buffer, only valid during the read callback, so it may already be freed and reused, producing garbage dnstap frames (e.g. the "upforwd" sig0-over-DoT test fails with UQ=0). When the request is offloaded (ns_client_setup_view() returns DNS_R_WAIT) and dnstap is enabled, copy the request buffer and point client->inner.buffer at the copy so it survives the asynchronous hop; free it in ns__client_reset_cb(). When dnstap is disabled there is no async consumer of the buffer, so detach it from the receive buffer instead. Assisted-by: Claude:claude-opus-4-8 --- lib/ns/client.c | 33 +++++++++++++++++++++++++++++++++ lib/ns/include/ns/client.h | 20 +++++++++++--------- 2 files changed, 44 insertions(+), 9 deletions(-) diff --git a/lib/ns/client.c b/lib/ns/client.c index 7d2f77e646..9b5e4d5f25 100644 --- a/lib/ns/client.c +++ b/lib/ns/client.c @@ -1722,6 +1722,16 @@ ns__client_reset_cb(void *client0) { client_put_tcp_buffer(client); } + if (client->inner.reqbuf != NULL) { + isc_mem_put(client->manager->mctx, client->inner.reqbuf, + client->inner.reqbuf_size); + client->inner.reqbuf_size = 0; + } + + if (client->inner.buffer != NULL) { + isc_buffer_initnull(client->inner.buffer); + } + if (client->inner.keytag != NULL) { isc_mem_put(client->manager->mctx, client->inner.keytag, client->inner.keytag_len); @@ -2162,6 +2172,29 @@ ns_client_request(isc_nmhandle_t *handle, isc_result_t eresult, result = ns_client_setup_view(client, &netaddr); if (result == DNS_R_WAIT) { +#ifdef HAVE_DNSTAP + /* + * The request is finished asynchronously, but the receive + * buffer is only valid during this callback; copy it so it + * survives the asynchronous hop for dnstap logging. + */ + isc_region_t r; + INSIST(client->inner.reqbuf == NULL); + isc_buffer_usedregion(client->inner.buffer, &r); + if (r.length != 0) { + client->inner.reqbuf = + isc_mem_get(client->manager->mctx, r.length); + client->inner.reqbuf_size = r.length; + memmove(client->inner.reqbuf, r.base, r.length); + isc_buffer_init(&client->inner.tbuffer, + client->inner.reqbuf, r.length); + isc_buffer_add(&client->inner.tbuffer, r.length); + client->inner.buffer = &client->inner.tbuffer; + } +#else + isc_buffer_initnull(client->inner.buffer); +#endif /* #ifdef HAVE_DNSTAP */ + return; } diff --git a/lib/ns/include/ns/client.h b/lib/ns/include/ns/client.h index f4360211af..a6b7c64c66 100644 --- a/lib/ns/include/ns/client.h +++ b/lib/ns/include/ns/client.h @@ -184,15 +184,17 @@ struct ns_client { int16_t ednsversion; /* -1 noedns */ uint16_t additionaldepth; void (*cleanup)(ns_client_t *); - isc_time_t requesttime; - isc_stdtime_t now; - isc_time_t tnow; - dns_name_t signername; /*%< [T]SIG key name */ - dns_name_t *signer; /*%< NULL if not valid sig */ - isc_result_t sigresult; - isc_result_t viewmatchresult; - isc_buffer_t *buffer; - isc_buffer_t tbuffer; + isc_time_t requesttime; + isc_stdtime_t now; + isc_time_t tnow; + dns_name_t signername; /*%< [T]SIG key name */ + dns_name_t *signer; /*%< NULL if not valid sig */ + isc_result_t sigresult; + isc_result_t viewmatchresult; + isc_buffer_t *buffer; + isc_buffer_t tbuffer; + unsigned char *reqbuf; /*%< request copy for async path */ + size_t reqbuf_size; dns_name_t rad; /* Zone rad domain */