diff --git a/CMakeLists.txt b/CMakeLists.txt index 60b6ec1..875d1fc 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -131,13 +131,20 @@ if(USE_CLANG_TIDY) message(STATUS "clang-tidy not found.") else() message(STATUS "clang-tidy found: ${CLANG_TIDY_EXE}") - set(CLANG_TIDY_CHECKS "-checks=*,-readability-identifier-length,-altera-unroll-loops,-concurrency-mt-unsafe,") - string(APPEND CLANG_TIDY_CHECKS "-bugprone-easily-swappable-parameters,-*magic-numbers,-hicpp-signed-bitwise,") - string(APPEND CLANG_TIDY_CHECKS "-readability-function-cognitive-complexity,-altera-id-dependent-backward-branch,") - string(APPEND CLANG_TIDY_CHECKS "-misc-include-cleaner,-llvmlibc-restrict-system-libc-headers,") + # Opt in to the check modules relevant to portable C instead of "*": the + # wildcard also enables domain-specific modules (altera-* for FPGA/OpenCL + # kernels, llvmlibc-*, android-*, ...) whose fixits have previously written + # inappropriate attributes into this codebase via -fix (see altera-struct- + # pack-align: extended alignment is UB on calloc'd structs, and packing + # misaligns embedded sockaddr_storage/libev watchers). + set(CLANG_TIDY_CHECKS "-checks=-*,bugprone-*,cert-*,clang-analyzer-*,concurrency-*,") + string(APPEND CLANG_TIDY_CHECKS "misc-*,performance-*,portability-*,readability-*,") + string(APPEND CLANG_TIDY_CHECKS "-readability-identifier-length,-concurrency-mt-unsafe,") + string(APPEND CLANG_TIDY_CHECKS "-bugprone-easily-swappable-parameters,-*magic-numbers,") + string(APPEND CLANG_TIDY_CHECKS "-readability-function-cognitive-complexity,-misc-include-cleaner,") string(APPEND CLANG_TIDY_CHECKS "-clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,") string(APPEND CLANG_TIDY_CHECKS "-*function-size,-clang-diagnostic-*variadic-macro-arguments*") - set(DO_CLANG_TIDY "${CLANG_TIDY_EXE}" "-fix" "-fix-errors" "${CLANG_TIDY_CHECKS}") + set(DO_CLANG_TIDY "${CLANG_TIDY_EXE}" "${CLANG_TIDY_CHECKS}") endif() else() message(STATUS "Not using clang-tidy.") diff --git a/src/dns_listener_tcp.c b/src/dns_listener_tcp.c index da94cf0..3b6d101 100644 --- a/src/dns_listener_tcp.c +++ b/src/dns_listener_tcp.c @@ -55,7 +55,7 @@ struct tcp_client_s { ev_timer timer_watcher; struct tcp_client_s * next; -} __attribute__((packed)) __attribute__((aligned(128))); +}; struct dns_listener_tcp_s { dns_listener_t base; @@ -73,7 +73,7 @@ struct dns_listener_tcp_s { uint16_t client_count; uint16_t client_limit; struct tcp_client_s * clients; -} __attribute__((packed)) __attribute__((aligned(128))); +}; static void remove_client(struct tcp_client_s * client) { @@ -332,7 +332,7 @@ static void tcp_respond(dns_listener_t *self, struct sockaddr *raddr, // Limit response size to prevent overflow when accounting for the 2-byte // length prefix. The total on-wire size would be resp_len + sizeof(uint16_t). if (resp_len < DNS_HEADER_LENGTH || resp_len > TCP_DNS_MAX_PAYLOAD) { - WLOG("Malformed response received, invalid length: %u", resp_len); + WLOG("Malformed response received, invalid length: %zu", resp_len); return; } const uint16_t response_id = ntohs(*((uint16_t*)resp)); @@ -355,7 +355,7 @@ static void tcp_respond(dns_listener_t *self, struct sockaddr *raddr, // below is a blocking syscall (not an event loop yield). If remove_client() // is called due to send errors, the function returns immediately. - DLOG_CLIENT("Sending %u bytes", resp_len); + DLOG_CLIENT("Sending %zu bytes", resp_len); // send length of response uint16_t resp_size = htons((uint16_t)resp_len); diff --git a/src/dns_listener_udp.c b/src/dns_listener_udp.c index 561e69c..96cef08 100644 --- a/src/dns_listener_udp.c +++ b/src/dns_listener_udp.c @@ -21,7 +21,7 @@ typedef struct dns_listener_udp_s { dns_request_fn cb; void *cb_data; -} __attribute__((aligned(128))) dns_listener_udp_t; +} dns_listener_udp_t; // Creates and binds a listening UDP socket for incoming requests. static int get_listen_sock(struct addrinfo *listen_addrinfo) { @@ -91,7 +91,7 @@ static void udp_respond(dns_listener_t *self, struct sockaddr *raddr, dns_listener_udp_t *d = (dns_listener_udp_t *)self; if (dns_resp_len < DNS_HEADER_LENGTH) { - WLOG("Malformed response received, invalid length: %u", dns_resp_len); + WLOG("Malformed response received, invalid length: %zu", dns_resp_len); return; } dns_truncate_for_udp(dns_req, dns_req_len, dns_resp, &dns_resp_len); diff --git a/src/dns_poller.c b/src/dns_poller.c index 8f5f030..211019a 100644 --- a/src/dns_poller.c +++ b/src/dns_poller.c @@ -237,7 +237,10 @@ void dns_poller_init(dns_poller_t *d, struct ev_loop *loop, } d->loop = loop; - d->hostname = hostname; + d->hostname = strdup(hostname); + if (d->hostname == NULL) { + FLOG("Out of mem"); + } d->family = family; set_bootstrap_source_addr(d->ares, source_addr, family); d->cb = cb; @@ -269,5 +272,6 @@ void dns_poller_cleanup(dns_poller_t *d) { ares_destroy(d->ares); ev_timer_stop(d->loop, &d->timer); ares_library_cleanup(); + free(d->hostname); free(d->io_events); } diff --git a/src/dns_poller.h b/src/dns_poller.h index 6c411ce..96792b6 100644 --- a/src/dns_poller.h +++ b/src/dns_poller.h @@ -21,7 +21,7 @@ typedef void (*dns_poller_cb)(const char* hostname, void *data, typedef struct { ares_channel ares; struct ev_loop *loop; - const char *hostname; + char *hostname; // owned; strdup'd in init, freed in cleanup int family; // AF_UNSPEC for IPv4 or IPv6, AF_INET for IPv4 only. dns_poller_cb cb; int polling_interval; @@ -40,8 +40,7 @@ typedef struct { // `source_addr` optionally binds bootstrap DNS lookups to a specific IP. // `family` should be AF_INET for IPv4 or AF_UNSPEC for both IPv4 and IPv6. // -// Note: hostname *not* copied. It should remain valid until -// dns_poller_cleanup called. +// Note: hostname is copied; the caller's buffer need not outlive this call. void dns_poller_init(dns_poller_t *d, struct ev_loop *loop, const char *bootstrap_dns, int bootstrap_dns_polling_interval, diff --git a/src/dns_truncate.c b/src/dns_truncate.c index babe65f..e9dcfe7 100644 --- a/src/dns_truncate.c +++ b/src/dns_truncate.c @@ -75,9 +75,9 @@ static uint16_t get_edns_udp_size(const char *dns_req, const size_t dns_req_len) * 3. EDNS0/OPT Preservation (RFC 6891): * The OPT pseudo-RR (Type 41) is critical for extended error tracking, cookies, and DNSSEC signaling. * RFC 6891 mandates that OPT records should be preserved in truncated messages if they were present - * in the request. This function scans the Additional section, locates the OPT record, and uses - * memmove() to safely relocate it to sit directly flush against the end of the Question section, - * preserving it in the truncated response stream. + * in the request. This function scans the Additional section and removes every record except the + * OPT one; re-serializing via c-ares then leaves the OPT record directly after the Question + * section, preserving it in the truncated response stream. * * 4. Trusted Data Assumption: * DoH resolver response is considered trusted input, so assuming that it complies with RFCs @@ -142,7 +142,7 @@ static void truncate_to_size_limit(uint8_t *buf, size_t *buflen, size_t size_lim memcpy(buf, new_resp, new_resp_len); *buflen = new_resp_len; buf[2] |= 0x02; // set truncation flag - ILOG("%04hX: DNS response size truncated from %u to %u to keep %u limit", + ILOG("%04hX: DNS response size truncated from %zu to %zu to keep %zu limit", resp_id, old_size, new_resp_len, size_limit); } diff --git a/src/doh_proxy.c b/src/doh_proxy.c index 3ba694e..3b485eb 100644 --- a/src/doh_proxy.c +++ b/src/doh_proxy.c @@ -25,7 +25,7 @@ struct doh_proxy { // fallback resolver and risk a recursion through our own listener). uint8_t awaiting_bootstrap; doh_proxy_bootstrap_done_cb bootstrap_done_cb; -} __attribute__((aligned(64))); +}; // Per-request transient state. Lives from doh_proxy_handle_request to // https_resp_cb, when the response (or failure) returns from libcurl. @@ -36,7 +36,7 @@ typedef struct { struct sockaddr_storage raddr; char *dns_req; size_t dns_req_len; -} __attribute__((packed)) __attribute__((aligned(128))) doh_request_t; +} doh_request_t; doh_proxy_t * doh_proxy_create(struct ev_loop *loop, https_client_t *client, @@ -146,7 +146,9 @@ void doh_proxy_handle_resolver_update(const char *hostname, void *ctx, if (p->awaiting_bootstrap) { p->awaiting_bootstrap = 0; - p->bootstrap_done_cb(); + if (p->bootstrap_done_cb != NULL) { + p->bootstrap_done_cb(); + } } } @@ -162,7 +164,7 @@ static void doh_response_cb(void *data, char *buf, size_t buflen) { if (buf != NULL) { // NULL on timeout / DNS failure / similar. if (buflen < DNS_HEADER_LENGTH) { - WLOG("%04hX: Malformed response received, too short: %u", req_id, buflen); + WLOG("%04hX: Malformed response received, too short: %zu", req_id, buflen); } else { const uint16_t resp_id = ntohs(*((uint16_t*)buf)); if (req_id != resp_id) { diff --git a/src/main.c b/src/main.c index cbac43b..95aafd7 100644 --- a/src/main.c +++ b/src/main.c @@ -24,11 +24,16 @@ #include "options.h" #include "stat.h" -static int is_ipv4_address(char *str) { +static int is_ipv4_address(const char *str) { struct in6_addr addr; return inet_pton(AF_INET, str, &addr) == 1; } +static int is_ip_address(const char *str) { + struct in6_addr addr; + return is_ipv4_address(str) || inet_pton(AF_INET6, str, &addr) == 1; +} + enum url_type { URL_TYPE_ERROR, URL_TYPE_IP, @@ -305,6 +310,9 @@ int main(int argc, char *argv[]) { doh_proxy_set_port(proxy, port); if (opt.resolver_ip == NULL) { dns_poller = (dns_poller_t *)calloc(1, sizeof(dns_poller_t)); + if (dns_poller == NULL) { + FLOG("Out of mem"); + } doh_proxy_await_bootstrap(proxy, systemd_notify_ready); dns_poller_init(dns_poller, loop, opt.bootstrap_dns, opt.bootstrap_dns_polling_interval, opt.source_addr, @@ -312,6 +320,9 @@ int main(int argc, char *argv[]) { doh_proxy_handle_resolver_update, proxy); ILOG("DNS polling initialized for '%s'", hostname); } else { + if (!is_ip_address(opt.resolver_ip)) { + FLOG("Resolver IP override '%s' is not a valid IP literal", opt.resolver_ip); + } const size_t resolv_buf_len = strlen(hostname) + 1 + PORT_STR_LENGTH + 1 + strlen(opt.resolver_ip) + 1; char * resolv_buf = (char *)calloc(resolv_buf_len, sizeof(char)); (void)snprintf(resolv_buf, resolv_buf_len, "%s:%u:%s", hostname, port, opt.resolver_ip); @@ -323,6 +334,9 @@ int main(int argc, char *argv[]) { break; case URL_TYPE_IP: doh_proxy_set_port(proxy, port); + if (opt.resolver_ip != NULL) { + WLOG("Resolver URL already contains an IP address, ignoring -R %s", opt.resolver_ip); + } ILOG("Resolver prefix '%s' doesn't appear to contain a hostname. " "DNS polling disabled.", opt.resolver_url); systemd_notify_ready(); diff --git a/src/options.h b/src/options.h index 30784d2..4e2aea4 100644 --- a/src/options.h +++ b/src/options.h @@ -66,7 +66,7 @@ struct Options { // Number of logs to be kept by flight recorder int flight_recorder_size; -} __attribute__((aligned(128))); +}; typedef struct Options options_t; enum OptionsParseResult { diff --git a/src/ring_buffer.c b/src/ring_buffer.c index f356d1a..a3883c1 100644 --- a/src/ring_buffer.c +++ b/src/ring_buffer.c @@ -15,7 +15,7 @@ struct ring_buffer uint32_t size; uint32_t next; // next slot to use in storage uint8_t full; -} __attribute__((packed)) __attribute__((aligned(32))); +}; void ring_buffer_init(struct ring_buffer **rbp, uint32_t size) {