Move the count of items in the slabheader from raw data to struct

The count of items was stored in the raw data as first two bytes.
Instead of reading this from the raw header, move the number of the
items into the structure itself.

This needs the flexible member raw[] to be aligned on the size of the
pointer to prevent unaligned access to the start of the header from
rdataset_getheader() function that casts the raw[] to dns_slabheader_t.

diff --git a/lib/dns/include/dns/rdataslab.h b/lib/dns/include/dns/rdataslab.h
index 0a19338dc086b8bd53e85711e9812be7cb9218b2..51a179247b94fbc8da59078a2bec5de188f0e66f 100644 (file)
--- a/lib/dns/include/dns/rdataslab.h
+++ b/lib/dns/include/dns/rdataslab.h
@@ -41,6 +41,7 @@
  *** Imports
  ***/
 
+#include <stdalign.h>
 #include <stdbool.h>
 
 #include <isc/atomic.h>
@@ -100,9 +101,6 @@ struct dns_slabheader {
        unsigned int heap_index;
        isc_heap_t  *heap;
 
-       /* Used for stale refresh */
-       _Atomic(isc_stdtime_t) last_refresh_fail_ts;
-
        dns_slabheader_proof_t *noqname;
        dns_slabheader_proof_t *closest;
 
@@ -138,11 +136,18 @@ struct dns_slabheader {
         */
        unsigned char upper[32];
 
+       /* Used for stale refresh */
+       _Atomic(isc_stdtime_t) last_refresh_fail_ts;
+
+       uint16_t nitems;
+
        /*%
         * Flexible member indicates the address of the raw data
-        * following this header.
+        * following this header.  This needs to be aligned to the
+        * size of the pointer because we cast raw[] to slabheader
+        * in rdataset_getheader().
         */
-       unsigned char raw[];
+       alignas(sizeof(void *)) unsigned char raw[];
 };
 
 enum {

diff --git a/lib/dns/rdataslab.c b/lib/dns/rdataslab.c
index d73e3ce2733a419ffb8a3a44172e8f3ee0f204d0..c56cd3b03bc67d65d2b7106e9e8c52abb1564ef2 100644 (file)
--- a/lib/dns/rdataslab.c
+++ b/lib/dns/rdataslab.c
@@ -104,7 +104,7 @@ compare_rdata(const void *p1, const void *p2) {
 
 static unsigned char *
 newslab(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
-       size_t size) {
+       uint16_t nitems, size_t size) {
        dns_slabheader_t *header = isc_mem_get(mctx, size);
 
        *header = (dns_slabheader_t){
@@ -112,6 +112,7 @@ newslab(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
                .trust = rdataset->trust,
                .expire = rdataset->ttl,
                .dirtylink = ISC_LINK_INITIALIZER,
+               .nitems = nitems,
        };
 
        region->base = (unsigned char *)header;
@@ -131,7 +132,7 @@ makeslab(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
        dns_rdata_t *rdata = NULL;
        unsigned char *rawbuf = NULL;
        unsigned int headerlen = sizeof(dns_slabheader_t);
-       unsigned int buflen = headerlen + 2;
+       unsigned int buflen = headerlen;
        isc_result_t result;
        unsigned int nitems;
        unsigned int nalloc;
@@ -148,7 +149,8 @@ makeslab(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
                dns_slabheader_t *header = rdataset_getheader(rdataset);
                buflen = dns_rdataslab_size(header);
 
-               rawbuf = newslab(rdataset, mctx, region, buflen);
+               rawbuf = newslab(rdataset, mctx, region, header->nitems,
+                                buflen);
 
                INSIST(headerlen <= buflen);
                memmove(rawbuf, (unsigned char *)header + headerlen,
@@ -165,8 +167,7 @@ makeslab(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
                if (rdataset->type != 0) {
                        return ISC_R_FAILURE;
                }
-               rawbuf = newslab(rdataset, mctx, region, buflen);
-               put_uint16(rawbuf, 0);
+               rawbuf = newslab(rdataset, mctx, region, 0, buflen);
                return ISC_R_SUCCESS;
        }
 
@@ -270,8 +271,7 @@ makeslab(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
         * Allocate the memory, set up a buffer, start copying in
         * data.
         */
-       rawbuf = newslab(rdataset, mctx, region, buflen);
-       put_uint16(rawbuf, nitems);
+       rawbuf = newslab(rdataset, mctx, region, nitems, buflen);
 
        for (i = 0; i < nalloc; i++) {
                if (rdata[i].data == &removed) {
@@ -309,33 +309,32 @@ free_rdatas:
 isc_result_t
 dns_rdataslab_fromrdataset(dns_rdataset_t *rdataset, isc_mem_t *mctx,
                           isc_region_t *region, uint32_t maxrrperset) {
-       isc_result_t result;
-
        if (rdataset->type == dns_rdatatype_none &&
            rdataset->covers == dns_rdatatype_none)
        {
                return DNS_R_DISALLOWED;
        }
 
-       result = makeslab(rdataset, mctx, region, maxrrperset);
-       if (result == ISC_R_SUCCESS) {
-               dns_slabheader_t *header = (dns_slabheader_t *)region->base;
+       isc_result_t result = makeslab(rdataset, mctx, region, maxrrperset);
+       if (result != ISC_R_SUCCESS) {
+               return result;
+       }
 
-               if (rdataset->attributes.negative) {
-                       INSIST(rdataset->type == dns_rdatatype_none);
-                       INSIST(rdataset->covers != dns_rdatatype_none);
-                       header->typepair = DNS_TYPEPAIR_VALUE(
-                               rdataset->covers, dns_rdatatype_none);
-               } else {
-                       INSIST(rdataset->type != dns_rdatatype_none);
-                       INSIST(dns_rdatatype_issig(rdataset->type) ||
-                              rdataset->covers == dns_rdatatype_none);
-                       header->typepair = DNS_TYPEPAIR_VALUE(rdataset->type,
-                                                             rdataset->covers);
-               }
+       dns_slabheader_t *header = (dns_slabheader_t *)region->base;
+       if (rdataset->attributes.negative) {
+               INSIST(rdataset->type == dns_rdatatype_none);
+               INSIST(rdataset->covers != dns_rdatatype_none);
+               header->typepair = DNS_TYPEPAIR_VALUE(rdataset->covers,
+                                                     dns_rdatatype_none);
+       } else {
+               INSIST(rdataset->type != dns_rdatatype_none);
+               INSIST(dns_rdatatype_issig(rdataset->type) ||
+                      rdataset->covers == dns_rdatatype_none);
+               header->typepair = DNS_TYPEPAIR_VALUE(rdataset->type,
+                                                     rdataset->covers);
        }
 
-       return result;
+       return ISC_R_SUCCESS;
 }
 
 unsigned int
@@ -347,7 +346,7 @@ dns_rdataslab_size(dns_slabheader_t *header) {
        INSIST(slab != NULL);
 
        unsigned char *current = slab;
-       uint16_t count = get_uint16(current);
+       uint16_t count = header->nitems;
 
        while (count-- > 0) {
                uint16_t length = get_uint16(current);
@@ -361,10 +360,7 @@ unsigned int
 dns_rdataslab_count(dns_slabheader_t *header) {
        REQUIRE(header != NULL);
 
-       unsigned char *current = (unsigned char *)header + sizeof(*header);
-       uint16_t count = get_uint16(current);
-
-       return count;
+       return header->nitems;
 }
 
 /*
@@ -404,10 +400,10 @@ dns_rdataslab_equal(dns_slabheader_t *slab1, dns_slabheader_t *slab2) {
        unsigned int count1, count2;
 
        current1 = (unsigned char *)slab1 + sizeof(dns_slabheader_t);
-       count1 = get_uint16(current1);
+       count1 = slab1->nitems;
 
        current2 = (unsigned char *)slab2 + sizeof(dns_slabheader_t);
-       count2 = get_uint16(current2);
+       count2 = slab2->nitems;
 
        if (count1 != count2) {
                return false;
@@ -438,10 +434,10 @@ dns_rdataslab_equalx(dns_slabheader_t *slab1, dns_slabheader_t *slab2,
        unsigned int count1, count2;
 
        current1 = (unsigned char *)slab1 + sizeof(dns_slabheader_t);
-       count1 = get_uint16(current1);
+       count1 = slab1->nitems;
 
        current2 = (unsigned char *)slab2 + sizeof(dns_slabheader_t);
-       count2 = get_uint16(current2);
+       count2 = slab2->nitems;
 
        if (count1 != count2) {
                return false;
@@ -547,8 +543,10 @@ rdataset_disassociate(dns_rdataset_t *rdataset DNS__DB_FLARG) {
 
 static isc_result_t
 rdataset_first(dns_rdataset_t *rdataset) {
+       dns_slabheader_t *header = rdataset_getheader(rdataset);
        unsigned char *raw = rdataset->slab.raw;
-       uint16_t count = peek_uint16(raw);
+       uint16_t count = header->nitems;
+
        if (count == 0) {
                rdataset->slab.iter_pos = NULL;
                rdataset->slab.iter_count = 0;
@@ -562,7 +560,7 @@ rdataset_first(dns_rdataset_t *rdataset) {
         *
         * 'raw' points to the first record.
         */
-       rdataset->slab.iter_pos = raw + sizeof(uint16_t);
+       rdataset->slab.iter_pos = raw;
        rdataset->slab.iter_count = count - 1;
 
        return ISC_R_SUCCESS;
@@ -634,13 +632,9 @@ rdataset_clone(const dns_rdataset_t *source,
 
 static unsigned int
 rdataset_count(dns_rdataset_t *rdataset) {
-       unsigned char *raw = NULL;
-       unsigned int count;
-
-       raw = rdataset->slab.raw;
-       count = get_uint16(raw);
+       dns_slabheader_t *header = rdataset_getheader(rdataset);
 
-       return count;
+       return header->nitems;
 }
 
 static isc_result_t