From: Alan T. DeKok <aland@freeradius.org>
Date: Wed, 13 Oct 2021 12:33:42 +0000 (-0400)
Subject: move dns_labels to thread-local variable
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=68bdbacf473fb6a237d6169e1d88a4ae4f069d0b;p=thirdparty%2Ffreeradius-server.git

move dns_labels to thread-local variable

because we don't yield when decoding the DNS packet.
---

diff --git a/src/listen/dns/proto_dns.c b/src/listen/dns/proto_dns.c
index c732b1f001d..98091b7e14e 100644
--- a/src/listen/dns/proto_dns.c
+++ b/src/listen/dns/proto_dns.c
@@ -197,10 +197,8 @@ static int mod_decode(void const *instance, request_t *request, uint8_t *const d
 	packet_ctx.tmp_ctx = talloc(request, uint8_t);
 	packet_ctx.packet = request->packet->data;
 	packet_ctx.packet_len = data_len;
-
-	packet_ctx.lb = fr_dns_labels_init(packet_ctx.tmp_ctx, packet_ctx.packet, data_len, 0);
+	packet_ctx.lb = fr_dns_labels_get(request->packet->data, data_len, true);
 	fr_assert(packet_ctx.lb != NULL);
-	packet_ctx.lb->mark = talloc_zero_array(packet_ctx.lb, uint8_t, data_len);
 
 	/*
 	 *	Note that we don't set a limit on max_attributes here.
@@ -270,9 +268,8 @@ static ssize_t mod_encode(void const *instance, request_t *request, uint8_t *buf
 	packet_ctx.packet = buffer;
 	packet_ctx.packet_len = buffer_len;
 
-	packet_ctx.lb = fr_dns_labels_init(packet_ctx.tmp_ctx, buffer, buffer_len, 256);
+	packet_ctx.lb = fr_dns_labels_get(buffer, buffer_len, false);
 	fr_assert(packet_ctx.lb != NULL);
-	/* no need to set "mark" here, as that field is only used for decoding */
 
 	data_len = fr_dns_encode(&FR_DBUFF_TMP(buffer, buffer_len), &request->reply_pairs, &packet_ctx);
 	talloc_free(packet_ctx.tmp_ctx);
diff --git a/src/protocols/dns/base.c b/src/protocols/dns/base.c
index 27abe690204..ed629fe200d 100644
--- a/src/protocols/dns/base.c
+++ b/src/protocols/dns/base.c
@@ -39,6 +39,8 @@ typedef struct {
 
 fr_dict_t const *dict_dns;
 
+static _Thread_local fr_dns_labels_t *fr_dns_labels;
+
 extern fr_dict_autoload_t dns_dict[];
 fr_dict_autoload_t dns_dict[] = {
 	{ .out = &dict_dns, .proto = "dns" },
@@ -146,6 +148,34 @@ size_t fr_dns_value_len(fr_pair_t const *vp)
 	}
 }
 
+fr_dns_labels_t *fr_dns_labels_get(uint8_t const *packet, size_t packet_len, bool init_mark)
+{
+	fr_dns_labels_t *lb = fr_dns_labels;
+
+	if (!lb) return NULL;
+
+	lb->start = packet;
+	lb->end = packet + packet_len;
+
+	lb->num = 1;
+	lb->blocks[0].start = DNS_HDR_LEN;
+	lb->blocks[0].end = DNS_HDR_LEN;
+
+	if (init_mark) memset(lb->mark, 0, talloc_array_length(lb->mark));
+
+	return lb;
+}
+
+/** Cleanup the memory pool used by vlog_request
+ *
+ */
+static void _dns_labels_free(void *arg)
+{
+	talloc_free(arg);
+	fr_dns_labels = NULL;
+}
+
+
 /** Resolve/cache attributes in the DNS dictionary
  *
  * @return
@@ -154,6 +184,8 @@ size_t fr_dns_value_len(fr_pair_t const *vp)
  */
 int fr_dns_global_init(void)
 {
+	fr_dns_labels_t *lb;
+
 	if (instance_count > 0) {
 		instance_count++;
 		return 0;
@@ -161,10 +193,24 @@ int fr_dns_global_init(void)
 
 	if (fr_dict_autoload(dns_dict) < 0) return -1;
 	if (fr_dict_attr_autoload(dns_dict_attr) < 0) {
+	fail:
 		fr_dict_autofree(dns_dict);
 		return -1;
 	}
 
+	lb =  (fr_dns_labels_t *) talloc_zero_array(NULL, uint8_t, sizeof(*lb) + sizeof(lb->blocks[0]) * 256);
+	if (!lb) goto fail;
+
+	lb->max = 256;
+
+	lb->mark = talloc_array(lb, uint8_t, 65536);
+	if (!lb->mark) {
+		talloc_free(lb);
+		goto fail;
+	}
+
+	fr_atexit_thread_local(fr_dns_labels, _dns_labels_free, lb);
+
 	instance_count++;
 
 	return 0;
diff --git a/src/protocols/dns/decode.c b/src/protocols/dns/decode.c
index ea2154d22d0..3356aa0e3dc 100644
--- a/src/protocols/dns/decode.c
+++ b/src/protocols/dns/decode.c
@@ -684,28 +684,8 @@ static ssize_t fr_dns_decode_proto(TALLOC_CTX *ctx, fr_pair_list_t *out, uint8_t
 
 	packet_ctx->packet = data;
 	packet_ctx->packet_len = data_len;
-
-	if (packet_ctx->lb) {
-		fr_dns_labels_t *lb = packet_ctx->lb;
-
-		lb->start = data;
-		lb->end = data + data_len;
-
-		/*
-		 *	Always skip the DNS packet header.
-		 */
-		lb->blocks[0].start = 12;
-		lb->blocks[0].end = 12;
-		lb->num = 1;
-
-		memset(packet_ctx->lb->mark, 0, talloc_array_length(packet_ctx->lb->mark));
-	} else {
-		packet_ctx->lb = fr_dns_labels_init(packet_ctx, data, data_len, 256);
-		fr_assert(packet_ctx->lb != NULL);
-
-		packet_ctx->lb->end = data + data_len;
-		packet_ctx->lb->mark = talloc_zero_array(packet_ctx->lb, uint8_t, 65535);
-	}
+	packet_ctx->lb = fr_dns_labels_get(data, data_len, true);
+	fr_assert(packet_ctx->lb != NULL);
 
 	return fr_dns_decode(ctx, out, data, data_len,  packet_ctx);
 }
diff --git a/src/protocols/dns/dns.h b/src/protocols/dns/dns.h
index 0e2ca23b535..34bee267bb5 100644
--- a/src/protocols/dns/dns.h
+++ b/src/protocols/dns/dns.h
@@ -113,6 +113,8 @@ extern char const *fr_dns_packet_codes[FR_DNS_CODE_MAX];
 
 bool fr_dns_packet_ok(uint8_t const *packet, size_t packet_len, bool query);
 
+fr_dns_labels_t *fr_dns_labels_get(uint8_t const *packet, size_t packet_len, bool init_mark);
+
 size_t fr_dns_value_len(fr_pair_t const *vp);
 
 ssize_t	fr_dns_decode(TALLOC_CTX *ctx, fr_pair_list_t *out,
diff --git a/src/protocols/dns/encode.c b/src/protocols/dns/encode.c
index a7736446d90..4a60c797c0a 100644
--- a/src/protocols/dns/encode.c
+++ b/src/protocols/dns/encode.c
@@ -704,8 +704,7 @@ static ssize_t fr_dns_encode_proto(UNUSED TALLOC_CTX *ctx, fr_pair_list_t *vps,
 
 	packet_ctx->packet = data;
 	packet_ctx->packet_len = data_len;
-
-	packet_ctx->lb = fr_dns_labels_init(packet_ctx, data, data_len, 256);
+	packet_ctx->lb = fr_dns_labels_get(data, data_len, false);
 	fr_assert(packet_ctx->lb != NULL);
 
 	slen = fr_dns_encode(&FR_DBUFF_TMP(data, data_len), vps, packet_ctx);