utils/cache_gc: fix crashes/assertions on RTT entries

I missed some parts when finishing this.  I should've tested it better.
GC would hit assertions or NULL dereferences when removing entries,
and eventually that would lead to cache overflowing (and getting
cleared).

diff --git a/NEWS b/NEWS
index 5fba50776895f6a332b469bf675f31a75f884afa..8ddd35ae7ba1d451a3018f7452b36d077d7c390b 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -8,6 +8,7 @@ Improvements
 Bugfixes
 --------
 - dnstap module: don't break request resolution on dnstap errors (!1147)
+- cache garbage collector: fix crashes introduced in 5.3.0 (!1153)
 
 
 Knot Resolver 5.3.0 (2021-02-25)

diff --git a/utils/cache_gc/db.c b/utils/cache_gc/db.c
index 31ff147673dfbe86ce68ddec066d98af0d76d657..1acd37e68575554571c5cf97be7d537a947a49b4 100644 (file)
--- a/utils/cache_gc/db.c
+++ b/utils/cache_gc/db.c
@@ -78,8 +78,10 @@ const uint16_t *kr_gc_key_consistent(knot_db_val_t key)
        } else {
                /* find the first double zero in the key */
                for (i = 2; kd[i - 1] || kd[i - 2]; ++i) {
-                       if (i >= key.len)
+                       if (i >= key.len) {
+                               // TODO: assert(!EINVAL) -> kr_assume()
                                return NULL;
+                       }
                }
        }
        // the next character can be used for classification
@@ -94,8 +96,10 @@ const uint16_t *kr_gc_key_consistent(knot_db_val_t key)
                return &NSEC1;
        case '3':
                return &NSEC3;
-       case 'S': // we don't GC the rtt_state entries, at least for now
+       case 'S': // the rtt_state entries are considered inconsistent, at least for now
+               return NULL;
        default:
+               assert(!EINVAL);
                return NULL;
        }
 }

diff --git a/utils/cache_gc/db.h b/utils/cache_gc/db.h
index 9d1bef51b67cd327b888631b5d7c1cc023234d97..15d26caa114c64d866c2be7e76c2743923dfcdb8 100644 (file)
--- a/utils/cache_gc/db.h
+++ b/utils/cache_gc/db.h
@@ -21,6 +21,11 @@ typedef int (*kr_gc_iter_callback)(const knot_db_val_t * key,
 int kr_gc_cache_iter(knot_db_t * knot_db, const  kr_cache_gc_cfg_t *cfg,
                        kr_gc_iter_callback callback, void *ctx);
 
+/** Return RR type corresponding to the key or NULL.
+ *
+ * NULL is returned on unexpected values (those also trigger assertion)
+ * and on other kinds of data in cache (e.g. struct rtt_state).
+ */
 const uint16_t *kr_gc_key_consistent(knot_db_val_t key);
 
 /** Printf a *binary* string in a human-readable way. */

diff --git a/utils/cache_gc/kr_cache_gc.c b/utils/cache_gc/kr_cache_gc.c
index 4d25c8f3f0abba7f7b1430f55657d4d3ee2e5b96..3843d5ca6b562369580c5a80bac16b317532a877 100644 (file)
--- a/utils/cache_gc/kr_cache_gc.c
+++ b/utils/cache_gc/kr_cache_gc.c
@@ -277,8 +277,8 @@ int kr_cache_gc(kr_cache_gc_cfg_t *cfg, kr_cache_gc_state_t **state)
                case KNOT_EOK:
                        deleted_records++;
                        const uint16_t *entry_type = kr_gc_key_consistent(**i);
-                       assert(entry_type != NULL);
-                       rrtypelist_add(&deleted_rrtypes, *entry_type);
+                       if (entry_type != NULL) // some "inconsistent" entries are OK
+                               rrtypelist_add(&deleted_rrtypes, *entry_type);
                        break;
                case KNOT_ENOENT:
                        already_gone++;