]> git.ipfire.org Git - thirdparty/knot-resolver.git/commitdiff
cache entry_list: fix crash on insertion via lua
authorVladimír Čunát <vcunat@gmail.com>
Thu, 24 Oct 2019 08:35:31 +0000 (10:35 +0200)
committerPetr Špaček <petr.spacek@nic.cz>
Wed, 20 Nov 2019 11:59:48 +0000 (12:59 +0100)
When inserting NS or xNAME, we could get into this place with
qry == NULL, and we'd crash when trying to use the memory pool.
Let's simply use the stack instead.

NEWS
lib/cache/entry_list.c
tests/config/cache.test.lua

diff --git a/NEWS b/NEWS
index df81c500d976ceaa650742a5679b27814479b612..b84918a4798a12dc3c12dfbb3f84298a6293cbad 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -7,6 +7,7 @@ Bugfixes
 - systemd: kresd@.service now properly starts after network interfaces
   have been configured with IP addresses after reboot (!884)
 - sendmmsg: improve reliability (!704)
+- cache: fix crash on insertion via lua for NS and CNAME (!889)
 
 Improvements
 ------------
index 4969b5df5a72f34b9584c2bb62a2dae40ce4e6a1..2295ac2096b2443ee19446ec100d370451eebff7 100644 (file)
@@ -268,7 +268,6 @@ int entry_h_splice(
        /* Now we're in trouble.  In some cases, parts of data to be written
         * is an lmdb entry that may be invalidated by our write request.
         * (lmdb does even in-place updates!) Therefore we copy all into a buffer.
-        * (We don't bother deallocating from the mempool.)
         * LATER(optim.): do this only when neccessary, or perhaps another approach.
         * This is also complicated by the fact that the val_new_entry part
         * is to be written *afterwards* by the caller.
@@ -281,13 +280,13 @@ int entry_h_splice(
                .len = entry_list_serial_size(el),
                .data = NULL,
        };
-       void *buf = mm_alloc(&qry->request->pool, val.len);
-       entry_list_memcpy(buf, el);
+       uint8_t buf[val.len];
+       entry_list_memcpy((struct entry_apex *)buf, el);
        ret = cache_write_or_clear(cache, &key, &val, qry);
        if (ret) return kr_error(ret);
        memcpy(val.data, buf, val.len); /* we also copy the "empty" space, but well... */
        val_new_entry->data = (uint8_t *)val.data
-                           + ((uint8_t *)el[i_type].data - (uint8_t *)buf);
+                           + ((uint8_t *)el[i_type].data - buf);
        return kr_ok();
 }
 
index 45ea9edd9d81d9576bc3c6e2b2ecd3e262b2f95b..79d73ba5f10231db566bce25cafcf4a827f980ae 100644 (file)
@@ -43,14 +43,19 @@ local function test_context_cache()
        is(type(c), 'cdata', 'context has a cache object')
        local s = c.stats
        isnt(s.read and s.read_miss and s.write, 'context cache stats works')
-       -- insert a record into cache
+       -- insert A record into cache
        local rdata = '\1\2\3\4'
        local rr = kres.rrset('\3com\0', kres.type.A, kres.class.IN, 66)
        rr:add_rdata(rdata, #rdata)
        local s_write = s.write
-       ok(c:insert(rr, nil, 0, 0), 'cache insertion works')
+       ok(c:insert(rr, nil, 0, 0), 'cache insertion works (A)')
        ok(c:commit(), 'cache commit works')
        isnt(s.write, s_write, 'cache insertion increments counters')
+       -- insert NS record into cache
+       local rr_ns = kres.rrset('\3com\0', kres.type.NS, kres.class.IN, 66)
+       local rdata_ns = todname('c.gtld-servers.net')
+       ok(rr_ns:add_rdata(rdata_ns, #rdata_ns), 'adding rdata works')
+       ok(c:insert(rr_ns, nil, 0), 'cache insertion works (NS)')
 end
 
 return {