upstream/bind9
1
0
Fork 0
mirror of https://github.com/isc-projects/bind9.git synced 2026-05-28 04:34:54 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Copy only the raw data when we are copying dns_slab{header,vec}

Browse source 
The makeslab function in rdataslab.c contains an optimization for cases
where the source is already an rdataslab. In these cases, it copies the
entire slab using memmove.  However, this creates a race condition: while
the target slab is protected by a node lock, the source slab is not
protected.  This becomes problematic because the TTL heap needs to
modify the heap index stored in the slab header, potentially while the
memmove operation is reading from it.

A closer look at makeslab shows that copying the header part of the slab
is unnecessary, the header can be default-initialized instead. This MR
modifies makeslab to copy only the raw part of the slab, while
default-initializing the header, eliminating the race condition.  For
consistency, it also applies the same change to vecheader/makevec.
This commit is contained in:
Ondřej Surý 2025-12-16 11:11:05 +01:00
parent 7cbf5f652a
commit 62cb5b30da
2 changed files with 54 additions and 40 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

58
lib/dns/rdataslab.c
View file

@ -17,6 +17,8 @@
#include <stdbool.h>
#include <stdlib.h>
#include <urcu/list.h>
#include <isc/ascii.h>
#include <isc/atomic.h>
#include <isc/list.h>
@ -100,6 +102,24 @@ compare_rdata(const void *p1, const void *p2) {
return dns_rdata_compare(p1, p2);
}
static unsigned char *
newslab(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
size_t size) {
dns_slabheader_t *header = isc_mem_get(mctx, size);
*header = (dns_slabheader_t){
.headers_link = CDS_LIST_HEAD_INIT(header->headers_link),
.trust = rdataset->trust,
.ttl = rdataset->ttl,
.dirtylink = ISC_LINK_INITIALIZER,
};
region->base = (unsigned char *)header;
region->length = size;
return (unsigned char *)header + sizeof(*header);
}
static isc_result_t
makeslab(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
uint32_t maxrrperset) {
@ -128,11 +148,11 @@ makeslab(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
dns_slabheader_t *header = rdataset_getheader(rdataset);
buflen = dns_rdataslab_size(header);
rawbuf = isc_mem_get(mctx, buflen);
region->base = rawbuf;
region->length = buflen;
rawbuf = newslab(rdataset, mctx, region, buflen);
memmove(rawbuf, header, buflen);
INSIST(headerlen <= buflen);
memmove(rawbuf, (unsigned char *)header + headerlen,
buflen - headerlen);
return ISC_R_SUCCESS;
}
@ -145,10 +165,7 @@ makeslab(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
if (rdataset->type != 0) {
return ISC_R_FAILURE;
}
rawbuf = isc_mem_get(mctx, buflen);
region->base = rawbuf;
region->length = buflen;
rawbuf += headerlen;
rawbuf = newslab(rdataset, mctx, region, buflen);
put_uint16(rawbuf, 0);
return ISC_R_SUCCESS;
}
@ -253,11 +270,7 @@ makeslab(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
* Allocate the memory, set up a buffer, start copying in
* data.
*/
rawbuf = isc_mem_get(mctx, buflen);
region->base = rawbuf;
region->length = buflen;
rawbuf += headerlen;
rawbuf = newslab(rdataset, mctx, region, buflen);
put_uint16(rawbuf, nitems);
for (i = 0; i < nalloc; i++) {
@ -306,29 +319,20 @@ dns_rdataslab_fromrdataset(dns_rdataset_t *rdataset, isc_mem_t *mctx,
result = makeslab(rdataset, mctx, region, maxrrperset);
if (result == ISC_R_SUCCESS) {
dns_slabheader_t *new = (dns_slabheader_t *)region->base;
dns_typepair_t typepair;
dns_slabheader_t *header = (dns_slabheader_t *)region->base;
if (rdataset->attributes.negative) {
INSIST(rdataset->type == dns_rdatatype_none);
INSIST(rdataset->covers != dns_rdatatype_none);
typepair = DNS_TYPEPAIR_VALUE(rdataset->covers,
dns_rdatatype_none);
header->typepair = DNS_TYPEPAIR_VALUE(
rdataset->covers, dns_rdatatype_none);
} else {
INSIST(rdataset->type != dns_rdatatype_none);
INSIST(dns_rdatatype_issig(rdataset->type) ||
rdataset->covers == dns_rdatatype_none);
typepair = DNS_TYPEPAIR_VALUE(rdataset->type,
rdataset->covers);
header->typepair = DNS_TYPEPAIR_VALUE(rdataset->type,
rdataset->covers);
}
*new = (dns_slabheader_t){
.headers_link = CDS_LIST_HEAD_INIT(new->headers_link),
.typepair = typepair,
.trust = rdataset->trust,
.ttl = rdataset->ttl,
.dirtylink = ISC_LINK_INITIALIZER,
};
}
return result;

36
lib/dns/rdatavec.c
View file

@ -123,6 +123,23 @@ rdatavec_count(dns_vecheader_t *header) {
return count;
}
static unsigned char *
newvec(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
size_t size) {
dns_vecheader_t *header = isc_mem_get(mctx, size);
*header = (dns_vecheader_t){
.next_header = ISC_SLINK_INITIALIZER,
.trust = rdataset->trust,
.ttl = rdataset->ttl,
};
region->base = (unsigned char *)header;
region->length = size;
return (unsigned char *)header + sizeof(*header);
}
static isc_result_t
makevec(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
uint32_t maxrrperset) {
@ -151,11 +168,11 @@ makevec(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
dns_vecheader_t *header = dns_vecheader_getheader(rdataset);
buflen = dns_rdatavec_size(header);
rawbuf = isc_mem_get(mctx, buflen);
region->base = rawbuf;
region->length = buflen;
rawbuf = newvec(rdataset, mctx, region, buflen);
memmove(rawbuf, header, buflen);
INSIST(headerlen <= buflen);
memmove(rawbuf, (unsigned char *)header + headerlen,
buflen - headerlen);
return ISC_R_SUCCESS;
}
@ -168,10 +185,7 @@ makevec(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
if (rdataset->type != 0) {
return ISC_R_FAILURE;
}
rawbuf = isc_mem_get(mctx, buflen);
region->base = rawbuf;
region->length = buflen;
rawbuf += headerlen;
rawbuf = newvec(rdataset, mctx, region, buflen);
put_uint16(rawbuf, 0);
return ISC_R_SUCCESS;
}
@ -276,11 +290,7 @@ makevec(dns_rdataset_t *rdataset, isc_mem_t *mctx, isc_region_t *region,
* Allocate the memory, set up a buffer, start copying in
* data.
*/
rawbuf = isc_mem_get(mctx, buflen);
region->base = rawbuf;
region->length = buflen;
rawbuf += headerlen;
rawbuf = newvec(rdataset, mctx, region, buflen);
put_uint16(rawbuf, nitems);
for (i = 0; i < nalloc; i++) {