From 6c119e11b26bde34ac9925cc2ad9cda690442c61 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Wed, 20 May 2026 10:37:58 +0200 Subject: [PATCH 1/2] dns-packet: drop unnecessary indentation Bail out early if QDCOUNT == 0, similarly to what we already do in dns_packet_extract_answer() when RRCOUNT == 0. --- src/shared/dns-packet.c | 81 ++++++++++++++++++++++------------------- 1 file changed, 43 insertions(+), 38 deletions(-) diff --git a/src/shared/dns-packet.c b/src/shared/dns-packet.c index 4fdef2b570a..bb17c94d4aa 100644 --- a/src/shared/dns-packet.c +++ b/src/shared/dns-packet.c @@ -2452,51 +2452,54 @@ static int dns_packet_extract_question(DnsPacket *p, DnsQuestion **ret_question) assert(ret_question); n = DNS_PACKET_QDCOUNT(p); - if (n > 0) { - question = dns_question_new(n); - if (!question) - return -ENOMEM; + if (n == 0) { + *ret_question = NULL; + return 0; + } - _cleanup_set_free_ Set *keys = NULL; /* references to keys are kept by Question */ + question = dns_question_new(n); + if (!question) + return -ENOMEM; - keys = set_new(&dns_resource_key_hash_ops); - if (!keys) - return log_oom(); + _cleanup_set_free_ Set *keys = NULL; /* references to keys are kept by Question */ - /* Pre-allocate the question hashmap, but cap the pre-allocation to a number of questions the - * packet can realistically contain. That is, pick the minimal value from the claimed number - * of questions (n) and a maximum number of potential questions the remaining packet data can - * actually contain: p->size - p->rindex are the remaining unread bytes in the packet, and 5U - * is the minimum size of each question - 1 (QNAME) + 2 (QTYPE) + 2 (QCLASS). - * - * Note for the multiplication: higher multipliers give slightly higher efficiency through - * hash collisions, but the gains quickly drop off after 2. */ - r = set_reserve(keys, MIN(n, (p->size - p->rindex) / 5U) * 2); + keys = set_new(&dns_resource_key_hash_ops); + if (!keys) + return log_oom(); + + /* Pre-allocate the question hashmap, but cap the pre-allocation to a number of questions the + * packet can realistically contain. That is, pick the minimal value from the claimed number + * of questions (n) and a maximum number of potential questions the remaining packet data can + * actually contain: p->size - p->rindex are the remaining unread bytes in the packet, and 5U + * is the minimum size of each question - 1 (QNAME) + 2 (QTYPE) + 2 (QCLASS). + * + * Note for the multiplication: higher multipliers give slightly higher efficiency through + * hash collisions, but the gains quickly drop off after 2. */ + r = set_reserve(keys, MIN(n, (p->size - p->rindex) / 5U) * 2); + if (r < 0) + return r; + + for (unsigned i = 0; i < n; i++) { + _cleanup_(dns_resource_key_unrefp) DnsResourceKey *key = NULL; + bool qu; + + r = dns_packet_read_key(p, &key, &qu, NULL); if (r < 0) return r; - for (unsigned i = 0; i < n; i++) { - _cleanup_(dns_resource_key_unrefp) DnsResourceKey *key = NULL; - bool qu; + if (!dns_type_is_valid_query(key->type)) + return -EBADMSG; - r = dns_packet_read_key(p, &key, &qu, NULL); - if (r < 0) - return r; + r = set_put(keys, key); + if (r < 0) + return r; + if (r == 0) + /* Already in the Question, let's skip */ + continue; - if (!dns_type_is_valid_query(key->type)) - return -EBADMSG; - - r = set_put(keys, key); - if (r < 0) - return r; - if (r == 0) - /* Already in the Question, let's skip */ - continue; - - r = dns_question_add_raw(question, key, qu ? DNS_QUESTION_WANTS_UNICAST_REPLY : 0); - if (r < 0) - return r; - } + r = dns_question_add_raw(question, key, qu ? DNS_QUESTION_WANTS_UNICAST_REPLY : 0); + if (r < 0) + return r; } *ret_question = TAKE_PTR(question); @@ -2514,8 +2517,10 @@ static int dns_packet_extract_answer(DnsPacket *p, DnsAnswer **ret_answer) { assert(ret_answer); n = DNS_PACKET_RRCOUNT(p); - if (n == 0) + if (n == 0) { + *ret_answer = NULL; return 0; + } /* Pre-allocate the answer hashmap, but cap the pre-allocation to a number of RRs the packet can * realistically contain. That is, pick the minimal value from the claimed number of RRs (n) and a From 180985c2cef3648aa1b95338272177610c89c185 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Wed, 20 May 2026 10:47:22 +0200 Subject: [PATCH 2/2] dns-packet: bail out early if the packet is too short Let's bail out early if the packet claims to contain some questions or answer RRs, but the remaining packet data size is not enough to hold a single such entry. Follow-up for e7cd836dcffb5f85d66a156904fc68f8b654a290. --- src/shared/dns-packet.c | 31 ++++++++++++++++++++++--------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/src/shared/dns-packet.c b/src/shared/dns-packet.c index bb17c94d4aa..429e5c82c17 100644 --- a/src/shared/dns-packet.c +++ b/src/shared/dns-packet.c @@ -2446,7 +2446,7 @@ static bool opt_is_good(DnsResourceRecord *rr, bool *rfc6975) { static int dns_packet_extract_question(DnsPacket *p, DnsQuestion **ret_question) { _cleanup_(dns_question_unrefp) DnsQuestion *question = NULL; - unsigned n; + unsigned n, prealloc; int r; assert(ret_question); @@ -2457,6 +2457,14 @@ static int dns_packet_extract_question(DnsPacket *p, DnsQuestion **ret_question) return 0; } + /* Calculate the maximum number of potential questions the remaining packet data can actually + * contain: p->size - p->rindex are the remaining unread bytes in the packet, and 5U is the minimum + * size of each question - 1 (QNAME) + 2 (QTYPE) + 2 (QCLASS). */ + prealloc = (p->size - p->rindex) / 5U; + if (prealloc == 0) + /* QDCOUNT > 0 but there's not enough space left for a single question. */ + return -EMSGSIZE; + question = dns_question_new(n); if (!question) return -ENOMEM; @@ -2470,12 +2478,11 @@ static int dns_packet_extract_question(DnsPacket *p, DnsQuestion **ret_question) /* Pre-allocate the question hashmap, but cap the pre-allocation to a number of questions the * packet can realistically contain. That is, pick the minimal value from the claimed number * of questions (n) and a maximum number of potential questions the remaining packet data can - * actually contain: p->size - p->rindex are the remaining unread bytes in the packet, and 5U - * is the minimum size of each question - 1 (QNAME) + 2 (QTYPE) + 2 (QCLASS). + * actually contain, see above. * * Note for the multiplication: higher multipliers give slightly higher efficiency through * hash collisions, but the gains quickly drop off after 2. */ - r = set_reserve(keys, MIN(n, (p->size - p->rindex) / 5U) * 2); + r = set_reserve(keys, MIN(n, prealloc) * 2); if (r < 0) return r; @@ -2509,7 +2516,7 @@ static int dns_packet_extract_question(DnsPacket *p, DnsQuestion **ret_question) static int dns_packet_extract_answer(DnsPacket *p, DnsAnswer **ret_answer) { _cleanup_(dns_answer_unrefp) DnsAnswer *answer = NULL; - unsigned n; + unsigned n, prealloc; _cleanup_(dns_resource_record_unrefp) DnsResourceRecord *previous = NULL; bool bad_opt = false; int r; @@ -2522,12 +2529,18 @@ static int dns_packet_extract_answer(DnsPacket *p, DnsAnswer **ret_answer) { return 0; } + /* Calculate the maximum number of potential RRs the remaining packet data can actually contain: + * p->size - p->rindex are the remaining unread bytes in the packet, and the 11U is the minimum size + * of each RR - 1 (NAME) + 2 (TYPE) + 2 (CLASS) + 4 (TTL) + 2 (RDLENGTH). */ + prealloc = (p->size - p->rindex) / 11U; + if (prealloc == 0) + /* RRCOUNT > 0 but there's not enough space left for a single RR. */ + return -EMSGSIZE; + /* Pre-allocate the answer hashmap, but cap the pre-allocation to a number of RRs the packet can * realistically contain. That is, pick the minimal value from the claimed number of RRs (n) and a - * maximum number of potential RRs the remaining packet data can actually contain: p->size - - * p->rindex are the remaining unread bytes in the packet, and the 11U is the minimum size of each RR - * - 1 (NAME) + 2 (TYPE) + 2 (CLASS) + 4 (TTL) + 2 (RDLENGTH). */ - answer = dns_answer_new(MIN(n, (p->size - p->rindex) / 11U)); + * maximum number of potential RRs the remaining packet data can actually contain, see above. */ + answer = dns_answer_new(MIN(n, prealloc)); if (!answer) return -ENOMEM;