ComputerSurge/bind9
3
0
Fork 0
Code Pull Requests 1 Activity

Merge branch '4055-improve-the-overmem-cache-cleaning' into 'security-main'

Browse Source 
Improve RBT overmem cache cleaning

See merge request isc-private/bind9!520
This commit is contained in:
Michal Nowak
2023-06-09 09:50:19 +00:00
parent 282c5605b6 1afde574e9
commit f69434dc47
4 changed files with 86 additions and 49 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CHANGES
+3 -1
View File
@@ -2,7 +2,9 @@
6191. [placeholder]
6190. [placeholder]
6190. [security] Improve the overmem cleaning process to prevent the
cache going over the configured limit. (CVE-2023-2828)
[GL #4055]
6189. [bug] Fix an extra dns_validator deatch when encountering
deadling which would lead to assertion failure.
doc/arm/reference.rst
+5
View File
@@ -3772,6 +3772,11 @@ system.
default value of that option (90% of physical memory for each
individual cache) may lead to memory exhaustion over time.
.. note::
:any:`max-cache-size` does not work reliably for the maximum
amount of memory of 100 MB or lower.
Upon startup and reconfiguration, caches with a limited size
preallocate a small amount of memory (less than 1% of
:any:`max-cache-size` for a given view). This preallocation serves as an
doc/notes/notes-current.rst
+8 -1
View File
@@ -15,7 +15,14 @@ Notes for BIND 9.19.14
Security Fixes
~~~~~~~~~~~~~~
- None.
- The overmem cleaning process has been improved, to prevent the cache from
significantly exceeding the configured :any:`max-cache-size` limit.
(CVE-2023-2828)
ISC would like to thank Shoham Danino from Reichman University, Anat
Bremler-Barr from Tel-Aviv University, Yehuda Afek from Tel-Aviv University,
and Yuval Shavitt from Tel-Aviv University for bringing this vulnerability to
our attention. :gl:`#4055`
New Features
~~~~~~~~~~~~
lib/dns/rbtdb.c
+70 -47
View File
@@ -622,7 +622,7 @@ expire_header(dns_rbtdb_t *rbtdb, rdatasetheader_t *header,
isc_rwlocktype_t *nlocktypep, isc_rwlocktype_t *tlocktypep,
expire_t reason DNS__DB_FLARG);
static void
overmem_purge(dns_rbtdb_t *rbtdb, unsigned int locknum_start, isc_stdtime_t now,
overmem_purge(dns_rbtdb_t *rbtdb, unsigned int locknum_start, size_t purgesize,
isc_rwlocktype_t *tlocktypep DNS__DB_FLARG);
static void
resign_insert(dns_rbtdb_t *rbtdb, int idx, rdatasetheader_t *newheader);
@@ -6878,6 +6878,16 @@ cleanup:
static dns_dbmethods_t zone_methods;
static size_t
rdataset_size(rdatasetheader_t *header) {
if (!NONEXISTENT(header)) {
return (dns_rdataslab_size((unsigned char *)header,
sizeof(*header)));
}
return (sizeof(*header));
}
static isc_result_t
addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
isc_stdtime_t now, dns_rdataset_t *rdataset, unsigned int options,
@@ -7042,7 +7052,7 @@ addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
}
if (cache_is_overmem) {
overmem_purge(rbtdb, rbtnode->locknum, now,
overmem_purge(rbtdb, rbtnode->locknum, rdataset_size(newheader),
&tlocktype DNS__DB_FLARG_PASS);
}
@@ -7062,12 +7072,19 @@ addrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
}
header = isc_heap_element(rbtdb->heaps[rbtnode->locknum], 1);
if (header != NULL &&
header->rdh_ttl + STALE_TTL(header, rbtdb) <
now - RBTDB_VIRTUAL)
{
expire_header(rbtdb, header, &nlocktype, &tlocktype,
expire_ttl DNS__DB_FLARG_PASS);
if (header != NULL) {
dns_ttl_t rdh_ttl = header->rdh_ttl;
/* Only account for stale TTL if cache is not overmem */
if (!cache_is_overmem) {
rdh_ttl += STALE_TTL(header, rbtdb);
}
if (rdh_ttl < now - RBTDB_VIRTUAL) {
expire_header(rbtdb, header, &nlocktype,
&tlocktype,
expire_ttl DNS__DB_FLARG_PASS);
}
}
/*
@@ -9971,54 +9988,61 @@ update_header(dns_rbtdb_t *rbtdb, rdatasetheader_t *header, isc_stdtime_t now) {
ISC_LIST_PREPEND(rbtdb->rdatasets[header->node->locknum], header, link);
}
static size_t
expire_lru_headers(dns_rbtdb_t *rbtdb, unsigned int locknum,
isc_rwlocktype_t *nlocktypep, isc_rwlocktype_t *tlocktypep,
size_t purgesize DNS__DB_FLARG) {
rdatasetheader_t *header, *header_prev;
size_t purged = 0;
for (header = ISC_LIST_TAIL(rbtdb->rdatasets[locknum]);
header != NULL && purged <= purgesize; header = header_prev)
{
header_prev = ISC_LIST_PREV(header, link);
/*
* Unlink the entry at this point to avoid checking it
* again even if it's currently used someone else and
* cannot be purged at this moment. This entry won't be
* referenced any more (so unlinking is safe) since the
* TTL was reset to 0.
*/
ISC_LIST_UNLINK(rbtdb->rdatasets[locknum], header, link);
size_t header_size = rdataset_size(header);
expire_header(rbtdb, header, nlocktypep, tlocktypep,
expire_lru DNS__DB_FLARG_PASS);
purged += header_size;
}
return (purged);
}
/*%
* Purge some expired and/or stale (i.e. unused for some period) cache entries
* under an overmem condition. To recover from this condition quickly, up to
* 2 entries will be purged. This process is triggered while adding a new
* entry, and we specifically avoid purging entries in the same LRU bucket as
* the one to which the new entry will belong. Otherwise, we might purge
* entries of the same name of different RR types while adding RRsets from a
* single response (consider the case where we're adding A and AAAA glue records
* of the same NS name).
* Purge some stale (i.e. unused for some period - LRU based cleaning) cache
* entries under the overmem condition. To recover from this condition quickly,
* we cleanup entries up to the size of newly added rdata (passed as purgesize).
*
* This process is triggered while adding a new entry, and we specifically avoid
* purging entries in the same LRU bucket as the one to which the new entry will
* belong. Otherwise, we might purge entries of the same name of different RR
* types while adding RRsets from a single response (consider the case where
* we're adding A and AAAA glue records of the same NS name).
*/
static void
overmem_purge(dns_rbtdb_t *rbtdb, unsigned int locknum_start, isc_stdtime_t now,
overmem_purge(dns_rbtdb_t *rbtdb, unsigned int locknum_start, size_t purgesize,
isc_rwlocktype_t *tlocktypep DNS__DB_FLARG) {
rdatasetheader_t *header, *header_prev;
unsigned int locknum;
int purgecount = 2;
size_t purged = 0;
for (locknum = (locknum_start + 1) % rbtdb->node_lock_count;
locknum != locknum_start && purgecount > 0;
locknum != locknum_start && purged <= purgesize;
locknum = (locknum + 1) % rbtdb->node_lock_count)
{
isc_rwlocktype_t nlocktype = isc_rwlocktype_none;
NODE_WRLOCK(&rbtdb->node_locks[locknum].lock, &nlocktype);
header = isc_heap_element(rbtdb->heaps[locknum], 1);
if (header && header->rdh_ttl < now - RBTDB_VIRTUAL) {
expire_header(rbtdb, header, &nlocktype, tlocktypep,
expire_ttl DNS__DB_FLARG_PASS);
purgecount--;
}
for (header = ISC_LIST_TAIL(rbtdb->rdatasets[locknum]);
header != NULL && purgecount > 0; header = header_prev)
{
header_prev = ISC_LIST_PREV(header, link);
/*
* Unlink the entry at this point to avoid checking it
* again even if it's currently used someone else and
* cannot be purged at this moment. This entry won't be
* referenced any more (so unlinking is safe) since the
* TTL was reset to 0.
*/
ISC_LIST_UNLINK(rbtdb->rdatasets[locknum], header,
link);
expire_header(rbtdb, header, &nlocktype, tlocktypep,
expire_lru DNS__DB_FLARG_PASS);
purgecount--;
}
purged += expire_lru_headers(
rbtdb, locknum, &nlocktype, tlocktypep,
purgesize - purged DNS__DB_FLARG_PASS);
NODE_UNLOCK(&rbtdb->node_locks[locknum].lock, &nlocktype);
}
@@ -10037,15 +10061,14 @@ expire_header(dns_rbtdb_t *rbtdb, rdatasetheader_t *header,
INSIST(*nlocktypep == isc_rwlocktype_write);
if (isc_refcount_current(&header->node->references) == 0) {
isc_rwlocktype_t nlocktype = isc_rwlocktype_write;
/*
* If no one else is using the node, we can clean it up now.
* We first need to gain a new reference to the node to meet a
* requirement of decrement_reference().
*/
new_reference(rbtdb, header->node,
nlocktype DNS__DB_FLARG_PASS);
decrement_reference(rbtdb, header->node, 0, &nlocktype,
*nlocktypep DNS__DB_FLARG_PASS);
decrement_reference(rbtdb, header->node, 0, nlocktypep,
tlocktypep, true, false DNS__DB_FLARG_PASS);
if (rbtdb->cachestats == NULL) {