From: Vladimír Čunát <vladimir.cunat@nic.cz>
Date: Tue, 18 Apr 2017 10:24:04 +0000 (+0200)
Subject: rrcache: don't clobber pkt if failing the second step
X-Git-Tag: v1.2.6~6^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a42a8af9ecef0cac00ea1be29a35938b2e5f330a;p=thirdparty%2Fknot-resolver.git

rrcache: don't clobber pkt if failing the second step

I hope fixing this bug should diminish the recent experiences of Google
domains failing to resolve on Turris Omnia.
---

diff --git a/lib/layer/rrcache.c b/lib/layer/rrcache.c
index e7f3a0f85..d5d908a3b 100644
--- a/lib/layer/rrcache.c
+++ b/lib/layer/rrcache.c
@@ -117,6 +117,21 @@ static int loot_rrcache(struct kr_cache *cache, knot_pkt_t *pkt,
 	} else if (ret == 0 && dobit) {
 		ret = loot_rr(cache, pkt, qry->sname, qry->sclass, rrtype, qry,
 			      &rank, &flags, true, lowest_rank);
+		if (ret) {
+			VERBOSE_MSG(qry, "=> RRSIG(s) expected but not found, skipping");
+			/* In some cases, e.g. due to bugs, this may fail.
+			 * A possible good example is that a cache backend
+			 * (such as redis) chose to evict RRSIG but not RRset.
+			 * Let's return cache failure, but the packet has been
+			 * updated already by the RRs!  Let's try to clear it.
+			 * The following command might theoretically fail again
+			 * while parsing question, but let's just log that
+			 * condition in non-debug mode (it might be non-fatal). */
+			if (kr_pkt_clear_payload(pkt)) {
+				kr_log_error("[ rc ] => ERROR: work-around failed\n");
+				assert(false);
+			}
+		}
 	}
 	return ret;
 }
@@ -411,7 +426,7 @@ static int rrcache_stash(kr_layer_t *ctx, knot_pkt_t *pkt)
 		if (ret == kr_error(ENOSPC)) {
 			ret = kr_cache_clear(cache);
 			if (ret != 0 && ret != kr_error(EEXIST)) {
-				kr_log_error("[cache] failed to clear cache: %s\n", kr_strerror(ret));
+				kr_log_error("[ rc ] failed to clear cache: %s\n", kr_strerror(ret));
 			}
 		}
 		kr_cache_sync(cache);