Hi!
After reading those changes a bit, it seems there are too many variables
used in a complex mess. It is very easy to get lost in them, especially
when sdetails just points somewhere to local function other variables.
I made attempt to reduce the complexity and amount of local variables.
Instead I moved contents of variables directly into server_details
structure and added function to just add server from its pointer. Tried
reducing repetitions by moving similar parts to shared functions.
But I admit I haven't tested the result stays unchanged. It should not
change any behaviour, just cleanups the code. I think I should finally
fix more serious runnable tests, which would ensure the functionality
stayed the same.
What do you think?
Cheers,
Petr
On 11/17/22 20:59, Simon Kelley wrote:
Thanks for testing these builds. That's a genuine problem.
Fixed in 2.88rc3, in git now.
Cheers,
Simon.
On 17/11/2022 18:26, Johnny S. Lee via Dnsmasq-discuss wrote:
"local=//" means "Do not forward unqualified names to any upstream
servers", right?
It has not generated any kind of error until I updated to v2.88rc2.
The last build I've been using was built against
d3c21c596ef96027429b11216fcdbf65c9434afa
_______________________________________________
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
--
Petr Menšík
Software Engineer, RHEL
Red Hat, https://www.redhat.com/
PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB
From 1a6eba5f3972efe31f1a03d33ed64e239ed47079 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Fri, 25 Nov 2022 15:02:37 +0100
Subject: [PATCH 3/3] Reuse similar dbus message parsing
Move almost same codes into single function, use map_address parameter
to make difference between them.
---
src/dbus.c | 91 +++++++++++++++++++++++++-----------------------------
1 file changed, 42 insertions(+), 49 deletions(-)
diff --git a/src/dbus.c b/src/dbus.c
index 6370b41..3e9b9a8 100644
--- a/src/dbus.c
+++ b/src/dbus.c
@@ -260,11 +260,46 @@ static DBusMessage *dbus_reply_server_loop(DBusMessage *message)
}
#endif
+/* Parse one entry from dbus. */
+static DBusMessage *dbus_parse_servers(DBusMessage *message,
+ struct server_details *sdetails,
+ char *str_addr, const char *str,
+ int map_address)
+{
+ const char *addr_err;
+ DBusMessage *error = NULL;
+
+ if ((addr_err = parse_server(str_addr, sdetails)))
+ return dbus_message_new_error_printf(message, DBUS_ERROR_INVALID_ARGS,
+ "Invalid IP address '%s': %s",
+ str, addr_err);
+
+ while (parse_server_next(sdetails))
+ {
+ if ((addr_err = parse_server_addr(sdetails)))
+ return dbus_message_new_error_printf(message, DBUS_ERROR_INVALID_ARGS,
+ "Invalid IP address '%s': %s",
+ str, addr_err);
+
+ if (map_address)
+ {
+ /* 0.0.0.0 for server address == NULL, for Dbus */
+ if (sdetails->addr.in.sin_family == AF_INET &&
+ sdetails->addr.in.sin_addr.s_addr == 0)
+ sdetails->flags |= SERV_LITERAL_ADDRESS;
+ else
+ sdetails->flags &= ~SERV_LITERAL_ADDRESS;
+ }
+
+ add_update_server_details(sdetails, str, NULL);
+ }
+ return error;
+}
+
static DBusMessage* dbus_read_servers_ex(DBusMessage *message, int strings)
{
DBusMessageIter iter, array_iter, string_iter;
DBusMessage *error = NULL;
- const char *addr_err;
char *dup = NULL;
if (!dbus_message_iter_init(message, &iter))
@@ -392,26 +427,9 @@ static DBusMessage* dbus_read_servers_ex(DBusMessage *message, int strings)
}
else
{
- if ((addr_err = parse_server(str_addr, &sdetails)))
- {
- error = dbus_message_new_error_printf(message, DBUS_ERROR_INVALID_ARGS,
- "Invalid IP address '%s': %s",
- str, addr_err);
- break;
- }
-
- while (parse_server_next(&sdetails))
- {
- if ((addr_err = parse_server_addr(&sdetails)))
- {
- error = dbus_message_new_error_printf(message, DBUS_ERROR_INVALID_ARGS,
- "Invalid IP address '%s': %s",
- str, addr_err);
- break;
- }
-
- add_update_server_details(&sdetails, str_domain, NULL);
- }
+ error = dbus_parse_servers(message, &sdetails, str_addr, str, 0);
+ if (error)
+ break;
}
} while ((str_domain = p));
}
@@ -427,34 +445,9 @@ static DBusMessage* dbus_read_servers_ex(DBusMessage *message, int strings)
dbus_message_iter_get_basic(&string_iter, &str);
dbus_message_iter_next (&string_iter);
- if ((addr_err = parse_server(str_addr, &sdetails)))
- {
- error = dbus_message_new_error_printf(message, DBUS_ERROR_INVALID_ARGS,
- "Invalid IP address '%s': %s",
- str, addr_err);
- break;
- }
-
- while (parse_server_next(&sdetails))
- {
- if ((addr_err = parse_server_addr(&sdetails)))
- {
- error = dbus_message_new_error_printf(message, DBUS_ERROR_INVALID_ARGS,
- "Invalid IP address '%s': %s",
- str, addr_err);
- break;
- }
-
- /* 0.0.0.0 for server address == NULL, for Dbus */
- if (sdetails.addr.in.sin_family == AF_INET &&
- sdetails.addr.in.sin_addr.s_addr == 0)
- sdetails.flags |= SERV_LITERAL_ADDRESS;
- else
- sdetails.flags &= ~SERV_LITERAL_ADDRESS;
-
- add_update_server_details(&sdetails, str, NULL);
- }
- } while (dbus_message_iter_get_arg_type(&string_iter) == DBUS_TYPE_STRING);
+ error = dbus_parse_servers(message, &sdetails, str_addr, str, 1);
+ } while (!error &&
+ dbus_message_iter_get_arg_type(&string_iter) == DBUS_TYPE_STRING);
}
if (sdetails.orig_hostinfo)
--
2.38.1
From 4fce0e3f40ed1ad41c428ae2d20096b486c199be Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Tue, 22 Nov 2022 01:51:23 +0100
Subject: [PATCH 2/3] Reuse some common parts of server details parsing
Use server_details direct member, instead of using multiple local
variables linked from by pointers. Move some common parts to shared
function.
---
src/dbus.c | 26 +++---
src/dnsmasq.h | 10 +-
src/domain-match.c | 8 ++
src/option.c | 225 +++++++++++++++++++--------------------------
4 files changed, 122 insertions(+), 147 deletions(-)
diff --git a/src/dbus.c b/src/dbus.c
index 2067ed9..6370b41 100644
--- a/src/dbus.c
+++ b/src/dbus.c
@@ -288,15 +288,8 @@ static DBusMessage* dbus_read_servers_ex(DBusMessage *message, int strings)
while (dbus_message_iter_get_arg_type(&array_iter) != DBUS_TYPE_INVALID)
{
const char *str = NULL;
- union mysockaddr addr, source_addr;
- u16 flags = 0;
- char interface[IF_NAMESIZE];
char *str_addr, *str_domain = NULL;
- struct server_details sdetails = { 0 };
- sdetails.addr = &addr;
- sdetails.source_addr = &source_addr;
- sdetails.interface = interface;
- sdetails.flags = &flags;
+ struct server_details sdetails = { .flags = SERV_FROM_DBUS, };
if (strings)
{
@@ -393,7 +386,10 @@ static DBusMessage* dbus_read_servers_ex(DBusMessage *message, int strings)
p = NULL;
if (strings && strlen(str_addr) == 0)
- add_update_server(SERV_LITERAL_ADDRESS | SERV_FROM_DBUS, &addr, &source_addr, interface, str_domain, NULL);
+ {
+ sdetails.flags |= SERV_LITERAL_ADDRESS;
+ add_update_server_details(&sdetails, str_domain, NULL);
+ }
else
{
if ((addr_err = parse_server(str_addr, &sdetails)))
@@ -414,7 +410,7 @@ static DBusMessage* dbus_read_servers_ex(DBusMessage *message, int strings)
break;
}
- add_update_server(flags | SERV_FROM_DBUS, &addr, &source_addr, interface, str_domain, NULL);
+ add_update_server_details(&sdetails, str_domain, NULL);
}
}
} while ((str_domain = p));
@@ -450,13 +446,13 @@ static DBusMessage* dbus_read_servers_ex(DBusMessage *message, int strings)
}
/* 0.0.0.0 for server address == NULL, for Dbus */
- if (addr.in.sin_family == AF_INET &&
- addr.in.sin_addr.s_addr == 0)
- flags |= SERV_LITERAL_ADDRESS;
+ if (sdetails.addr.in.sin_family == AF_INET &&
+ sdetails.addr.in.sin_addr.s_addr == 0)
+ sdetails.flags |= SERV_LITERAL_ADDRESS;
else
- flags &= ~SERV_LITERAL_ADDRESS;
+ sdetails.flags &= ~SERV_LITERAL_ADDRESS;
- add_update_server(flags | SERV_FROM_DBUS, &addr, &source_addr, interface, str, NULL);
+ add_update_server_details(&sdetails, str, NULL);
}
} while (dbus_message_iter_get_arg_type(&string_iter) == DBUS_TYPE_STRING);
}
diff --git a/src/dnsmasq.h b/src/dnsmasq.h
index 90dc986..68fc01f 100644
--- a/src/dnsmasq.h
+++ b/src/dnsmasq.h
@@ -1294,11 +1294,12 @@ extern struct daemon {
} *daemon;
struct server_details {
- union mysockaddr *addr, *source_addr;
+ union mysockaddr addr, source_addr;
+ char interface[IF_NAMESIZE+1];
struct addrinfo *hostinfo, *orig_hostinfo;
- char *interface, *source, *scope_id, *interface_opt;
+ char *source, *scope_id, *interface_opt;
int serv_port, source_port, addr_type, scope_index, valid;
- u16 *flags;
+ u16 flags;
};
/* cache.c */
@@ -1860,3 +1861,6 @@ int add_update_server(int flags,
const char *interface,
const char *domain,
union all_addr *local_addr);
+int add_update_server_details(struct server_details *sdetails,
+ const char *domain,
+ union all_addr *local_addr);
diff --git a/src/domain-match.c b/src/domain-match.c
index bef460a..e66efbc 100644
--- a/src/domain-match.c
+++ b/src/domain-match.c
@@ -745,3 +745,11 @@ int add_update_server(int flags,
return 1;
}
+int add_update_server_details(struct server_details *sdetails,
+ const char *domain,
+ union all_addr *local_addr)
+{
+ char *interface = (sdetails->interface[0]) ? sdetails->interface : NULL;
+ return add_update_server(sdetails->flags, &sdetails->addr, &sdetails->source_addr,
+ interface, domain, local_addr);
+}
diff --git a/src/option.c b/src/option.c
index efd5f49..4ffc64d 100644
--- a/src/option.c
+++ b/src/option.c
@@ -857,11 +857,12 @@ static char *parse_mysockaddr(char *arg, union mysockaddr *addr)
char *parse_server(char *arg, struct server_details *sdetails)
{
- sdetails->serv_port = NAMESERVER_PORT;
char *portno;
int ecode = 0;
struct addrinfo hints;
+ memset(sdetails, 0, sizeof(struct server_details));
+ sdetails->serv_port = NAMESERVER_PORT;
memset(&hints, 0, sizeof(struct addrinfo));
*sdetails->interface = 0;
@@ -869,8 +870,7 @@ char *parse_server(char *arg, struct server_details *sdetails)
if (strcmp(arg, "#") == 0)
{
- if (sdetails->flags)
- *sdetails->flags |= SERV_USE_RESOLV;
+ sdetails->flags |= SERV_USE_RESOLV;
sdetails->addr_type = AF_LOCAL;
sdetails->valid = 1;
return NULL;
@@ -901,9 +901,9 @@ char *parse_server(char *arg, struct server_details *sdetails)
}
}
- if (inet_pton(AF_INET, arg, &sdetails->addr->in.sin_addr) > 0)
+ if (inet_pton(AF_INET, arg, &sdetails->addr.in.sin_addr) > 0)
sdetails->addr_type = AF_INET;
- else if (inet_pton(AF_INET6, arg, &sdetails->addr->in6.sin6_addr) > 0)
+ else if (inet_pton(AF_INET6, arg, &sdetails->addr.in6.sin6_addr) > 0)
sdetails->addr_type = AF_INET6;
else
{
@@ -966,24 +966,23 @@ char *parse_server_addr(struct server_details *sdetails)
{
if (sdetails->addr_type == AF_INET)
{
- sdetails->addr->in.sin_port = htons(sdetails->serv_port);
- sdetails->addr->sa.sa_family = sdetails->source_addr->sa.sa_family = AF_INET;
+ sdetails->addr.in.sin_port = htons(sdetails->serv_port);
+ sdetails->addr.sa.sa_family = sdetails->source_addr.sa.sa_family = AF_INET;
#ifdef HAVE_SOCKADDR_SA_LEN
- sdetails->source_addr->in.sin_len = sdetails->addr->in.sin_len = sizeof(struct sockaddr_in);
+ sdetails->source_addr.in.sin_len = sdetails->addr->in.sin_len = sizeof(struct sockaddr_in);
#endif
- sdetails->source_addr->in.sin_addr.s_addr = INADDR_ANY;
- sdetails->source_addr->in.sin_port = htons(daemon->query_port);
+ sdetails->source_addr.in.sin_addr.s_addr = INADDR_ANY;
+ sdetails->source_addr.in.sin_port = htons(daemon->query_port);
if (sdetails->source)
{
- if (sdetails->flags)
- *sdetails->flags |= SERV_HAS_SOURCE;
- sdetails->source_addr->in.sin_port = htons(sdetails->source_port);
- if (inet_pton(AF_INET, sdetails->source, &sdetails->source_addr->in.sin_addr) == 0)
+ sdetails->flags |= SERV_HAS_SOURCE;
+ sdetails->source_addr.in.sin_port = htons(sdetails->source_port);
+ if (inet_pton(AF_INET, sdetails->source, &sdetails->source_addr.in.sin_addr) == 0)
{
- if (inet_pton(AF_INET6, sdetails->source, &sdetails->source_addr->in6.sin6_addr) == 1)
+ if (inet_pton(AF_INET6, sdetails->source, &sdetails->source_addr.in6.sin6_addr) == 1)
{
- sdetails->source_addr->sa.sa_family = AF_INET6;
+ sdetails->source_addr.sa.sa_family = AF_INET6;
/* When resolving a server IP by hostname, we can simply skip mismatching
server / source IP pairs. Otherwise, when an IP address is given directly,
this is a fatal error. */
@@ -996,7 +995,7 @@ char *parse_server_addr(struct server_details *sdetails)
if (sdetails->interface_opt)
return _("interface can only be specified once");
- sdetails->source_addr->in.sin_addr.s_addr = INADDR_ANY;
+ sdetails->source_addr.in.sin_addr.s_addr = INADDR_ANY;
safe_strncpy(sdetails->interface, sdetails->source, IF_NAMESIZE);
#else
return _("interface binding not supported");
@@ -1010,26 +1009,25 @@ char *parse_server_addr(struct server_details *sdetails)
if (sdetails->scope_id && (sdetails->scope_index = if_nametoindex(sdetails->scope_id)) == 0)
return _("bad interface name");
- sdetails->addr->in6.sin6_port = htons(sdetails->serv_port);
- sdetails->addr->in6.sin6_scope_id = sdetails->scope_index;
- sdetails->source_addr->in6.sin6_addr = in6addr_any;
- sdetails->source_addr->in6.sin6_port = htons(daemon->query_port);
- sdetails->source_addr->in6.sin6_scope_id = 0;
- sdetails->addr->sa.sa_family = sdetails->source_addr->sa.sa_family = AF_INET6;
- sdetails->addr->in6.sin6_flowinfo = sdetails->source_addr->in6.sin6_flowinfo = 0;
+ sdetails->addr.in6.sin6_port = htons(sdetails->serv_port);
+ sdetails->addr.in6.sin6_scope_id = sdetails->scope_index;
+ sdetails->source_addr.in6.sin6_addr = in6addr_any;
+ sdetails->source_addr.in6.sin6_port = htons(daemon->query_port);
+ sdetails->source_addr.in6.sin6_scope_id = 0;
+ sdetails->addr.sa.sa_family = sdetails->source_addr.sa.sa_family = AF_INET6;
+ sdetails->addr.in6.sin6_flowinfo = sdetails->source_addr.in6.sin6_flowinfo = 0;
#ifdef HAVE_SOCKADDR_SA_LEN
- sdetails->addr->in6.sin6_len = sdetails->source_addr->in6.sin6_len = sizeof(sdetails->addr->in6);
+ sdetails->addr.in6.sin6_len = sdetails->source_addr.in6.sin6_len = sizeof(sdetails->addr->in6);
#endif
if (sdetails->source)
{
- if (sdetails->flags)
- *sdetails->flags |= SERV_HAS_SOURCE;
- sdetails->source_addr->in6.sin6_port = htons(sdetails->source_port);
- if (inet_pton(AF_INET6, sdetails->source, &sdetails->source_addr->in6.sin6_addr) == 0)
+ sdetails->flags |= SERV_HAS_SOURCE;
+ sdetails->source_addr.in6.sin6_port = htons(sdetails->source_port);
+ if (inet_pton(AF_INET6, sdetails->source, &sdetails->source_addr.in6.sin6_addr) == 0)
{
- if (inet_pton(AF_INET, sdetails->source, &sdetails->source_addr->in.sin_addr) == 1)
+ if (inet_pton(AF_INET, sdetails->source, &sdetails->source_addr.in.sin_addr) == 1)
{
- sdetails->source_addr->sa.sa_family = AF_INET;
+ sdetails->source_addr.sa.sa_family = AF_INET;
/* When resolving a server IP by hostname, we can simply skip mismatching
server / source IP pairs. Otherwise, when an IP address is given directly,
this is a fatal error. */
@@ -1042,7 +1040,7 @@ char *parse_server_addr(struct server_details *sdetails)
if (sdetails->interface_opt)
return _("interface can only be specified once");
- sdetails->source_addr->in6.sin6_addr = in6addr_any;
+ sdetails->source_addr.in6.sin6_addr = in6addr_any;
safe_strncpy(sdetails->interface, sdetails->source, IF_NAMESIZE);
#else
return _("interface binding not supported");
@@ -1067,13 +1065,13 @@ int parse_server_next(struct server_details *sdetails)
/* Get address */
if (sdetails->addr_type == AF_INET)
- memcpy(&sdetails->addr->in.sin_addr,
+ memcpy(&sdetails->addr.in.sin_addr,
&((struct sockaddr_in *) sdetails->hostinfo->ai_addr)->sin_addr,
- sizeof(sdetails->addr->in.sin_addr));
+ sizeof(sdetails->addr.in.sin_addr));
else if (sdetails->addr_type == AF_INET6)
- memcpy(&sdetails->addr->in6.sin6_addr,
+ memcpy(&sdetails->addr.in6.sin6_addr,
&((struct sockaddr_in6 *) sdetails->hostinfo->ai_addr)->sin6_addr,
- sizeof(sdetails->addr->in6.sin6_addr));
+ sizeof(sdetails->addr.in6.sin6_addr));
/* Iterate to the next available address */
sdetails->valid = sdetails->hostinfo->ai_next != NULL;
@@ -1090,31 +1088,63 @@ int parse_server_next(struct server_details *sdetails)
return 0;
}
+static char *add_server_rev(struct server_details *sdetails,
+ const char *domain,
+ union all_addr *local_addr)
+{
+ if (sdetails->flags & SERV_LITERAL_ADDRESS)
+ {
+ if (!add_update_server_details(sdetails, domain, local_addr))
+ return _("error");
+ }
+ else
+ {
+ char *string;
+
+ while (parse_server_next(sdetails))
+ {
+ if ((string = parse_server_addr(sdetails)))
+ return string;
+
+ if (!add_update_server_details(sdetails, domain, local_addr))
+ return _("error");
+ }
+
+ if (sdetails->orig_hostinfo)
+ {
+ freeaddrinfo(sdetails->orig_hostinfo);
+ sdetails->orig_hostinfo = NULL;
+ }
+ }
+ return NULL;
+}
+
+static char *parse_server_rev(int from_file, char *server, struct server_details *sdetails)
+{
+ char *string;
+
+ if (!server)
+ sdetails->flags = SERV_LITERAL_ADDRESS;
+ else if ((string = parse_server(server, sdetails)))
+ return string;
+
+ if (from_file)
+ sdetails->flags |= SERV_FROM_FILE;
+
+ return NULL;
+}
+
static char *domain_rev4(int from_file, char *server, struct in_addr *addr4, int size)
{
int i, j;
- char *string;
+ char *string, *ret = NULL;
int msize;
- u16 flags = 0;
char domain[29]; /* strlen("xxx.yyy.zzz.ttt.in-addr.arpa")+1 */
- union mysockaddr serv_addr, source_addr;
- char interface[IF_NAMESIZE+1];
int count = 1, rem, addrbytes, addrbits;
struct server_details sdetails;
- memset(&sdetails, 0, sizeof(struct server_details));
- sdetails.addr = &serv_addr;
- sdetails.source_addr = &source_addr;
- sdetails.interface = interface;
- sdetails.flags = &flags;
-
- if (!server)
- flags = SERV_LITERAL_ADDRESS;
- else if ((string = parse_server(server, &sdetails)))
- return string;
-
- if (from_file)
- flags |= SERV_FROM_FILE;
+ if ((ret = parse_server_rev(from_file, server, &sdetails)))
+ return ret;
rem = size & 0x7;
addrbytes = (32 - size) >> 3;
@@ -1131,7 +1161,7 @@ static char *domain_rev4(int from_file, char *server, struct in_addr *addr4, int
if (rem != 0)
count = 1 << (8 - rem);
- for (i = 0; i < count; i++)
+ for (i = 0; !ret && i < count; i++)
{
*domain = 0;
string = domain;
@@ -1149,59 +1179,24 @@ static char *domain_rev4(int from_file, char *server, struct in_addr *addr4, int
sprintf(string, "in-addr.arpa");
- if (flags & SERV_LITERAL_ADDRESS)
- {
- if (!add_update_server(flags, &serv_addr, &source_addr, interface, domain, NULL))
- return _("error");
- }
- else
- {
- while (parse_server_next(&sdetails))
- {
- if ((string = parse_server_addr(&sdetails)))
- return string;
-
- if (!add_update_server(flags, &serv_addr, &source_addr, interface, domain, NULL))
- return _("error");
- }
-
- if (sdetails.orig_hostinfo)
- {
- freeaddrinfo(sdetails.orig_hostinfo);
- sdetails.orig_hostinfo = NULL;
- }
- }
+ ret = add_server_rev(&sdetails, domain, NULL);
}
- return NULL;
+ return ret;
}
static char *domain_rev6(int from_file, char *server, struct in6_addr *addr6, int size)
{
int i, j;
- char *string;
+ char *string, *ret = NULL;
int msize;
- u16 flags = 0;
char domain[73]; /* strlen("32*<n.>ip6.arpa")+1 */
- union mysockaddr serv_addr, source_addr;
- char interface[IF_NAMESIZE+1];
int count = 1, rem, addrbytes, addrbits;
struct server_details sdetails;
- memset(&sdetails, 0, sizeof(struct server_details));
- sdetails.addr = &serv_addr;
- sdetails.source_addr = &source_addr;
- sdetails.interface = interface;
- sdetails.flags = &flags;
-
- if (!server)
- flags = SERV_LITERAL_ADDRESS;
- else if ((string = parse_server(server, &sdetails)))
- return string;
+ if ((ret = parse_server_rev(from_file, server, &sdetails)))
+ return ret;
- if (from_file)
- flags |= SERV_FROM_FILE;
-
rem = size & 0x3;
addrbytes = (128 - size) >> 3;
addrbits = (128 - size) & 7;
@@ -1217,7 +1212,7 @@ static char *domain_rev6(int from_file, char *server, struct in6_addr *addr6, in
if (rem != 0)
count = 1 << (4 - rem);
- for (i = 0; i < count; i++)
+ for (i = 0; !ret && i < count; i++)
{
*domain = 0;
string = domain;
@@ -1237,31 +1232,10 @@ static char *domain_rev6(int from_file, char *server, struct in6_addr *addr6, in
sprintf(string, "ip6.arpa");
- if (flags & SERV_LITERAL_ADDRESS)
- {
- if (!add_update_server(flags, &serv_addr, &source_addr, interface, domain, NULL))
- return _("error");
- }
- else
- {
- while (parse_server_next(&sdetails))
- {
- if ((string = parse_server_addr(&sdetails)))
- return string;
-
- if (!add_update_server(flags, &serv_addr, &source_addr, interface, domain, NULL))
- return _("error");
- }
-
- if (sdetails.orig_hostinfo)
- {
- freeaddrinfo(sdetails.orig_hostinfo);
- sdetails.orig_hostinfo = NULL;
- }
- }
+ ret = add_server_rev(&sdetails, domain, NULL);
}
- return NULL;
+ return ret;
}
#ifdef HAVE_DHCP
@@ -2975,16 +2949,8 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma
u16 flags = 0;
char *err;
union all_addr addr;
- union mysockaddr serv_addr, source_addr;
- char interface[IF_NAMESIZE+1];
struct server_details sdetails;
- memset(&sdetails, 0, sizeof(struct server_details));
- sdetails.addr = &serv_addr;
- sdetails.source_addr = &source_addr;
- sdetails.interface = interface;
- sdetails.flags = &flags;
-
unhide_metas(arg);
/* split the domain args, if any and skip to the end of them. */
@@ -3033,7 +2999,7 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma
ret_err(err);
/* When source is set only use DNS records of the same type and skip all others */
- if (flags & SERV_HAS_SOURCE && sdetails.addr_type != sdetails.source_addr->sa.sa_family)
+ if (flags & SERV_HAS_SOURCE && sdetails.addr_type != sdetails.source_addr.sa.sa_family)
continue;
while (1)
@@ -3050,8 +3016,9 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma
if (option == 'A' && cur_domain[0] == '#' && cur_domain[1] == 0)
cur_domain[0] = 0;
}
-
- if (!add_update_server(flags, sdetails.addr, sdetails.source_addr, sdetails.interface, cur_domain, &addr))
+
+ sdetails.flags = flags;
+ if (!add_update_server_details(&sdetails, cur_domain, &addr))
ret_err(gen_err);
if (!lastdomain || cur_domain == lastdomain)
--
2.38.1
From a9cbe6d9e75680202e9df269b214da6c5a53250d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemen...@redhat.com>
Date: Fri, 25 Nov 2022 15:39:19 +0100
Subject: [PATCH 1/3] Ensure pointer is always invalidated after freeaddrinfo
freeaddrinfo is called conditionally. Ensure when the pointer is
invalidated, it no longer points to non-NULL memory and free again
cannot happen on the same structure.
---
src/dbus.c | 5 ++++-
src/option.c | 15 ++++++++++++---
2 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/src/dbus.c b/src/dbus.c
index fd5d1ca..2067ed9 100644
--- a/src/dbus.c
+++ b/src/dbus.c
@@ -462,7 +462,10 @@ static DBusMessage* dbus_read_servers_ex(DBusMessage *message, int strings)
}
if (sdetails.orig_hostinfo)
- freeaddrinfo(sdetails.orig_hostinfo);
+ {
+ freeaddrinfo(sdetails.orig_hostinfo);
+ sdetails.orig_hostinfo = NULL;
+ }
/* jump to next element in outer array */
dbus_message_iter_next(&array_iter);
diff --git a/src/option.c b/src/option.c
index 6a63268..efd5f49 100644
--- a/src/option.c
+++ b/src/option.c
@@ -1166,7 +1166,10 @@ static char *domain_rev4(int from_file, char *server, struct in_addr *addr4, int
}
if (sdetails.orig_hostinfo)
- freeaddrinfo(sdetails.orig_hostinfo);
+ {
+ freeaddrinfo(sdetails.orig_hostinfo);
+ sdetails.orig_hostinfo = NULL;
+ }
}
}
@@ -1251,7 +1254,10 @@ static char *domain_rev6(int from_file, char *server, struct in6_addr *addr6, in
}
if (sdetails.orig_hostinfo)
- freeaddrinfo(sdetails.orig_hostinfo);
+ {
+ freeaddrinfo(sdetails.orig_hostinfo);
+ sdetails.orig_hostinfo = NULL;
+ }
}
}
@@ -3059,7 +3065,10 @@ static int one_opt(int option, char *arg, char *errstr, char *gen_err, int comma
}
if (sdetails.orig_hostinfo)
- freeaddrinfo(sdetails.orig_hostinfo);
+ {
+ freeaddrinfo(sdetails.orig_hostinfo);
+ sdetails.orig_hostinfo = NULL;
+ }
break;
}
--
2.38.1
_______________________________________________
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
