Re: [Dnsmasq-discuss] [PATCH] Refuse to start with EADDRINUSE in --bind-dynamic mode
Isn't this sufficient to fix the problem?
Not calling die() when bind-dynamic is set is intended to handle the
case that bind returns EADDRNOTAVAIL because you've configured
--listen-address=1.2.3.4 but there's not a local interface with that
address. dnsmasq runs anyway in the expectation that such an interface
will appear in the future and a socket will be bound then.
I don't think there's a die()/syslog() conflict at all.
diff --git a/src/network.c b/src/network.c
index ca9fada..db1d528 100644
--- a/src/network.c
+++ b/src/network.c
@@ -924,7 +924,7 @@ static int make_sock(union mysockaddr *addr, int
type, int dienow)
{
/* failure to bind addresses given by --listen-address at
this point
is OK if we're doing bind-dynamic */
- if (!option_bool(OPT_CLEVERBIND))
+ if (!option_bool(OPT_CLEVERBIND) || errno == EADDRINUSE)
die(s, daemon->addrbuff, EC_BADNET);
}
else
Cheers,
Simon.
On 22/11/2023 19:27, Petr Menšík wrote:
Hello everyone,
I have received error report RHEL-16398 [1], which I think makes sense
to fix even in the lastest version. I believe it allows non-intentional
another instance running without error. What is worse, it does not even
show any warning that initialization is incomplete.
Of course the problem at start is those errors happen in time when no
log is available. I think that can be fixed easily by using stderr at
that time. That is patch #1.
Second makes EADDRNOTAVAIL bind errors still hidden, but prints all
other errors at least to stderr. On a system with systemd that should
make it present in journalctl -u dnsmasq anyway. EADDRINUSE is made
fatal, because that would not be usually handled by new addresses added
later. If there is a need to start another dnsmasq instance without TCP
listeners, I think that should be specified more explicitly. Makes
EADDRINUSE fatal the same way as with --bind-interfaces.
Would you find any other errors, which should be hidden or made fatal?
What would you think of those changes?
1. https://issues.redhat.com/browse/RHEL-16398
___
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] runtime error: left shift of 128 by 24 places cannot be represented in type 'int'
Thanks for that. I don't think this bug has any practical effect. If the hash is calculated wrongly, it's only ever compared to another has calculated the same way, so the code will still work as designed. I think that this patch fixes things. Please could you test? https://thekelleys.org.uk/gitweb/?p=dnsmasq.git;a=commit;h=65c2d6afd67a032f45f40d7e4d620f5d73e5f07d Cheers, Simon. On 22/11/2023 06:44, 吴非凡 wrote: Hello dnsmasq experts, There is a runtime error in the latest version of dnsmasq. If we compile dnsmasq with UBSan. and send it with ``` echo -n 'c97b0101047465737403636f6d020001' | xxd -r -p | nc -u target_addr target_port ``` And then we'll get a error message: ``` hash-questions.c:168:21: runtime error: left shift of 128 by 24 places cannot be represented in type 'int' #0 0x6f0252 in sha256_transform /root/dnsmasq/src/hash-questions.c:168:21 #1 0x6eea82 in sha256_final /root/dnsmasq/src/hash-questions.c:267:3 #2 0x6eea82 in hash_questions /root/dnsmasq/src/hash-questions.c:127:3 #3 0x56d2e9 in forward_query /root/dnsmasq/src/forward.c:179:16 #4 0x580327 in receive_query /root/dnsmasq/src/forward.c:1877:12 #5 0x5c977f in check_dns_listeners /root/dnsmasq/src/dnsmasq.c:1838:2 #6 0x5c0c95 in main /root/dnsmasq/src/dnsmasq.c:1259:7 #7 0x7f3e2ec1dd8f (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f) #8 0x7f3e2ec1de3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f) #9 0x4206d4 in _start (/root/dnsmasq/src/dnsmasq+0x4206d4) ``` here is my conf and cli args: ``` no-daemon no-resolv interface=lo bind-interfaces no-hosts server=/baidu.com/223.5.5.5 server=/thekelleys.org.uk/114.114.114.114 address=/test.com/233.233.233.233 cache-size=150 ``` ``` dnsmasq -p 5355 -d -N -q -R -h -n -y -C config.conf ``` Kind regards, Feifan Wu ___ 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
[Dnsmasq-discuss] [PATCH] Refuse to start with EADDRINUSE in --bind-dynamic mode
Hello everyone,
I have received error report RHEL-16398 [1], which I think makes sense
to fix even in the lastest version. I believe it allows non-intentional
another instance running without error. What is worse, it does not even
show any warning that initialization is incomplete.
Of course the problem at start is those errors happen in time when no
log is available. I think that can be fixed easily by using stderr at
that time. That is patch #1.
Second makes EADDRNOTAVAIL bind errors still hidden, but prints all
other errors at least to stderr. On a system with systemd that should
make it present in journalctl -u dnsmasq anyway. EADDRINUSE is made
fatal, because that would not be usually handled by new addresses added
later. If there is a need to start another dnsmasq instance without TCP
listeners, I think that should be specified more explicitly. Makes
EADDRINUSE fatal the same way as with --bind-interfaces.
Would you find any other errors, which should be hidden or made fatal?
What would you think of those changes?
1. https://issues.redhat.com/browse/RHEL-16398
--
Petr Menšík
Software Engineer, RHEL
Red Hat, http://www.redhat.com/
PGP: DFCF908DB7C87E8E529925BC4931CA5B6C9FC5CB
From 207e9f4241c79b703320ae3568208e3a47cd25ef Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?=
Date: Wed, 22 Nov 2023 20:04:14 +0100
Subject: [PATCH 2/2] Prevent starting another instance with --bind-dynamic
Previously startup bind() errors were silently dropped when starting with
--bind-dynamic. Make even in that mode EADDRINUSE error fatal to prevent
running another instance with half-initialized listeners.
On the other hand still hide address not available error, which is very
likely the reason for using bind-dynamic. Expect the address specified
will just appear later.
---
src/network.c | 19 +++
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/src/network.c b/src/network.c
index ca9fada..f18be24 100644
--- a/src/network.c
+++ b/src/network.c
@@ -921,13 +921,9 @@ static int make_sock(union mysockaddr *addr, int type, int dienow)
errno = errsave;
if (dienow)
- {
- /* failure to bind addresses given by --listen-address at this point
- is OK if we're doing bind-dynamic */
- if (!option_bool(OPT_CLEVERBIND))
- die(s, daemon->addrbuff, EC_BADNET);
- }
- else
+ die(s, daemon->addrbuff, EC_BADNET);
+ else if (!option_bool(OPT_CLEVERBIND)
+ || (option_bool(OPT_CLEVERBIND) && errno != EADDRNOTAVAIL))
my_syslog(LOG_WARNING, s, daemon->addrbuff, strerror(errno));
return -1;
@@ -940,7 +936,14 @@ static int make_sock(union mysockaddr *addr, int type, int dienow)
goto err;
if ((rc = bind(fd, (struct sockaddr *)addr, sa_len(addr))) == -1)
-goto err;
+{
+ if (dienow && option_bool(OPT_CLEVERBIND) && errno != EADDRINUSE)
+ /* failure to bind addresses given by --listen-address at this point
+ is OK if we're doing bind-dynamic, except EADDRINUSE */
+ dienow = 0;
+
+ goto err;
+}
if (type == SOCK_STREAM)
{
--
2.42.0
From c1982e364c01a00c8b6454b677ae0dbe1ea4a382 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?=
Date: Wed, 22 Nov 2023 20:00:01 +0100
Subject: [PATCH 1/2] Make stderr logging enabled until normal logging starts
Some kinds of errors like socket bind errors are done before dnsmasq
starts regular logging facility. Do not have those messages disappear,
but log them to stderr. As soon as log_start is called, that is resetted
according to configuration settings.
---
src/log.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/log.c b/src/log.c
index 77032fb..6edcc09 100644
--- a/src/log.c
+++ b/src/log.c
@@ -35,7 +35,7 @@
/* defaults in case we die() before we log_start() */
static int log_fac = LOG_DAEMON;
static int log_stderr = 0;
-static int echo_stderr = 0;
+static int echo_stderr = 1;
static int log_fd = -1;
static int log_to_file = 0;
static int entries_alloced = 0;
--
2.42.0
___
Dnsmasq-discuss mailing list
Dnsmasq-discuss@lists.thekelleys.org.uk
https://lists.thekelleys.org.uk/cgi-bin/mailman/listinfo/dnsmasq-discuss
