resolve: Try to remove the ambiguity about the mtu parameter of dns_packet_new (...
authorBenjamin Robin <benjarobin@users.noreply.github.com>
Thu, 6 Jul 2017 02:56:17 +0000 (04:56 +0200)
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Thu, 6 Jul 2017 02:56:17 +0000 (22:56 -0400)
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.

src/resolve/resolved-dns-packet.c
src/resolve/resolved-dns-packet.h
src/resolve/test-resolved-packet.c

index a486216..49a0461 100644 (file)
@@ -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;
 
index 5dff272..a65d6d3 100644 (file)
@@ -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);
 
index 8b7da14..1b00412 100644 (file)
@@ -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) {