From ed3745b51cd823d4a651e250050b2b674f055585 Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Fri, 7 Jul 2023 08:00:00 +0000 Subject: [PATCH 1/5] kbd-util: fix use of ERRNO_IS_RESOURCE() Given that ERRNO_IS_RESOURCE() also matches positive values, make sure this macro is not called with arguments that do not have errno semantics. In this case the argument passed to ERRNO_IS_RESOURCE() is the value returned by recurse_dir_at() which can legitimately return positive values without errno semantics, so fix this by moving the ERRNO_IS_RESOURCE() invocation to the branch where the return value is known to be negative. --- src/shared/kbd-util.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/shared/kbd-util.c b/src/shared/kbd-util.c index 3100862548..e6fe973471 100644 --- a/src/shared/kbd-util.c +++ b/src/shared/kbd-util.c @@ -145,11 +145,11 @@ int keymap_exists(const char *name) { &(struct recurse_dir_userdata) { .keymap_name = name, }); - if (r == -ENOENT) - continue; - if (ERRNO_IS_RESOURCE(r)) - return r; if (r < 0) { + if (r == -ENOENT) + continue; + if (ERRNO_IS_RESOURCE(r)) + return r; log_debug_errno(r, "Failed to read keymap list from %s, ignoring: %m", dir); continue; } From d5f8890bbf375075c7042b31ff6e79ad491df04c Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Fri, 7 Jul 2023 08:00:00 +0000 Subject: [PATCH 2/5] socket: fix use of ERRNO_IS_DISCONNECT() Given that ERRNO_IS_DISCONNECT() also matches positive values, make sure this macro is not called with arguments that do not have errno semantics. In this case the argument passed to ERRNO_IS_DISCONNECT() is the value returned by socket_acquire_peer() which can legitimately return 1 without errno semantics, so fix this by moving ERRNO_IS_DISCONNECT() invocation to the branch where the return value is known to be negative. --- src/core/socket.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/core/socket.c b/src/core/socket.c index d72194f20b..03b8cbd164 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -2358,10 +2358,12 @@ static void socket_enter_running(Socket *s, int cfd_in) { if (s->max_connections_per_source > 0) { r = socket_acquire_peer(s, cfd, &p); - if (ERRNO_IS_DISCONNECT(r)) - return; - if (r < 0) /* We didn't have enough resources to acquire peer information, let's fail. */ + if (r < 0) { + if (ERRNO_IS_DISCONNECT(r)) + return; + /* We didn't have enough resources to acquire peer information, let's fail. */ goto fail; + } if (r > 0 && p->n_ref > s->max_connections_per_source) { _cleanup_free_ char *t = NULL; From bb228f0ebc9b691ee2a871bffbf949936568f3ea Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Fri, 7 Jul 2023 08:00:00 +0000 Subject: [PATCH 3/5] sd-bus: fix use of ERRNO_IS_DISCONNECT() Given that ERRNO_IS_DISCONNECT() also matches positive values, make sure this macro is not called with arguments that do not have errno semantics. In this case the argument passed to ERRNO_IS_DISCONNECT() is the value returned by bus_socket_process_watch_bind(), bus_socket_process_opening(), and bus_socket_process_authenticating() which can legitimately return positive values without errno semantics, so fix this by moving the ERRNO_IS_DISCONNECT() invocation to the branch where the return value is known to be negative. --- src/libsystemd/sd-bus/sd-bus.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/libsystemd/sd-bus/sd-bus.c b/src/libsystemd/sd-bus/sd-bus.c index 2758309ac5..a250e7b81a 100644 --- a/src/libsystemd/sd-bus/sd-bus.c +++ b/src/libsystemd/sd-bus/sd-bus.c @@ -3284,11 +3284,13 @@ static int bus_process_internal(sd_bus *bus, sd_bus_message **ret) { assert_not_reached(); } - if (ERRNO_IS_DISCONNECT(r)) { - bus_enter_closing(bus); - r = 1; - } else if (r < 0) - return r; + if (r < 0) { + if (ERRNO_IS_DISCONNECT(r)) { + bus_enter_closing(bus); + r = 1; + } else + return r; + } if (ret) *ret = NULL; From 0bdea17c0aa37c4cdf586c072a7b35f8d0598cc3 Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Fri, 7 Jul 2023 08:00:00 +0000 Subject: [PATCH 4/5] resolved: fix use of ERRNO_IS_DISCONNECT() Given that ERRNO_IS_DISCONNECT() also matches positive values, make sure this macro is not called with arguments that do not have errno semantics. In this case the argument passed to ERRNO_IS_DISCONNECT() is the value returned by manager_recv() which can legitimately return 1 without errno semantics, so fix this by moving ERRNO_IS_DISCONNECT() invocation to the branch where the return value is known to be negative. --- src/resolve/resolved-dns-transaction.c | 33 ++++++++++++-------------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c index a5293357c0..323786896b 100644 --- a/src/resolve/resolved-dns-transaction.c +++ b/src/resolve/resolved-dns-transaction.c @@ -1367,25 +1367,22 @@ static int on_dns_packet(sd_event_source *s, int fd, uint32_t revents, void *use assert(t->scope); r = manager_recv(t->scope->manager, fd, DNS_PROTOCOL_DNS, &p); - if (ERRNO_IS_DISCONNECT(r)) { - usec_t usec; - - /* UDP connection failures get reported via ICMP and then are possibly delivered to us on the - * next recvmsg(). Treat this like a lost packet. */ - - log_debug_errno(r, "Connection failure for DNS UDP packet: %m"); - assert_se(sd_event_now(t->scope->manager->event, CLOCK_BOOTTIME, &usec) >= 0); - dns_server_packet_lost(t->server, IPPROTO_UDP, t->current_feature_level); - - dns_transaction_close_connection(t, /* use_graveyard = */ false); - - if (dns_transaction_limited_retry(t)) /* Try a different server */ - return 0; - - dns_transaction_complete_errno(t, r); - return 0; - } if (r < 0) { + if (ERRNO_IS_DISCONNECT(r)) { + usec_t usec; + + /* UDP connection failures get reported via ICMP and then are possibly delivered to us on the + * next recvmsg(). Treat this like a lost packet. */ + + log_debug_errno(r, "Connection failure for DNS UDP packet: %m"); + assert_se(sd_event_now(t->scope->manager->event, CLOCK_BOOTTIME, &usec) >= 0); + dns_server_packet_lost(t->server, IPPROTO_UDP, t->current_feature_level); + + dns_transaction_close_connection(t, /* use_graveyard = */ false); + + if (dns_transaction_limited_retry(t)) /* Try a different server */ + return 0; + } dns_transaction_complete_errno(t, r); return 0; } From fce846e0ace95920155088a7e70dac914e437e68 Mon Sep 17 00:00:00 2001 From: "Dmitry V. Levin" Date: Fri, 7 Jul 2023 08:00:00 +0000 Subject: [PATCH 5/5] test: fix use of ERRNO_IS_PRIVILEGE() Given that ERRNO_IS_PRIVILEGE() also matches positive values, make sure this macro is not called with arguments that do not have errno semantics. In this case the arguments passed to ERRNO_IS_PRIVILEGE() are the values returned by read_one_line_file() which can legitimately return positive values without errno semantics, so fix this by moving ERRNO_IS_PRIVILEGE() invocations to the branches where the return values are known to be negative. --- src/test/test-capability.c | 4 ++-- src/test/test-fileio.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/test-capability.c b/src/test/test-capability.c index bfdba4f294..a45e06db22 100644 --- a/src/test/test-capability.c +++ b/src/test/test-capability.c @@ -39,7 +39,7 @@ static void test_last_cap_file(void) { int r; r = read_one_line_file("/proc/sys/kernel/cap_last_cap", &content); - if (r == -ENOENT || ERRNO_IS_PRIVILEGE(r)) /* kernel pre 3.2 or no access */ + if (r == -ENOENT || (r < 0 && ERRNO_IS_PRIVILEGE(r))) /* kernel pre 3.2 or no access */ return; assert_se(r >= 0); @@ -235,7 +235,7 @@ static void test_ensure_cap_64_bit(void) { int r; r = read_one_line_file("/proc/sys/kernel/cap_last_cap", &content); - if (r == -ENOENT || ERRNO_IS_PRIVILEGE(r)) /* kernel pre 3.2 or no access */ + if (r == -ENOENT || (r < 0 && ERRNO_IS_PRIVILEGE(r))) /* kernel pre 3.2 or no access */ return; assert_se(r >= 0); diff --git a/src/test/test-fileio.c b/src/test/test-fileio.c index 07f4355142..51c8d8ee88 100644 --- a/src/test/test-fileio.c +++ b/src/test/test-fileio.c @@ -490,7 +490,7 @@ TEST(write_string_file_verify) { int r; r = read_one_line_file("/proc/version", &buf); - if (ERRNO_IS_PRIVILEGE(r)) + if (r < 0 && ERRNO_IS_PRIVILEGE(r)) return; assert_se(r >= 0); assert_se(buf2 = strjoin(buf, "\n"));