4869. [bug] Address some cases where NULL with zero length could

                        be passed to memmove which is undefined behaviour and
                        can lead to bad optimisation. [RT #46888]

(cherry picked from commit fdd8838bf9c4de07372196607f860dd240986577)

diff --git a/CHANGES b/CHANGES
index 2285bfb69020d45c3d0318f1a1175543b466f485..7fb9a321d4de169a7f6405d3e1b3397067782370 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -1,3 +1,7 @@
+4869.  [bug]           Address some cases where NULL with zero length could
+                       be passed to memmove which is undefined behaviour and
+                       can lead to bad optimisation. [RT #46888]
+
 4867.  [cleanup]       Normalize rndc on/off commands (validation and
                        querylog) so they accept the same synonyms
                        for on/off (yes/no, true/false, enable/disable).

diff --git a/lib/dns/catz.c b/lib/dns/catz.c
index 69f862d3ad6231df27c29748716689baf412c209..049907b12e8475ed5ef163cf5c27ebc7c46a39c9 100644 (file)
--- a/lib/dns/catz.c
+++ b/lib/dns/catz.c
@@ -1002,7 +1002,7 @@ catz_process_version(dns_catz_zone_t *zone, dns_rdataset_t *value) {
                result = ISC_R_BADNUMBER;
                goto cleanup;
        }
-       memcpy(t, rdatastr.data, rdatastr.length);
+       memmove(t, rdatastr.data, rdatastr.length);
        t[rdatastr.length] = 0;
        result = isc_parse_uint32(&tversion, t, 10);
        if (result != ISC_R_SUCCESS) {
@@ -1097,7 +1097,7 @@ catz_process_masters(dns_catz_zone_t *zone, dns_ipkeylist_t *ipkl,
                        if (keyname == NULL)
                                return (ISC_R_NOMEMORY);
                        dns_name_init(keyname, 0);
-                       memcpy(keycbuf, rdatastr.data, rdatastr.length);
+                       memmove(keycbuf, rdatastr.data, rdatastr.length);
                        keycbuf[rdatastr.length] = 0;
                        result = dns_name_fromstring(keyname, keycbuf, 0, mctx);
                        if (result != ISC_R_SUCCESS) {
@@ -1126,8 +1126,8 @@ catz_process_masters(dns_catz_zone_t *zone, dns_ipkeylist_t *ipkl,
                        if (value->type == dns_rdatatype_txt)
                                ipkl->keys[i] = keyname;
                        else /* A/AAAA */
-                               memcpy(&ipkl->addrs[i], &sockaddr,
-                                      sizeof(isc_sockaddr_t));
+                               memmove(&ipkl->addrs[i], &sockaddr,
+                                       sizeof(isc_sockaddr_t));
                } else {
                        result = dns_ipkeylist_resize(mctx, ipkl,
                                                      i+1);
@@ -1158,8 +1158,8 @@ catz_process_masters(dns_catz_zone_t *zone, dns_ipkeylist_t *ipkl,
                        if (value->type == dns_rdatatype_txt)
                                ipkl->keys[i] = keyname;
                        else /* A/AAAA */
-                               memcpy(&ipkl->addrs[i], &sockaddr,
-                                      sizeof(isc_sockaddr_t));
+                               memmove(&ipkl->addrs[i], &sockaddr,
+                                       sizeof(isc_sockaddr_t));
                        ipkl->count++;
                }
                return (ISC_R_SUCCESS);
@@ -1251,7 +1251,8 @@ catz_process_apl(dns_catz_zone_t *zone, isc_buffer_t **aclbp,
                result = dns_rdata_apl_current(&rdata_apl, &apl_ent);
                RUNTIME_CHECK(result == ISC_R_SUCCESS);
                memset(buf, 0, sizeof(buf));
-               memcpy(buf, apl_ent.data, apl_ent.length);
+               if (apl_ent.data != NULL && apl_ent.length > 0)
+                       memmove(buf, apl_ent.data, apl_ent.length);
                if (apl_ent.family == 1)
                        isc_netaddr_fromin(&addr, (struct in_addr*) buf);
                else if (apl_ent.family == 2)

diff --git a/lib/dns/diff.c b/lib/dns/diff.c
index eb5e21d1889eb5a11610aa10d33eb4a794cf0c85..47fa026285f0f1064ba3a05b48bbb5406b525770 100644 (file)
--- a/lib/dns/diff.c
+++ b/lib/dns/diff.c
@@ -80,11 +80,16 @@ dns_difftuple_create(isc_mem_t *mctx,
 
        t->ttl = ttl;
 
-       memmove(datap, rdata->data, rdata->length);
        dns_rdata_init(&t->rdata);
        dns_rdata_clone(rdata, &t->rdata);
-       t->rdata.data = datap;
-       datap += rdata->length;
+       if (rdata->data != NULL) {
+               memmove(datap, rdata->data, rdata->length);
+               t->rdata.data = datap;
+               datap += rdata->length;
+       } else {
+               t->rdata.data = NULL;
+               INSIST(rdata->length == 0);
+       }
 
        ISC_LINK_INIT(&t->rdata, link);
        ISC_LINK_INIT(t, link);

diff --git a/lib/dns/ipkeylist.c b/lib/dns/ipkeylist.c
index dadcfd6df7a8e769a6105ed6b5168e3c3fa38b5e..be6d8a7b88d41f43dd4af305dbf097049b687479 100644 (file)
--- a/lib/dns/ipkeylist.c
+++ b/lib/dns/ipkeylist.c
@@ -187,34 +187,42 @@ dns_ipkeylist_resize(isc_mem_t *mctx, dns_ipkeylist_t *ipkl, unsigned int n) {
        if (labels == NULL)
                goto nomemory;
 
-       memmove(addrs, ipkl->addrs, ipkl->allocated * sizeof(isc_sockaddr_t));
-       if (ipkl->addrs != NULL)
+       if (ipkl->addrs != NULL) {
+               memmove(addrs, ipkl->addrs,
+                       ipkl->allocated * sizeof(isc_sockaddr_t));
                isc_mem_put(mctx, ipkl->addrs,
                            ipkl->allocated * sizeof(isc_sockaddr_t));
+       }
        ipkl->addrs = addrs;
        memset(&ipkl->addrs[ipkl->allocated], 0,
               (n - ipkl->allocated) * sizeof(isc_sockaddr_t));
 
-       memmove(dscps, ipkl->dscps, ipkl->allocated * sizeof(isc_dscp_t));
-       if (ipkl->dscps != NULL)
+       if (ipkl->dscps != NULL) {
+               memmove(dscps, ipkl->dscps,
+                       ipkl->allocated * sizeof(isc_dscp_t));
                isc_mem_put(mctx, ipkl->dscps,
                            ipkl->allocated * sizeof(isc_dscp_t));
+       }
        ipkl->dscps = dscps;
        memset(&ipkl->dscps[ipkl->allocated], 0,
               (n - ipkl->allocated) * sizeof(isc_dscp_t));
 
-       memmove(keys, ipkl->keys, ipkl->allocated * sizeof(dns_name_t *));
-       if (ipkl->keys)
+       if (ipkl->keys) {
+               memmove(keys, ipkl->keys,
+                       ipkl->allocated * sizeof(dns_name_t *));
                isc_mem_put(mctx, ipkl->keys,
                            ipkl->allocated * sizeof(dns_name_t *));
+       }
        ipkl->keys = keys;
        memset(&ipkl->keys[ipkl->allocated], 0,
               (n - ipkl->allocated) * sizeof(dns_name_t *));
 
-       memmove(labels, ipkl->labels, ipkl->allocated * sizeof(dns_name_t *));
-       if (ipkl->labels)
+       if (ipkl->labels != NULL) {
+               memmove(labels, ipkl->labels,
+                       ipkl->allocated * sizeof(dns_name_t *));
                isc_mem_put(mctx, ipkl->labels,
                            ipkl->allocated * sizeof(dns_name_t *));
+       }
        ipkl->labels = labels;
        memset(&ipkl->labels[ipkl->allocated], 0,
               (n - ipkl->allocated) * sizeof(dns_name_t *));

diff --git a/lib/dns/tests/dispatch_test.c b/lib/dns/tests/dispatch_test.c
index 9627e7e065bc65f98edaa356f342e5ad382788f9..0b40256f86fed94b6157e31dbc0533e370d2ff9d 100644 (file)
--- a/lib/dns/tests/dispatch_test.c
+++ b/lib/dns/tests/dispatch_test.c
@@ -157,11 +157,11 @@ nameserver(isc_task_t *task, isc_event_t *event) {
        static unsigned char buf1[16];
        static unsigned char buf2[16];
 
-       memcpy(buf1, ev->region.base, 12);
+       memmove(buf1, ev->region.base, 12);
        memset(buf1 + 12, 0, 4);
        buf1[2] |= 0x80;        /* qr=1 */
 
-       memcpy(buf2, ev->region.base, 12);
+       memmove(buf2, ev->region.base, 12);
        memset(buf2 + 12, 1, 4);
        buf2[2] |= 0x80;        /* qr=1 */