Tomas Krizek [Tue, 30 Mar 2021 17:24:08 +0000 (19:24 +0200)]
daemon/http: fix memleak if http_write_pkt() fails
This can happen for example when we want to send an answer, but the
http stream (or the connection?) is already closed.
Direct leak of 48 byte(s) in 1 object(s) allocated from:
#0 0x7f5ad2445459 in __interceptor_malloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cpp:145
#1 0x55c0db3fc442 in http_write_pkt ../daemon/http.c:610
#2 0x55c0db3fc882 in http_write ../daemon/http.c:651
#3 0x55c0db3e9bb1 in qr_task_send ../daemon/worker.c:700
#4 0x55c0db3ee86c in qr_task_finalize ../daemon/worker.c:1321
#5 0x55c0db3f0123 in qr_task_step ../daemon/worker.c:1633
#6 0x55c0db3f0982 in worker_submit ../daemon/worker.c:1755
#7 0x55c0db3d992a in session_wirebuf_process ../daemon/session.c:759
#8 0x55c0db3c5f01 in udp_recv ../daemon/io.c:89
#9 0x7f5ad22b0e0e (/usr/lib/libuv.so.1+0x20e0e)
Tomas Krizek [Thu, 1 Apr 2021 15:04:10 +0000 (17:04 +0200)]
daemon/http: improve handling of stream errors
The on_frame_recv() callback ins't guaranteed to be called by nghttp2.
This can happen e.g. in a case when nghttp2 issues a PROTOCOL_ERROR
RST_STREAM frame. Previously, it would leave the connection in a
stream-processing state, making it completely useless.
While this guarantees a cleanup will be called eventually, some streams
may still get ignored due to the order of various callbacks and data
processing procedures. Still, it's better than the previous
implementation.
murmurhash3.c:43:40: runtime error: addition of unsigned offset
to 0x7ffce41c2014 overflowed to 0x7ffce41c2000
The `i` was used in a super-ugly way; I suspect the only reason was
to optimize that end-loop condition was zero comparison *vomit*
Vladimír Čunát [Tue, 6 Apr 2021 15:28:52 +0000 (17:28 +0200)]
treewide: fix unaligned access
Some less common HW (not x86, usually ARM) doesn't tolerate unaligned
access to memory and it's breakage of C as well.
It's easiest to check by meson's -Db_sanitize=undefined (on any HW).
I pushed millions of real-life QNAME+QTYPE queries over UDP in default
mode and the sanitizer seems clear now.
Vladimír Čunát [Fri, 26 Mar 2021 10:58:42 +0000 (11:58 +0100)]
clear kr_query::flags.CACHED
I suspect there's an edge case where cache thinks it provided enough
data but iterator (or who) disagrees and resolution continues.
We observed (flags.CACHED == true) even when processing a reply from
internet, and that could be confusing and even trigger a segfault.
Clearing the flag sounds OK semantically; it never meant that no cached
data have been used within the kr_query (e.g. zone cut, DS/DNSKEY, ...)
Vladimír Čunát [Tue, 6 Apr 2021 10:11:41 +0000 (12:11 +0200)]
cache: fix caching NSEC ranges with \000 in *ending*
Our aggressive NSEC cache doesn't handle these well and the case
with only the end-label being like this was forgotten.
See the parent commit for a test case.
Vladimír Čunát [Fri, 2 Apr 2021 08:59:32 +0000 (10:59 +0200)]
validate: fix on some NSEC ranges with zero byte(s)
Example case: denying existence of ok.rdns.dev by
oj\255.rdns.dev. NSEC ok\000.rdns.dev.
This NSEC end was incorrectly ordered with the QNAME.
https://gitter.im/CZ-NIC/knot-resolver?at=606055b82beb1e1da3d73892
Štěpán Balážik [Wed, 17 Mar 2021 14:53:33 +0000 (15:53 +0100)]
selection: cap the timeout value when probing a random server
This patch caps the timeout set on UDP queries to servers chosen in the
EXPLORE phase of the selection algorithm to two times the timeout that
would be set if we were EXPLOITing.
This measns that we no longer spend an unreasonable amount of time
probing servers that are probably dead anyway while ensuring that we do
probe them from time to time to check if they didn't come to life.
If the timeout value is capped and the server fails to respond, we don't
punish the server for it i.e. we don't cache the timeout.
Vladimír Čunát [Tue, 16 Mar 2021 09:39:50 +0000 (10:39 +0100)]
utils/cache_gc: fix crashes/assertions on RTT entries
I missed some parts when finishing this. I should've tested it better.
GC would hit assertions or NULL dereferences when removing entries,
and eventually that would lead to cache overflowing (and getting
cleared).
Vladimír Čunát [Thu, 11 Mar 2021 14:23:28 +0000 (15:23 +0100)]
dnstap: don't break request resolution on dnstap errors
This isn't a regression of 5.3.0 changes.
Layer functions are supposed to return new values for ctx->state,
but here we were sometimes returning kr_error(EFOO) which altered
processing of the request.
Our case: answers directly from policy module would not end up
finishing the request and we'd hit an assert at the end of processing.
Štěpán Balážik [Thu, 18 Feb 2021 11:10:26 +0000 (12:10 +0100)]
lib/selection{,_iter}.c: allow switching back to UDP
Switching to TCP instead of querying very slow servers over UDP has had
unwanted side effect – we would sometimes get stuck with a server
permanently switched to TCP. And if the server happens to not reply over
TCP we were in trouble.
Therefore after we TCP connect fails or timeouts we provide one last
chance for the server over UDP. This will not prevent the next request
to try TCP again on this server again, but we don't care because
DNS MUST ******* work over TCP.
Vladimír Čunát [Fri, 12 Feb 2021 09:06:25 +0000 (10:06 +0100)]
daemon/udp_queue: drop the error logging
We should do this for all transports and probably just in verbose mode.
We were printing lots of these on Turris OS (for one user at least):
https://forum.turris.cz/t/5-1-8-kresd-throwing-many-errors-in-var-log-messages/14775
EACCESS in particular apparently may happen (on Linux) when the network
is "unavailable", EPERM because of firewall/netfilter:
https://stackoverflow.com/a/23869102
Vladimír Čunát [Wed, 10 Feb 2021 11:56:14 +0000 (12:56 +0100)]
modules/{http,watchdog}: fix stability problems
As first noted in commit d1a229ae9, in some cases we do call chains that
are not supported for JIT in LuaJIT.
I'm not 100% sure all of these are needed to comply, but the functions
here are really small and probably not to be that heavily used,
so I don't think it will be costly to interpret them
(and avoiding crashes is more important).
In my tests this fixed occasional crashes when using http://*/trace/*
Vladimír Čunát [Thu, 28 Jan 2021 10:37:05 +0000 (11:37 +0100)]
policy.ANSWER: minor fixes, mainly around NODATA answers
- return SOA in NODATA answers and allow customizing it
- only call ensure_answer() if really generating an answer
(otherwise we might e.g. deplete XDP buffers, in extreme cases)
Vladimír Čunát [Mon, 1 Feb 2021 09:09:16 +0000 (10:09 +0100)]
when FORMERR comes, differentiate based on OPT
In particular, non-support of EDNS is implied iff FORMERR without OPT
comes. If OPT is there, one possibility is that there was something
wrong in the OPT that *we* sent, but it seems much more likely that
this particular server is just bad and we want to try another one.
https://tools.ietf.org/html/rfc6891#section-7
In particular, we would be in trouble if we dropped OPT in a zone
that is covered by DNSSEC.
Vladimír Čunát [Mon, 1 Feb 2021 08:57:46 +0000 (09:57 +0100)]
lib/selection: rename to *_FORMERR for consistency
It's now consistent with KNOT_RCODE_FORMERR and the official name
https://www.iana.org/assignments/dns-parameters/dns-parameters.xhtml#dns-parameters-6
projects / thirdparty / knot-resolver.git / log
