From: Benjamin Robin <benjarobin@users.noreply.github.com>
Date: Thu, 6 Jul 2017 02:56:17 +0000 (+0200)
Subject: resolve: Try to remove the ambiguity about the mtu parameter of dns_packet_new (... 
X-Git-Tag: v234~28
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=46a58596731684ba511b04ee9e8f7a9af490fbc2;p=thirdparty%2Fsystemd.git

resolve: Try to remove the ambiguity about the mtu parameter of dns_packet_new (#6285)

Actually the caller of dns_packet_new() pass 0 or the data size of the UDP message.
So try to reflect that, so rename the `mtu` parameter to `min_alloc_dsize`.

In fact `mtu` is the size of the whole UDP message, including the UDP header,
and here we just need to pass the size of data (without header). This was confusing.

Also add a check on the requested allocated size, since some caller do not check what is really allocated.
Indeed the function do not allocate more than DNS_PACKET_SIZE_MAX whatever the value of the `mtu` parameter.
---

diff --git a/src/resolve/resolved-dns-packet.c b/src/resolve/resolved-dns-packet.c
index a486216d68d..49a04615d4e 100644
--- a/src/resolve/resolved-dns-packet.c
+++ b/src/resolve/resolved-dns-packet.c
@@ -29,7 +29,7 @@
 #define EDNS0_OPT_DO (1<<15)
 
 #define DNS_PACKET_SIZE_START 512u
-assert_cc(DNS_PACKET_SIZE_START > UDP_PACKET_HEADER_SIZE)
+assert_cc(DNS_PACKET_SIZE_START > DNS_PACKET_HEADER_SIZE)
 
 typedef struct DnsPacketRewinder {
         DnsPacket *packet;
@@ -44,20 +44,28 @@ static void rewind_dns_packet(DnsPacketRewinder *rewinder) {
 #define INIT_REWINDER(rewinder, p) do { rewinder.packet = p; rewinder.saved_rindex = p->rindex; } while (0)
 #define CANCEL_REWINDER(rewinder) do { rewinder.packet = NULL; } while (0)
 
-int dns_packet_new(DnsPacket **ret, DnsProtocol protocol, size_t mtu) {
+int dns_packet_new(DnsPacket **ret, DnsProtocol protocol, size_t min_alloc_dsize) {
         DnsPacket *p;
         size_t a;
 
         assert(ret);
 
-        /* When dns_packet_new() is called with mtu == 0, allocate more than the
+        /* The caller may not check what is going to be truly allocated, so do not allow to
+         * allocate a DNS packet bigger than DNS_PACKET_SIZE_MAX.
+         */
+        if (min_alloc_dsize > DNS_PACKET_SIZE_MAX) {
+                log_error("Requested packet data size too big: %zu", min_alloc_dsize);
+                return -EFBIG;
+        }
+
+        /* When dns_packet_new() is called with min_alloc_dsize == 0, allocate more than the
          * absolute minimum (which is the dns packet header size), to avoid
          * resizing immediately again after appending the first data to the packet.
          */
-        if (mtu < UDP_PACKET_HEADER_SIZE)
+        if (min_alloc_dsize < DNS_PACKET_HEADER_SIZE)
                 a = DNS_PACKET_SIZE_START;
         else
-                a = MAX(mtu, DNS_PACKET_HEADER_SIZE);
+                a = min_alloc_dsize;
 
         /* round up to next page size */
         a = PAGE_ALIGN(ALIGN(sizeof(DnsPacket)) + a) - ALIGN(sizeof(DnsPacket));
@@ -131,13 +139,13 @@ void dns_packet_set_flags(DnsPacket *p, bool dnssec_checking_disabled, bool trun
         }
 }
 
-int dns_packet_new_query(DnsPacket **ret, DnsProtocol protocol, size_t mtu, bool dnssec_checking_disabled) {
+int dns_packet_new_query(DnsPacket **ret, DnsProtocol protocol, size_t min_alloc_dsize, bool dnssec_checking_disabled) {
         DnsPacket *p;
         int r;
 
         assert(ret);
 
-        r = dns_packet_new(&p, protocol, mtu);
+        r = dns_packet_new(&p, protocol, min_alloc_dsize);
         if (r < 0)
                 return r;
 
diff --git a/src/resolve/resolved-dns-packet.h b/src/resolve/resolved-dns-packet.h
index 5dff272fd99..a65d6d38cf9 100644
--- a/src/resolve/resolved-dns-packet.h
+++ b/src/resolve/resolved-dns-packet.h
@@ -183,8 +183,8 @@ static inline unsigned DNS_PACKET_RRCOUNT(DnsPacket *p) {
                 (unsigned) DNS_PACKET_ARCOUNT(p);
 }
 
-int dns_packet_new(DnsPacket **p, DnsProtocol protocol, size_t mtu);
-int dns_packet_new_query(DnsPacket **p, DnsProtocol protocol, size_t mtu, bool dnssec_checking_disabled);
+int dns_packet_new(DnsPacket **p, DnsProtocol protocol, size_t min_alloc_dsize);
+int dns_packet_new_query(DnsPacket **p, DnsProtocol protocol, size_t min_alloc_dsize, bool dnssec_checking_disabled);
 
 void dns_packet_set_flags(DnsPacket *p, bool dnssec_checking_disabled, bool truncated);
 
diff --git a/src/resolve/test-resolved-packet.c b/src/resolve/test-resolved-packet.c
index 8b7da1408d9..1b0041214bb 100644
--- a/src/resolve/test-resolved-packet.c
+++ b/src/resolve/test-resolved-packet.c
@@ -22,8 +22,9 @@
 
 static void test_dns_packet_new(void) {
         size_t i;
+         _cleanup_(dns_packet_unrefp) DnsPacket *p2 = NULL;
 
-        for (i = 0; i < DNS_PACKET_SIZE_MAX + 2; i++) {
+        for (i = 0; i <= DNS_PACKET_SIZE_MAX; i++) {
                 _cleanup_(dns_packet_unrefp) DnsPacket *p = NULL;
 
                 assert_se(dns_packet_new(&p, DNS_PROTOCOL_DNS, i) == 0);
@@ -31,6 +32,8 @@ static void test_dns_packet_new(void) {
                 log_debug("dns_packet_new: %zu → %zu", i, p->allocated);
                 assert_se(p->allocated >= MIN(DNS_PACKET_SIZE_MAX, i));
         }
+
+        assert_se(dns_packet_new(&p2, DNS_PROTOCOL_DNS, DNS_PACKET_SIZE_MAX + 1) == -EFBIG);
 }
 
 int main(int argc, char **argv) {