Re: [Dnsmasq-discuss] Memory leak for SRV records with TTL=0 in v2.88

2023-10-08 Thread Simon Kelley



On 05/10/2023 16:56, Damian Sawicki via Dnsmasq-discuss wrote:

Hello dnsmasq experts,

There seems to be a memory leak in v2.88. The reproduction steps are
as follows: insert an SRV record with TTL=0 in an upstream DNS server
and query dnsmasq for this record. I inserted a record with name
"dnsmasq-reproduction.entirelynew.com." and 20 almost identical
entries of the form "0 1 587 mail.example.com." and queried dnsmasq at
12 qps: after an hour, the memory consumption increased by about 32
MB.

AFAIU, the leaking memory is allocated in rfc1035.c in


  if (!(addr.srv.target = blockdata_alloc(name, addr.srv.targetlen)))


(line 825 in v2.88). When the following insertion fails (and it fails
when ttl == 0 and daemon->min_cache_ttl == 0 in cache_insert)


  newc = cache_insert(name, , C_IN, now, attl, flags | F_FORWARD | 
secflag);


(line 867 in v2.88), the memory is never freed. I haven't reproduced a
leak in this scenario, but lines 829-830 also don't include
deallocation.


  if (!extract_name(header, qlen, , name, 1, 0))
return 2;


The relevant code hasn’t changed much between 2.81 and 2.89, so the
entire range might potentially be affected.

I’ve noticed the relevant part is currently being rewritten. Until a
new version is ready, the patch (not tested!) pasted below should
solve the problem. The default behaviour of Unbound is to serve stale
records with TTL=0 (see
https://unbound.docs.nlnetlabs.nl/en/latest/topics/core/serve-stale.html),
so this leak may affect many users.

I would be grateful if you could possibly let me know when the release
of v2.90 is planned. If something is not clear, or I could be of any
assistance regarding the leak, please don't hesitate to let me know!

Kind regards,

Damian Sawicki, 




That all makes sense.

You're right that this code being re-written: it's been generalised to 
cache any RR-type. That hasn't fixed the memory leak :( just generalized 
it. Now you can leak memory with TTL=0 records of almost any type!


I shall port your fix to the new code forthwith.

The 2.90 release is planned just as soon as I can tidy up all the loose 
ends. It's not hanging on any big upgrades and insoluble bugs, just on 
my finding time.


I think your patch misses one thing: you need to check that the address 
being inserted is a SRV record: it's a big union, and may be something 
completely different, so you can't rely on addr.srv.target without checking


if (!newc && (flags & F_SRV) && addr.srv.target)
  blockdata_free(addr.srv.target);


Cheers,

Simon.



-

The patch is identical for v2.88 and v2.89.

diff --git a/src/rfc1035.c b/src/rfc1035.c
index 5c0df56..e85fe2e 100644
--- a/src/rfc1035.c
+++ b/src/rfc1035.c
@@ -826,8 +826,10 @@ int extract_addresses(struct dns_header *header,
size_t qlen, char *name, time_t
 return 0;

   /* we overwrote the original name, so get it back here. */
- if (!extract_name(header, qlen, , name, 1, 0))
+ if (!extract_name(header, qlen, , name, 1, 0)){
+   blockdata_free(addr.srv.target);
 return 2;
+ }
 }
   else if (flags & (F_IPV4 | F_IPV6))
 {
@@ -865,6 +867,8 @@ int extract_addresses(struct dns_header *header,
size_t qlen, char *name, time_t
   if (insert)
 {
   newc = cache_insert(name, , C_IN, now, attl,
flags | F_FORWARD | secflag);
+ if (!newc && addr.srv.target)
+   blockdata_free(addr.srv.target);
   if (newc && cpp)
 {
   next_uid(newc);

___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss


___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss


Re: [Dnsmasq-discuss] DHCPv6 doesn't work on Linux interfaces enslaved to a VRF

2023-10-08 Thread Simon Kelley



On 07/10/2023 14:02, Luci Stanescu via Dnsmasq-discuss wrote:

Hi,

I've discovered that DHCPv6 doesn't work on Linux interfaces enslaved to 
a VRF. Now, I believe this to be a bug in the kernel and I've reported 
it, but in case you'd like to implement a workaround in dnsmasq, this is 
quite trivial, as I'll explain in a bit.


The issue is that when a datagram is received from an interface enslaved 
to a VRF device, the sin6_scope_id of the msg_name field returned from 
recvmsg() points to the interface index of the VRF device, instead of 
the enslaved device. Unfortunately, this is completely useless when the 
source address is a link-local address, as a subsequent sendmsg() which 
specifies that scope will fail with ENETUNREACH, as expected, 
considering the interface index of the enslaved device would have to be 
specified as the scope (there can of course be multiple interfaces 
enslaved to a single VRF device).


With DHCPv6, a DHCPSOLICIT is received from a link-local address and 
DHCPADVERTISE is sent to the source of that address, with a scope 
specified according to the scope from the msg_name field returned by 
recvmsg(). I've debugged this using strace, as dnsmasq doesn't print any 
errors when the send fails. Here is the recvmsg() call:


recvmsg(6, {msg_name={sa_family=AF_INET6, sin6_port=htons(546), 
sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "fe80::216:3eff:fed0:4e7d", 
_addr), sin6_scope_id=if_nametoindex("myvrf")}, msg_namelen=28, 
msg_iov=[{iov_base="\1\203\273\n\0\1\0\16\0\1\0\1,\262\320k\0\26>\320N}\0\6\0\10\0\27\0\30\0'"..., iov_len=548}], msg_iovlen=1, msg_control=[{cmsg_len=36, cmsg_level=SOL_IPV6, cmsg_type=0x32}], msg_controllen=40, msg_flags=0}, MSG_PEEK|MSG_TRUNC) = 56


and the sending of the response later on:

sendto(6, 
"\2\203\273\n\0\1\0\16\0\1\0\1,\262\320k\0\26>\320N}\0\2\0\16\0\1\0\1,\262"..., 114, 0, {sa_family=AF_INET6, sin6_port=htons(546), sin6_flowinfo=htonl(0), inet_pton(AF_INET6, "fe80::216:3eff:fed0:4e7d", _addr), sin6_scope_id=if_nametoindex("myvrf")}, 28) = -1 ENETUNREACH (Network is unreachable)


Please notice that the scope is the index of the VRF master device, so 
the sendto() call is certain to fail.


When reporting the issue as a kernel bug, I reproduced the issue using 
local communication with unicast and a couple of simple Python scripts. 
Here's reproduction using local communication, but with multicast, to 
make it closer to home:


First, set up a VRF device and a veth pair, with one end enslaved to the 
VRF master (on which we'll be receiving datagrams) and the other end 
used to send datagrams.


ip link add myvrf type vrf table 42
ip link set myvrf up
ip link add veth1 type veth peer name veth2
ip link set veth1 master myvrf up
ip link set veth2 up

# ip link sh dev myvrf
110: myvrf:  mtu 65575 qdisc noqueue state UP 
mode DEFAULT group default qlen 1000

     link/ether da:ca:c9:2b:6e:02 brd ff:ff:ff:ff:ff:ff
# ip addr sh dev veth1
112: veth1@veth2:  mtu 1500 qdisc 
noqueue master myvrf state UP group default qlen 1000

     link/ether 32:63:cf:f5:08:35 brd ff:ff:ff:ff:ff:ff
     inet6 fe80::3063:cfff:fef5:835/64 scope link
        valid_lft forever preferred_lft forever
# ip addr sh dev veth2
111: veth2@veth1:  mtu 1500 qdisc 
noqueue state UP group default qlen 1000

     link/ether 1a:8f:5a:85:3c:c0 brd ff:ff:ff:ff:ff:ff
     inet6 fe80::188f:5aff:fe85:3cc0/64 scope link
        valid_lft forever preferred_lft forever

The receiver:
import socket
import struct

s = socket.socket(socket.AF_INET6, socket.SOCK_DGRAM, socket.IPPROTO_UDP)
s.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_RECVPKTINFO, 1)
s.setsockopt(socket.SOL_SOCKET, socket.SO_BINDTODEVICE, b'veth1')
s.bind(('', 2000, 0, 0))
mreq = struct.pack('@16sI', socket.inet_pton(socket.AF_INET6, 
'ff02::1:2'), socket.if_nametoindex('veth1'))

s.setsockopt(socket.IPPROTO_IPV6, socket.IPV6_JOIN_GROUP, mreq)

while True:
     data, cmsg_list, flags, source = s.recvmsg(4096, 4096)
     for level, type, cmsg_data in cmsg_list:
         if level == socket.IPPROTO_IPV6 and type == socket.IPV6_PKTINFO:
             dest_address, dest_scope = struct.unpack('@16sI', cmsg_data)
             dest_address = socket.inet_ntop(socket.AF_INET6, dest_address)
             dest_scope = socket.if_indextoname(dest_scope)
             print("PKTINFO destination {} {}".format(dest_address, 
dest_scope))

     source_address, source_port, source_flow, source_scope = source
     source_scope = socket.if_indextoname(source_scope)
     print("name source {} {}".format(source_address, source_scope))

And the sender:
import socket

s = socket.socket(socket.AF_INET6, socket.SOCK_DGRAM, socket.IPPROTO_UDP)
dest = ('ff02::1:2', 2000, 0, socket.if_nametoindex('veth2'))
s.sendto(b'foo', dest)

The receiver will print:
PKTINFO destination ff02::1:2 veth1
name source fe80::188f:5aff:fe85:3cc0 myvrf

Please notice that the receiver gets the right address, the one 
associated to veth2, but the scope identifies the VRF 

[Dnsmasq-discuss] [PATCH] Set pointers to NULL after memory is freed

2023-10-08 Thread renmingshuai via Dnsmasq-discuss
Set pointers to NULL after memory is freed to reduce dangling pointers, 
although they are later set to new values.

>From 5567d99099191f0cdb2922555e6ade2634f94f30 Mon Sep 17 00:00:00 2001
From: renmingshuai 
Date: Sun, 8 Oct 2023 16:06:46 +0800
Subject: [PATCH] Set pointers to NULL after memory is freed

---
src/option.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/src/option.c b/src/option.c
index 286f06b..23601e1 100644
--- a/src/option.c
+++ b/src/option.c
@@ -5732,6 +5732,7 @@ static void clear_dynamic_conf(void)
   else
up = >next;
 }
+  daemon->dhcp_conf = NULL;
}

static void clear_dhcp_opt(struct dhcp_opt **dhcp_opts)
@@ -5755,8 +5756,10 @@ static void clear_dhcp_opt(struct dhcp_opt **dhcp_opts)
static void clear_dynamic_opt(void)
{
   clear_dhcp_opt(>dhcp_opts);
+  daemon->dhcp_opts = NULL;
#ifdef HAVE_DHCP6
   clear_dhcp_opt(>dhcp_opts6);
+  daemon->dhcp_opts6 = NULL;
#endif
}

--
2.33.0
___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss