Commit 080d4ffc authored by Marek Vavruša's avatar Marek Vavruša

lib/cache: cache fixes, use substruct instead of pointer aliasing

fixes potential cache key oob write
parent b5f3fb7b
......@@ -319,7 +319,7 @@ static int cache_count(lua_State *L)
lua_error(L);
}
lua_pushinteger(L, storage->count((namedb_txn_t *)&txn));
lua_pushinteger(L, storage->count(&txn.t));
kr_cache_txn_abort(&txn);
return 1;
}
......
......@@ -29,7 +29,8 @@
#include "lib/defines.h"
/* Key size */
#define KEY_SIZE (sizeof(uint8_t) + KNOT_DNAME_MAXLEN + sizeof(uint16_t))
#define KEY_HSIZE (1 + sizeof(uint16_t))
#define KEY_SIZE (KEY_HSIZE + KNOT_DNAME_MAXLEN)
#define txn_api(txn) (txn->owner->api)
int kr_cache_open(struct kr_cache *cache, const namedb_api_t *api, void *opts, mm_ctx_t *mm)
......@@ -62,7 +63,7 @@ int kr_cache_txn_begin(struct kr_cache *cache, struct kr_cache_txn *txn, unsigne
return kr_error(EINVAL);
}
/* Open new transaction */
int ret = cache->api->txn_begin(cache->db, (namedb_txn_t *)txn, flags);
int ret = cache->api->txn_begin(cache->db, &txn->t, flags);
if (ret != 0) {
memset(txn, 0, sizeof(*txn));
} else {
......@@ -83,7 +84,7 @@ int kr_cache_txn_commit(struct kr_cache_txn *txn)
return kr_error(EINVAL);
}
int ret = txn_api(txn)->txn_commit((namedb_txn_t *)txn);
int ret = txn_api(txn)->txn_commit(&txn->t);
if (ret != 0) {
kr_cache_txn_abort(txn);
}
......@@ -93,32 +94,38 @@ int kr_cache_txn_commit(struct kr_cache_txn *txn)
void kr_cache_txn_abort(struct kr_cache_txn *txn)
{
if (txn && txn->owner && txn_api(txn)) {
txn_api(txn)->txn_abort((namedb_txn_t *)txn);
txn_api(txn)->txn_abort(&txn->t);
}
}
/** @internal Composed key as { u8 tag, u8[1-255] name, u16 type } */
static size_t cache_key(uint8_t *buf, uint8_t tag, const knot_dname_t *name, uint16_t type)
static size_t cache_key(uint8_t *buf, uint8_t tag, const knot_dname_t *name, uint16_t rrtype)
{
knot_dname_lf(buf, name, NULL);
size_t len = buf[0] + 1;
memcpy(buf + len, &type, sizeof(type));
/* Write tag + type */
buf[0] = tag;
return len + sizeof(type);
memcpy(buf + 1, &rrtype, sizeof(uint16_t));
buf += KEY_HSIZE;
/* Write lowercased name */
int ret = knot_dname_to_wire(buf, name, KNOT_DNAME_MAXLEN);
if (ret <= 0) {
return 0;
}
knot_dname_to_lower(buf);
return KEY_HSIZE + ret;
}
static struct kr_cache_entry *cache_entry(struct kr_cache_txn *txn, uint8_t tag, const knot_dname_t *name, uint16_t type)
{
uint8_t keybuf[KEY_SIZE];
size_t key_len = cache_key(keybuf, tag, name, type);
if (!txn || !txn->owner || !txn_api(txn)) {
if (!txn || !txn->owner || !txn_api(txn) || !name) {
return NULL;
}
/* Look up and return value */
namedb_val_t key = { keybuf, key_len };
namedb_val_t val = { NULL, 0 };
int ret = txn_api(txn)->find((namedb_txn_t *)txn, &key, &val, 0);
int ret = txn_api(txn)->find(&txn->t, &key, &val, 0);
if (ret != KNOT_EOK) {
return NULL;
}
......@@ -129,7 +136,7 @@ static struct kr_cache_entry *cache_entry(struct kr_cache_txn *txn, uint8_t tag,
int kr_cache_peek(struct kr_cache_txn *txn, uint8_t tag, const knot_dname_t *name, uint16_t type,
struct kr_cache_entry **entry, uint32_t *timestamp)
{
if (!txn || !txn->owner || !tag || !name || !entry) {
if (!txn || !txn->owner || !name || !entry) {
return kr_error(EINVAL);
}
......@@ -173,13 +180,16 @@ static void entry_write(struct kr_cache_entry *dst, struct kr_cache_entry *heade
int kr_cache_insert(struct kr_cache_txn *txn, uint8_t tag, const knot_dname_t *name, uint16_t type,
struct kr_cache_entry *header, namedb_val_t data)
{
if (!txn || !txn->owner || !txn_api(txn) || !name || !tag || !header) {
if (!txn || !txn->owner || !txn_api(txn) || !name || !header) {
return kr_error(EINVAL);
}
/* Insert key */
uint8_t keybuf[KEY_SIZE];
size_t key_len = cache_key(keybuf, tag, name, type);
if (key_len == 0) {
return kr_error(EILSEQ);
}
namedb_val_t key = { keybuf, key_len };
namedb_val_t entry = { NULL, sizeof(*header) + data.len };
const namedb_api_t *db_api = txn_api(txn);
......@@ -187,7 +197,7 @@ int kr_cache_insert(struct kr_cache_txn *txn, uint8_t tag, const knot_dname_t *n
/* LMDB can do late write and avoid copy */
txn->owner->stats.insert += 1;
if (db_api == namedb_lmdb_api()) {
int ret = db_api->insert((namedb_txn_t *)txn, &key, &entry, 0);
int ret = db_api->insert(&txn->t, &key, &entry, 0);
if (ret != 0) {
return ret;
}
......@@ -199,7 +209,7 @@ int kr_cache_insert(struct kr_cache_txn *txn, uint8_t tag, const knot_dname_t *n
return kr_error(ENOMEM);
}
entry_write(entry.data, header, data);
int ret = db_api->insert((namedb_txn_t *)txn, &key, &entry, 0);
int ret = db_api->insert(&txn->t, &key, &entry, 0);
free(entry.data);
if (ret != 0) {
return ret;
......@@ -211,15 +221,18 @@ int kr_cache_insert(struct kr_cache_txn *txn, uint8_t tag, const knot_dname_t *n
int kr_cache_remove(struct kr_cache_txn *txn, uint8_t tag, const knot_dname_t *name, uint16_t type)
{
if (!txn || !txn->owner || !txn_api(txn) || !tag || !name ) {
if (!txn || !txn->owner || !txn_api(txn) || !name ) {
return kr_error(EINVAL);
}
uint8_t keybuf[KEY_SIZE];
size_t key_len = cache_key(keybuf, tag, name, type);
if (key_len == 0) {
return kr_error(EILSEQ);
}
namedb_val_t key = { keybuf, key_len };
txn->owner->stats.delete += 1;
return txn_api(txn)->del((namedb_txn_t *)txn, &key);
return txn_api(txn)->del(&txn->t, &key);
}
int kr_cache_clear(struct kr_cache_txn *txn)
......@@ -228,7 +241,7 @@ int kr_cache_clear(struct kr_cache_txn *txn)
return kr_error(EINVAL);
}
return txn_api(txn)->clear((namedb_txn_t *)txn);
return txn_api(txn)->clear(&txn->t);
}
int kr_cache_peek_rr(struct kr_cache_txn *txn, knot_rrset_t *rr, uint32_t *timestamp)
......
......@@ -57,7 +57,7 @@ struct kr_cache
/** Cache transaction */
struct kr_cache_txn {
namedb_txn_t txn; /**< Storage transaction */
namedb_txn_t t; /**< Storage transaction */
struct kr_cache *owner; /**< Transaction owner */
};
......
......@@ -155,9 +155,17 @@ static int merge_rr(knot_rrset_t *cache_rr, const knot_rrset_t *rr, mm_ctx_t *po
static int stash_add(map_t *stash, const knot_rrset_t *rr, mm_ctx_t *pool)
{
/* Stash key = {[1-5] type, [1-255] owner, [1] \x00 } */
char key[6 + KNOT_DNAME_MAXLEN];
sprintf(key, "%hu%*s", rr->type, knot_dname_size(rr->owner), rr->owner);
/* Stash key = {[1-255] owner, [1-5] type, [1] \x00 } */
char key[8 + KNOT_DNAME_MAXLEN];
int ret = knot_dname_to_wire((uint8_t *)key, rr->owner, KNOT_DNAME_MAXLEN);
if (ret <= 0) {
return ret;
}
knot_dname_to_lower((uint8_t *)key);
ret = snprintf(key + ret - 1, sizeof(key) - KNOT_DNAME_MAXLEN, "%hu", rr->type);
if (ret <= 0 || ret >= KNOT_DNAME_MAXLEN) {
return kr_error(EILSEQ);
}
/* Check if already exists */
knot_rrset_t *stashed = map_get(stash, key);
......@@ -252,14 +260,25 @@ static int stash(knot_layer_t *ctx, knot_pkt_t *pkt)
/* Open write transaction */
struct kr_cache *cache = &req->ctx->cache;
struct kr_cache_txn txn;
if (kr_cache_txn_begin(cache, &txn, 0) != 0) {
return ctx->state; /* Couldn't acquire cache, ignore. */
if (kr_cache_txn_begin(cache, &txn, 0) == 0) {
ret = stash_commit(&stash, qry->timestamp.tv_sec, &txn);
if (ret == 0) {
kr_cache_txn_commit(&txn);
} else {
kr_cache_txn_abort(&txn);
}
}
ret = stash_commit(&stash, qry->timestamp.tv_sec, &txn);
/* Clear if full */
if (ret == KNOT_ESPACE) {
kr_cache_clear(&txn);
if (kr_cache_txn_begin(cache, &txn, 0) == 0) {
ret = kr_cache_clear(&txn);
if (ret == 0) {
kr_cache_txn_commit(&txn);
} else {
kr_cache_txn_abort(&txn);
}
}
}
kr_cache_txn_commit(&txn);
}
return ctx->state;
......
......@@ -248,7 +248,7 @@ static int fetch_ns(struct kr_context *ctx, struct kr_zonecut *cut, const knot_d
int kr_zonecut_find_cached(struct kr_context *ctx, struct kr_zonecut *cut, const knot_dname_t *name,
struct kr_cache_txn *txn, uint32_t timestamp)
{
if (cut == NULL) {
if (!ctx || !cut || !name) {
return kr_error(EINVAL);
}
......
......@@ -100,8 +100,10 @@ static inline void test_random_rr(knot_rrset_t *rr, uint32_t ttl)
uint8_t tmp_buf[KNOT_DNAME_MAXLEN];
/* Create random label. */
owner_buf[0] = num;
test_randstr((char *)(owner_buf + 1), owner_buf[0] + 1);
memset(owner_buf, 0, sizeof(owner_buf));
uint8_t label_len = num % KNOT_DNAME_MAXLABELLEN;
owner_buf[0] = label_len;
test_randstr((char *)(owner_buf + 1), label_len);
/* Create payload */
tmp_buf[0] = num;
......
......@@ -265,17 +265,13 @@ static void test_invalid(void **state)
assert_int_not_equal(kr_cache_txn_begin(*state, NULL, 0), 0);
assert_int_not_equal(kr_cache_txn_commit(NULL), 0);
assert_int_not_equal(kr_cache_peek(NULL, KR_CACHE_USER, dname, KNOT_RRTYPE_TSIG, NULL, &timestamp), 0);
assert_int_not_equal(kr_cache_peek(&global_txn, 0, dname, KNOT_RRTYPE_TSIG, &entry, &timestamp), 0);
assert_int_not_equal(kr_cache_peek(&global_txn, KR_CACHE_USER, NULL, KNOT_RRTYPE_TSIG, &entry, &timestamp), 0);
assert_int_not_equal(kr_cache_peek_rr(NULL, NULL, NULL), 0);
assert_int_not_equal(kr_cache_peek_rr(&global_txn, NULL, NULL), 0);
assert_int_not_equal(kr_cache_insert_rr(&global_txn, NULL, 0), 0);
assert_int_not_equal(kr_cache_insert_rr(NULL, NULL, 0), 0);
assert_int_equal(kr_cache_insert_rr(&global_txn, &global_rr, 0), 0);
assert_int_not_equal(kr_cache_insert(NULL, KR_CACHE_USER, dname,
KNOT_RRTYPE_TSIG, &global_fake_ce, global_namedb_data), 0);
assert_int_not_equal(kr_cache_insert(&global_txn, 0, dname,
KNOT_RRTYPE_TSIG, &global_fake_ce, global_namedb_data), 0);
assert_int_not_equal(kr_cache_insert(&global_txn, KR_CACHE_USER, NULL,
KNOT_RRTYPE_TSIG, &global_fake_ce, global_namedb_data), 0);
assert_int_not_equal(kr_cache_insert(&global_txn, KR_CACHE_USER, dname,
......
......@@ -105,6 +105,7 @@ static void test_deinit(void **state)
{
lru_int_t *lru = *state;
lru_deinit(lru);
free(lru);
}
/* Program entry point */
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment