resolve: slightly optimize dns_answer_add()
Previously, dns_answer_add() was O(n^2). With this change dns_packet_extract() becomes ~15 times faster for some extremal case. Before: ``` $ time ./fuzz-dns-packet ~/downloads/clusterfuzz-testcase-minimized-fuzz-dns-packet-5631106733047808 /home/watanabe/downloads/clusterfuzz-testcase-minimized-fuzz-dns-packet-5631106733047808... ok real 0m15.453s user 0m15.430s sys 0m0.007s ``` After: ``` $ time ./fuzz-dns-packet ~/downloads/clusterfuzz-testcase-minimized-fuzz-dns-packet-5631106733047808 /home/watanabe/downloads/clusterfuzz-testcase-minimized-fuzz-dns-packet-5631106733047808... ok real 0m0.831s user 0m0.824s sys 0m0.006s ``` Hopefully fixes oss-fuzz#19227. https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=19227
This commit is contained in:
parent
b652cccab9
commit
ae45e1a383
|
@ -8,18 +8,53 @@
|
||||||
#include "resolved-dns-dnssec.h"
|
#include "resolved-dns-dnssec.h"
|
||||||
#include "string-util.h"
|
#include "string-util.h"
|
||||||
|
|
||||||
|
static void dns_answer_item_hash_func(const DnsAnswerItem *a, struct siphash *state) {
|
||||||
|
assert(a);
|
||||||
|
assert(state);
|
||||||
|
|
||||||
|
siphash24_compress(&a->ifindex, sizeof(a->ifindex), state);
|
||||||
|
|
||||||
|
dns_resource_record_hash_func(a->rr, state);
|
||||||
|
}
|
||||||
|
|
||||||
|
static int dns_answer_item_compare_func(const DnsAnswerItem *a, const DnsAnswerItem *b) {
|
||||||
|
int r;
|
||||||
|
|
||||||
|
assert(a);
|
||||||
|
assert(b);
|
||||||
|
|
||||||
|
r = CMP(a->ifindex, b->ifindex);
|
||||||
|
if (r != 0)
|
||||||
|
return r;
|
||||||
|
|
||||||
|
return dns_resource_record_compare_func(a->rr, b->rr);
|
||||||
|
}
|
||||||
|
|
||||||
|
DEFINE_PRIVATE_HASH_OPS(dns_answer_item_hash_ops, DnsAnswerItem, dns_answer_item_hash_func, dns_answer_item_compare_func);
|
||||||
|
|
||||||
DnsAnswer *dns_answer_new(size_t n) {
|
DnsAnswer *dns_answer_new(size_t n) {
|
||||||
|
_cleanup_set_free_ Set *s = NULL;
|
||||||
DnsAnswer *a;
|
DnsAnswer *a;
|
||||||
|
|
||||||
if (n > UINT16_MAX) /* We can only place 64K RRs in an answer at max */
|
if (n > UINT16_MAX) /* We can only place 64K RRs in an answer at max */
|
||||||
n = UINT16_MAX;
|
n = UINT16_MAX;
|
||||||
|
|
||||||
|
s = set_new(&dns_answer_item_hash_ops);
|
||||||
|
if (!s)
|
||||||
|
return NULL;
|
||||||
|
|
||||||
|
/* Higher multipliers give slightly higher efficiency through hash collisions, but the gains
|
||||||
|
* quickly drop off after 2. */
|
||||||
|
if (set_reserve(s, n * 2) < 0)
|
||||||
|
return NULL;
|
||||||
|
|
||||||
a = malloc0(offsetof(DnsAnswer, items) + sizeof(DnsAnswerItem) * n);
|
a = malloc0(offsetof(DnsAnswer, items) + sizeof(DnsAnswerItem) * n);
|
||||||
if (!a)
|
if (!a)
|
||||||
return NULL;
|
return NULL;
|
||||||
|
|
||||||
a->n_ref = 1;
|
a->n_ref = 1;
|
||||||
a->n_allocated = n;
|
a->n_allocated = n;
|
||||||
|
a->set_items = TAKE_PTR(s);
|
||||||
|
|
||||||
return a;
|
return a;
|
||||||
}
|
}
|
||||||
|
@ -30,6 +65,8 @@ static void dns_answer_flush(DnsAnswer *a) {
|
||||||
if (!a)
|
if (!a)
|
||||||
return;
|
return;
|
||||||
|
|
||||||
|
a->set_items = set_free(a->set_items);
|
||||||
|
|
||||||
DNS_ANSWER_FOREACH(rr, a)
|
DNS_ANSWER_FOREACH(rr, a)
|
||||||
dns_resource_record_unref(rr);
|
dns_resource_record_unref(rr);
|
||||||
|
|
||||||
|
@ -46,6 +83,8 @@ static DnsAnswer *dns_answer_free(DnsAnswer *a) {
|
||||||
DEFINE_TRIVIAL_REF_UNREF_FUNC(DnsAnswer, dns_answer, dns_answer_free);
|
DEFINE_TRIVIAL_REF_UNREF_FUNC(DnsAnswer, dns_answer, dns_answer_free);
|
||||||
|
|
||||||
static int dns_answer_add_raw(DnsAnswer *a, DnsResourceRecord *rr, int ifindex, DnsAnswerFlags flags) {
|
static int dns_answer_add_raw(DnsAnswer *a, DnsResourceRecord *rr, int ifindex, DnsAnswerFlags flags) {
|
||||||
|
int r;
|
||||||
|
|
||||||
assert(rr);
|
assert(rr);
|
||||||
|
|
||||||
if (!a)
|
if (!a)
|
||||||
|
@ -54,12 +93,21 @@ static int dns_answer_add_raw(DnsAnswer *a, DnsResourceRecord *rr, int ifindex,
|
||||||
if (a->n_rrs >= a->n_allocated)
|
if (a->n_rrs >= a->n_allocated)
|
||||||
return -ENOSPC;
|
return -ENOSPC;
|
||||||
|
|
||||||
a->items[a->n_rrs++] = (DnsAnswerItem) {
|
a->items[a->n_rrs] = (DnsAnswerItem) {
|
||||||
.rr = dns_resource_record_ref(rr),
|
.rr = rr,
|
||||||
.ifindex = ifindex,
|
.ifindex = ifindex,
|
||||||
.flags = flags,
|
.flags = flags,
|
||||||
};
|
};
|
||||||
|
|
||||||
|
r = set_put(a->set_items, &a->items[a->n_rrs]);
|
||||||
|
if (r < 0)
|
||||||
|
return r;
|
||||||
|
if (r == 0)
|
||||||
|
return -EEXIST;
|
||||||
|
|
||||||
|
dns_resource_record_ref(rr);
|
||||||
|
a->n_rrs++;
|
||||||
|
|
||||||
return 1;
|
return 1;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -78,8 +126,7 @@ static int dns_answer_add_raw_all(DnsAnswer *a, DnsAnswer *source) {
|
||||||
}
|
}
|
||||||
|
|
||||||
int dns_answer_add(DnsAnswer *a, DnsResourceRecord *rr, int ifindex, DnsAnswerFlags flags) {
|
int dns_answer_add(DnsAnswer *a, DnsResourceRecord *rr, int ifindex, DnsAnswerFlags flags) {
|
||||||
size_t i;
|
DnsAnswerItem tmp, *exist;
|
||||||
int r;
|
|
||||||
|
|
||||||
assert(rr);
|
assert(rr);
|
||||||
|
|
||||||
|
@ -88,36 +135,26 @@ int dns_answer_add(DnsAnswer *a, DnsResourceRecord *rr, int ifindex, DnsAnswerFl
|
||||||
if (a->n_ref > 1)
|
if (a->n_ref > 1)
|
||||||
return -EBUSY;
|
return -EBUSY;
|
||||||
|
|
||||||
for (i = 0; i < a->n_rrs; i++) {
|
tmp = (DnsAnswerItem) {
|
||||||
if (a->items[i].ifindex != ifindex)
|
.rr = rr,
|
||||||
continue;
|
.ifindex = ifindex,
|
||||||
|
};
|
||||||
r = dns_resource_key_equal(a->items[i].rr->key, rr->key);
|
|
||||||
if (r < 0)
|
|
||||||
return r;
|
|
||||||
if (r == 0)
|
|
||||||
continue;
|
|
||||||
|
|
||||||
|
exist = set_get(a->set_items, &tmp);
|
||||||
|
if (exist) {
|
||||||
/* There's already an RR of the same RRset in place! Let's see if the TTLs more or less
|
/* There's already an RR of the same RRset in place! Let's see if the TTLs more or less
|
||||||
* match. We don't really care if they match precisely, but we do care whether one is 0 and
|
* match. We don't really care if they match precisely, but we do care whether one is 0 and
|
||||||
* the other is not. See RFC 2181, Section 5.2. */
|
* the other is not. See RFC 2181, Section 5.2. */
|
||||||
if ((rr->ttl == 0) != (a->items[i].rr->ttl == 0))
|
if ((rr->ttl == 0) != (exist->rr->ttl == 0))
|
||||||
return -EINVAL;
|
return -EINVAL;
|
||||||
|
|
||||||
r = dns_resource_record_payload_equal(a->items[i].rr, rr);
|
/* Entry already exists, keep the entry with the higher TTL. */
|
||||||
if (r < 0)
|
if (rr->ttl > exist->rr->ttl) {
|
||||||
return r;
|
dns_resource_record_unref(exist->rr);
|
||||||
if (r == 0)
|
exist->rr = dns_resource_record_ref(rr);
|
||||||
continue;
|
|
||||||
|
|
||||||
/* Entry already exists, keep the entry with the higher RR. */
|
|
||||||
if (rr->ttl > a->items[i].rr->ttl) {
|
|
||||||
dns_resource_record_ref(rr);
|
|
||||||
dns_resource_record_unref(a->items[i].rr);
|
|
||||||
a->items[i].rr = rr;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
a->items[i].flags |= flags;
|
exist->flags |= flags;
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -6,6 +6,7 @@ typedef struct DnsAnswerItem DnsAnswerItem;
|
||||||
|
|
||||||
#include "macro.h"
|
#include "macro.h"
|
||||||
#include "resolved-dns-rr.h"
|
#include "resolved-dns-rr.h"
|
||||||
|
#include "set.h"
|
||||||
|
|
||||||
/* A simple array of resource records. We keep track of the
|
/* A simple array of resource records. We keep track of the
|
||||||
* originating ifindex for each RR where that makes sense, so that we
|
* originating ifindex for each RR where that makes sense, so that we
|
||||||
|
@ -30,6 +31,7 @@ struct DnsAnswerItem {
|
||||||
|
|
||||||
struct DnsAnswer {
|
struct DnsAnswer {
|
||||||
unsigned n_ref;
|
unsigned n_ref;
|
||||||
|
Set *set_items; /* Used by dns_answer_add() for optimization. */
|
||||||
size_t n_rrs, n_allocated;
|
size_t n_rrs, n_allocated;
|
||||||
DnsAnswerItem items[0];
|
DnsAnswerItem items[0];
|
||||||
};
|
};
|
||||||
|
|
|
@ -1479,7 +1479,7 @@ void dns_resource_record_hash_func(const DnsResourceRecord *rr, struct siphash *
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
static int dns_resource_record_compare_func(const DnsResourceRecord *x, const DnsResourceRecord *y) {
|
int dns_resource_record_compare_func(const DnsResourceRecord *x, const DnsResourceRecord *y) {
|
||||||
int r;
|
int r;
|
||||||
|
|
||||||
r = dns_resource_key_compare_func(x->key, y->key);
|
r = dns_resource_key_compare_func(x->key, y->key);
|
||||||
|
|
|
@ -330,6 +330,7 @@ DnsTxtItem *dns_txt_item_copy(DnsTxtItem *i);
|
||||||
int dns_txt_item_new_empty(DnsTxtItem **ret);
|
int dns_txt_item_new_empty(DnsTxtItem **ret);
|
||||||
|
|
||||||
void dns_resource_record_hash_func(const DnsResourceRecord *i, struct siphash *state);
|
void dns_resource_record_hash_func(const DnsResourceRecord *i, struct siphash *state);
|
||||||
|
int dns_resource_record_compare_func(const DnsResourceRecord *x, const DnsResourceRecord *y);
|
||||||
|
|
||||||
extern const struct hash_ops dns_resource_key_hash_ops;
|
extern const struct hash_ops dns_resource_key_hash_ops;
|
||||||
extern const struct hash_ops dns_resource_record_hash_ops;
|
extern const struct hash_ops dns_resource_record_hash_ops;
|
||||||
|
|
Binary file not shown.
Loading…
Reference in New Issue