From: Vladimír Čunát <vladimir.cunat@nic.cz>
Date: Fri, 30 Oct 2020 15:29:44 +0000 (+0100)
Subject: modules: fix issues with dropped answers
X-Git-Tag: v5.2.0~1^2~5
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=77f5445f00efbaa331f1b579e0e87ee316adccae;p=thirdparty%2Fknot-resolver.git

modules: fix issues with dropped answers
---

diff --git a/lib/resolve.h b/lib/resolve.h
index d18bd6479..1095fa9ac 100644
--- a/lib/resolve.h
+++ b/lib/resolve.h
@@ -264,7 +264,8 @@ int kr_resolve_begin(struct kr_request *request, struct kr_context *ctx);
  *
  * It may return NULL, in which case it marks ->state with _FAIL and no answer will be sent.
  * Only use this when it's guaranteed that there will be no delay before sending it.
- * You don't need to call this in places where "resolver knows" that there will be no delay.
+ * You don't need to call this in places where "resolver knows" that there will be no delay,
+ * but even there you need to check if the ->answer is NULL (unless you check for _FAIL anyway).
  */
 KR_EXPORT
 knot_pkt_t * kr_request_ensure_answer(struct kr_request *request);
diff --git a/modules/dnstap/dnstap.c b/modules/dnstap/dnstap.c
index 7b1dde55a..b77486011 100644
--- a/modules/dnstap/dnstap.c
+++ b/modules/dnstap/dnstap.c
@@ -136,9 +136,11 @@ static int dnstap_log(kr_layer_t *ctx) {
 
 	if (dnstap_dt->log_resp_pkt) {
 		const knot_pkt_t *rpkt = req->answer;
-		m.response_message.len = rpkt->size;
-		m.response_message.data = (uint8_t *)rpkt->wire;
-		m.has_response_message = true;
+		m.has_response_message = rpkt != NULL;
+		if (rpkt != NULL) {
+			m.response_message.len = rpkt->size;
+			m.response_message.data = (uint8_t *)rpkt->wire;
+		}
 	}
 
 	/* set query time to the timestamp of the first kr_query
diff --git a/modules/policy/policy.lua b/modules/policy/policy.lua
index 999859471..e19d71466 100644
--- a/modules/policy/policy.lua
+++ b/modules/policy/policy.lua
@@ -654,9 +654,14 @@ local debug_logfinish_cb = ffi.cast('trace_callback_f', function (req)
 	ffi.C.kr_log_req(req, 0, 0, 'dbg',
 		'following rrsets were marked as interesting:\n' ..
 		req:selected_tostring())
-	ffi.C.kr_log_req(req, 0, 0, 'dbg',
-		'answer packet:\n' ..
-		tostring(req.answer))
+	if req.answer ~= nil then
+		ffi.C.kr_log_req(req, 0, 0, 'dbg',
+			'answer packet:\n' ..
+			tostring(req.answer))
+	else
+		ffi.C.kr_log_req(req, 0, 0, 'dbg',
+			'answer packet DROPPED\n')
+	end
 end)
 ffi.gc(debug_logfinish_cb, free_cb)
 
diff --git a/modules/stats/stats.c b/modules/stats/stats.c
index a83ead891..132c05c49 100644
--- a/modules/stats/stats.c
+++ b/modules/stats/stats.c
@@ -118,7 +118,7 @@ static inline int collect_key(char *key, const knot_dname_t *name, uint16_t type
 	return key_len + sizeof(type);
 }
 
-static void collect_sample(struct stat_data *data, struct kr_rplan *rplan, knot_pkt_t *pkt)
+static void collect_sample(struct stat_data *data, struct kr_rplan *rplan)
 {
 	/* Sample key = {[2] type, [1-255] owner} */
 	char key[sizeof(uint16_t) + KNOT_DNAME_MAXLEN];
@@ -208,9 +208,14 @@ static int collect(kr_layer_t *ctx)
 	struct kr_rplan *rplan = &param->rplan;
 	struct stat_data *data = module->data;
 
+	collect_sample(data, rplan);
+	if (!param->answer) {
+		/* The answer is being dropped.  TODO: perhaps add some stat for this? */
+		return ctx->state;
+	}
+
 	/* Collect data on final answer */
 	collect_answer(data, param->answer);
-	collect_sample(data, rplan, param->answer);
 	/* Count cached and unresolved */
 	if (rplan->resolved.len > 0) {
 		/* Histogram of answer latency. */
diff --git a/modules/ta_sentinel/ta_sentinel.lua b/modules/ta_sentinel/ta_sentinel.lua
index 2cb04b11a..a0fb67ec8 100644
--- a/modules/ta_sentinel/ta_sentinel.lua
+++ b/modules/ta_sentinel/ta_sentinel.lua
@@ -4,6 +4,7 @@ M.layer = {}
 local ffi = require('ffi')
 
 function M.layer.finish(state, req, pkt)
+	if pkt == nil then return end
 	-- fast filter by the length of the first QNAME label
 	if pkt.wire[5] == 0 then return state end -- QDCOUNT % 256 == 0, in case we produced that
 	local label_len = pkt.wire[12]