cache: fixed crash with RR sets with over 255 records

The previous cache version encoded RR count as uint8_t, which doesn't
work with RR sets with over 255 records. This caused cache writes
to fail and subsequently ending in an assertion failure.
It is not very common to have large RR sets, but it has legitimate
use cases such as a lot of SRV or address records for large container
deployments etc.

diff --git a/lib/cache/api.c b/lib/cache/api.c
index 01cec8e8f9ad21b283bf884ed1d82f1c3574748d..8f584523cb134ade53a5c61f0e6b36248c1c3e2f 100644 (file)
--- a/lib/cache/api.c
+++ b/lib/cache/api.c
@@ -51,7 +51,7 @@
 
 
 /** Cache version */
-static const uint16_t CACHE_VERSION = 2;
+static const uint16_t CACHE_VERSION = 3;
 /** Key size */
 #define KEY_HSIZE (sizeof(uint8_t) + sizeof(uint16_t))
 #define KEY_SIZE (KEY_HSIZE + KNOT_DNAME_MAXLEN)

diff --git a/lib/cache/entry_list.c b/lib/cache/entry_list.c
index 9afdafee7bf16e8a3a3c1e944c46af96ff3cd35d..860ee1b3b47ed87ad939432561730f9996e699ea 100644 (file)
--- a/lib/cache/entry_list.c
+++ b/lib/cache/entry_list.c
@@ -36,9 +36,10 @@ static int entry_h_len(const knot_db_val_t val)
        if (!eh->is_packet) { /* Positive RRset + its RRsig set (may be empty). */
                int sets = 2;
                while (sets-- > 0) {
-                       if (d + 1 > data_bound) return kr_error(EILSEQ);
-                       uint8_t rr_count;
-                       memcpy(&rr_count, d++, sizeof(rr_count));
+                       if (d + 2 > data_bound) return kr_error(EILSEQ);
+                       uint16_t rr_count;
+                       memcpy(&rr_count, d, sizeof(rr_count));
+                       d += sizeof(rr_count);
                        for (int i = 0; i < rr_count; ++i) {
                                if (d + 2 > data_bound) return kr_error(EILSEQ);
                                uint16_t len;

diff --git a/lib/cache/entry_rr.c b/lib/cache/entry_rr.c
index b0019def41f84f4030fe4903c5e804da946da8b9..3d518009ab265dc57603dcd7e69085d71d4b5a64 100644 (file)
--- a/lib/cache/entry_rr.c
+++ b/lib/cache/entry_rr.c
@@ -25,11 +25,12 @@ int rdataset_dematerialize(const knot_rdataset_t *rds, void * restrict data)
 {
        //const void *data0 = data;
        assert(data);
-       if (rds && rds->rr_count > 255) {
-               return kr_error(ENOSPC);
+       if (!data) {
+               return kr_error(EINVAL);
        }
-       uint8_t rr_count = rds ? rds->rr_count : 0;
-       memcpy(data++, &rr_count, sizeof(rr_count));
+       uint16_t rr_count = rds ? rds->rr_count : 0;
+       memcpy(data, &rr_count, sizeof(rr_count));
+       data += sizeof(rr_count);
 
        knot_rdata_t *rd = rds ? rds->data : NULL;
        for (int i = 0; i < rr_count; ++i, rd = kr_rdataset_next(rd)) {
@@ -53,8 +54,9 @@ static int rdataset_materialize(knot_rdataset_t * restrict rds, const void * con
        assert(pool); /* not required, but that's our current usage; guard leaks */
        const void *d = data; /* iterates over the cache data */
        {
-               uint8_t rr_count;
-               memcpy(&rr_count, d++, sizeof(rr_count));
+               uint16_t rr_count;
+               memcpy(&rr_count, d, sizeof(rr_count));
+               d += sizeof(rr_count);
                rds->rr_count = rr_count;
                if (!rr_count) { /* avoid mm_alloc(pool, 0); etc. */
                        return d - data;
@@ -78,7 +80,7 @@ static int rdataset_materialize(knot_rdataset_t * restrict rds, const void * con
                return kr_error(ENOMEM);
        }
        /* Construct the output, one "RR" at a time. */
-       d = data + 1/*sizeof(rr_count)*/;
+       d = data + KR_CACHE_RR_COUNT_SIZE;
        knot_rdata_t *d_out = rds->data; /* iterates over the output being materialized */
        for (int i = 0; i < rds->rr_count; ++i) {
                uint16_t len;

diff --git a/lib/cache/impl.h b/lib/cache/impl.h
index 72e33704a76dc66823597fa703b6e3c690ced263..60ee9fe5dcd33610be8018b5c839c951f7cccaa2 100644 (file)
--- a/lib/cache/impl.h
+++ b/lib/cache/impl.h
@@ -156,10 +156,13 @@ int32_t get_new_ttl(const struct entry_h *entry, const struct kr_query *qry,
 
 /* RRset (de)materialization; implementation in ./entry_rr.c */
 
+/** Size of the RR count field */
+#define KR_CACHE_RR_COUNT_SIZE sizeof(uint16_t)
+
 /** Compute size of dematerialized rdataset.  NULL is accepted as empty set. */
 static inline int rdataset_dematerialize_size(const knot_rdataset_t *rds)
 {
-       return 1/*sizeof(rr_count)*/ + (rds
+       return KR_CACHE_RR_COUNT_SIZE + (rds
                ? knot_rdataset_size(rds) - 4 * rds->rr_count /*TTLs*/
                : 0);
 }

diff --git a/lib/cache/nsec1.c b/lib/cache/nsec1.c
index ce1c39f0dea9cb83928857c937f4017ea811c892..c3e3e2208c90f192783ccc9f418a562e2ca24477 100644 (file)
--- a/lib/cache/nsec1.c
+++ b/lib/cache/nsec1.c
@@ -216,7 +216,7 @@ static const char * find_leq_NSEC1(struct kr_cache *cache, const struct kr_query
        }
        /* We know it starts before sname, so let's check the other end.
         * 1. construct the key for the next name - kwz_hi. */
-       const knot_dname_t *next = eh->data + 3; /* it's *full* name ATM */
+       const knot_dname_t *next = eh->data + 2 * KR_CACHE_RR_COUNT_SIZE; /* it's *full* name ATM */
        if (!eh->data[0]) {
                assert(false);
                return "ERROR";