resolved: don't disable the listening socket when a datagram is dropped
manager_recv() sizes its buffer with next_datagram_size_fd(), which peeks at the datagram with MSG_PEEK|MSG_TRUNC. That peek makes the kernel verify the UDP checksum - which is the whole point of using it over FIONREAD - and hence also drop the datagram when the checksum is bad. The datagram that woke the event source can thus be gone by the time we peek at it, and the peek returns EAGAIN. manager_recv() returned that error, while the recvmsg_safe() below, which can fail the same way, already treats it as "no packet". The callers on the mDNS, LLMNR and stub listening sockets pass a negative return on to the event loop, which disables the event source, so a single datagram with a bad checksum - something anyone on the link can send - stops resolved from receiving on that socket for good. on_dns_packet() completes the transaction with -EAGAIN instead of retrying it. So return 0 on a transient error here too, like the other users of the helper on an event source do. Disconnect errors are left as they are: they show up here from ICMP errors on the connected UDP socket, and on_dns_packet() handles them.
This commit is contained in:
committed by
Yu Watanabe
parent
175b52e294
commit
4a25049a9e
@@ -123,6 +123,9 @@ executables += [
|
||||
resolve_test_template + {
|
||||
'sources' : files('test-resolved-etc-hosts.c'),
|
||||
},
|
||||
resolve_test_template + {
|
||||
'sources' : files('test-resolved-manager.c'),
|
||||
},
|
||||
resolve_test_template + {
|
||||
'sources' : files('test-resolved-packet.c'),
|
||||
},
|
||||
|
||||
@@ -956,6 +956,13 @@ int manager_recv(Manager *m, int fd, DnsProtocol protocol, DnsPacket **ret) {
|
||||
assert(ret);
|
||||
|
||||
ms = next_datagram_size_fd(fd);
|
||||
/* The MSG_PEEK recv() in next_datagram_size_fd() validates the datagram's checksum, and drops it if
|
||||
* it is bad. Hence we can end up here with nothing left to read even though the event source said
|
||||
* otherwise, and must treat that like the recvmsg_safe() below does: as "no packet", not as a
|
||||
* failure. Our callers propagate a failure to the event loop, which would then turn the listening
|
||||
* socket's event source off for good. */
|
||||
if (ERRNO_IS_NEG_TRANSIENT(ms))
|
||||
return 0;
|
||||
if (ms < 0)
|
||||
return ms;
|
||||
|
||||
|
||||
@@ -0,0 +1,32 @@
|
||||
/* SPDX-License-Identifier: LGPL-2.1-or-later */
|
||||
|
||||
#include <sys/socket.h>
|
||||
|
||||
#include "dns-packet.h"
|
||||
#include "fd-util.h"
|
||||
#include "resolved-manager.h"
|
||||
#include "tests.h"
|
||||
|
||||
/* ================================================================
|
||||
* manager_recv()
|
||||
* ================================================================ */
|
||||
|
||||
TEST(manager_recv_nothing_to_read) {
|
||||
Manager manager = {};
|
||||
_cleanup_(dns_packet_unrefp) DnsPacket *p = NULL;
|
||||
_cleanup_close_ int fd = -EBADF;
|
||||
|
||||
/* The event source may hand us a socket whose datagram is gone by the time we look at it: the
|
||||
* MSG_PEEK recv() in next_datagram_size_fd() verifies the UDP checksum and drops the datagram if it
|
||||
* is bad. That must be reported as "no packet", not as an error, as our callers pass an error on to
|
||||
* the event loop, which then turns the listening socket's event source off. An empty non-blocking
|
||||
* socket puts us in exactly that situation. */
|
||||
|
||||
fd = socket(AF_INET, SOCK_DGRAM|SOCK_NONBLOCK|SOCK_CLOEXEC, 0);
|
||||
ASSERT_OK_ERRNO(fd);
|
||||
|
||||
ASSERT_OK_ZERO(manager_recv(&manager, fd, DNS_PROTOCOL_DNS, &p));
|
||||
ASSERT_NULL(p);
|
||||
}
|
||||
|
||||
DEFINE_TEST_MAIN(LOG_DEBUG);
|
||||
Reference in New Issue
Block a user