From 90293a9583f7bc54f61f677cb2b9f7b5ea99361f Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 24 Apr 2025 09:22:43 +0900 Subject: [PATCH 1/4] resolvectl: make three booleans for read_dns_server_one() into one enum --- src/resolve/resolvectl.c | 50 ++++++++++++++++++++-------------------- 1 file changed, 25 insertions(+), 25 deletions(-) diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index 98eadb3943..fa6ace9bec 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -1354,13 +1354,13 @@ static int reset_server_features(int argc, char **argv, void *userdata) { return 0; } -static int read_dns_server_one( - sd_bus_message *m, - bool with_ifindex, /* read "ifindex" reply that also carries an interface index */ - bool extended, /* read "extended" reply, i.e. with port number and server name */ - bool only_global, /* suppress entries with an (non-loopback) ifindex set (i.e. which are specific to some interface) */ - char **ret) { +typedef enum { + READ_DNS_WITH_IFINDEX = 1 << 0, /* read "ifindex" reply that also carries an interface index */ + READ_DNS_EXTENDED = 1 << 1, /* read "extended" reply, i.e. with port number and server name */ + READ_DNS_ONLY_GLOBAL = 1 << 2, /* suppress entries with an (non-loopback) ifindex set (i.e. which are specific to some interface) */ +} ReadDNSFlag; +static int read_dns_server_one(sd_bus_message *m, ReadDNSFlag flags, char **ret) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_free_ char *pretty = NULL; union in_addr_union a; @@ -1375,12 +1375,12 @@ static int read_dns_server_one( r = sd_bus_message_enter_container( m, 'r', - with_ifindex ? (extended ? "iiayqs" : "iiay") : - (extended ? "iayqs" : "iay")); + FLAGS_SET(flags, READ_DNS_WITH_IFINDEX) ? (FLAGS_SET(flags, READ_DNS_EXTENDED) ? "iiayqs" : "iiay") : + (FLAGS_SET(flags, READ_DNS_EXTENDED) ? "iayqs" : "iay")); if (r <= 0) return r; - if (with_ifindex) { + if (FLAGS_SET(flags, READ_DNS_WITH_IFINDEX)) { r = sd_bus_message_read(m, "i", &ifindex); if (r < 0) return r; @@ -1390,7 +1390,7 @@ static int read_dns_server_one( if (k < 0 && !sd_bus_error_has_name(&error, SD_BUS_ERROR_INVALID_ARGS)) return k; - if (extended) { + if (FLAGS_SET(flags, READ_DNS_EXTENDED)) { r = sd_bus_message_read(m, "q", &port); if (r < 0) return r; @@ -1410,7 +1410,7 @@ static int read_dns_server_one( return 1; } - if (only_global && ifindex > 0 && ifindex != LOOPBACK_IFINDEX) { + if (FLAGS_SET(flags, READ_DNS_ONLY_GLOBAL) && ifindex > 0 && ifindex != LOOPBACK_IFINDEX) { /* This one has an (non-loopback) ifindex set, and we were told to suppress those. Hence do so. */ *ret = NULL; return 1; @@ -1424,7 +1424,7 @@ static int read_dns_server_one( return 1; } -static int map_link_dns_servers_internal(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata, bool extended) { +static int map_link_dns_servers_internal(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata, ReadDNSFlag flags) { char ***l = ASSERT_PTR(userdata); int r; @@ -1432,14 +1432,14 @@ static int map_link_dns_servers_internal(sd_bus *bus, const char *member, sd_bus assert(member); assert(m); - r = sd_bus_message_enter_container(m, 'a', extended ? "(iayqs)" : "(iay)"); + r = sd_bus_message_enter_container(m, 'a', FLAGS_SET(flags, READ_DNS_EXTENDED) ? "(iayqs)" : "(iay)"); if (r < 0) return r; for (;;) { _cleanup_free_ char *pretty = NULL; - r = read_dns_server_one(m, /* with_ifindex= */ false, extended, /* only_global= */ false, &pretty); + r = read_dns_server_one(m, flags, &pretty); if (r < 0) return r; if (r == 0) @@ -1461,25 +1461,25 @@ static int map_link_dns_servers_internal(sd_bus *bus, const char *member, sd_bus } static int map_link_dns_servers(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) { - return map_link_dns_servers_internal(bus, member, m, error, userdata, false); + return map_link_dns_servers_internal(bus, member, m, error, userdata, /* flags = */ 0); } static int map_link_dns_servers_ex(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) { - return map_link_dns_servers_internal(bus, member, m, error, userdata, true); + return map_link_dns_servers_internal(bus, member, m, error, userdata, READ_DNS_EXTENDED); } static int map_link_current_dns_server(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) { assert(m); assert(userdata); - return read_dns_server_one(m, /* with_ifindex= */ false, /* extended= */ false, /* only_global= */ false, userdata); + return read_dns_server_one(m, /* flags = */ 0, userdata); } static int map_link_current_dns_server_ex(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) { assert(m); assert(userdata); - return read_dns_server_one(m, /* with_ifindex= */ false, /* extended= */ true, /* only_global= */ false, userdata); + return read_dns_server_one(m, READ_DNS_EXTENDED, userdata); } static int read_domain_one(sd_bus_message *m, bool with_ifindex, char **ret) { @@ -1905,7 +1905,7 @@ static int map_global_dns_servers_internal( sd_bus_message *m, sd_bus_error *error, void *userdata, - bool extended) { + ReadDNSFlag flags) { char ***l = ASSERT_PTR(userdata); int r; @@ -1914,14 +1914,14 @@ static int map_global_dns_servers_internal( assert(member); assert(m); - r = sd_bus_message_enter_container(m, 'a', extended ? "(iiayqs)" : "(iiay)"); + r = sd_bus_message_enter_container(m, 'a', FLAGS_SET(flags, READ_DNS_EXTENDED) ? "(iiayqs)" : "(iiay)"); if (r < 0) return r; for (;;) { _cleanup_free_ char *pretty = NULL; - r = read_dns_server_one(m, /* with_ifindex= */ true, extended, /* only_global= */ true, &pretty); + r = read_dns_server_one(m, flags, &pretty); if (r < 0) return r; if (r == 0) @@ -1943,19 +1943,19 @@ static int map_global_dns_servers_internal( } static int map_global_dns_servers(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) { - return map_global_dns_servers_internal(bus, member, m, error, userdata, /* extended= */ false); + return map_global_dns_servers_internal(bus, member, m, error, userdata, READ_DNS_WITH_IFINDEX | READ_DNS_ONLY_GLOBAL); } static int map_global_dns_servers_ex(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) { - return map_global_dns_servers_internal(bus, member, m, error, userdata, /* extended= */ true); + return map_global_dns_servers_internal(bus, member, m, error, userdata, READ_DNS_WITH_IFINDEX | READ_DNS_ONLY_GLOBAL | READ_DNS_EXTENDED); } static int map_global_current_dns_server(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) { - return read_dns_server_one(m, /* with_ifindex= */ true, /* extended= */ false, /* only_global= */ true, userdata); + return read_dns_server_one(m, READ_DNS_WITH_IFINDEX | READ_DNS_ONLY_GLOBAL, userdata); } static int map_global_current_dns_server_ex(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) { - return read_dns_server_one(m, /* with_ifindex= */ true, /* extended= */ true, /* only_global= */ true, userdata); + return read_dns_server_one(m, READ_DNS_WITH_IFINDEX | READ_DNS_ONLY_GLOBAL | READ_DNS_EXTENDED, userdata); } static int map_global_domains(sd_bus *bus, const char *member, sd_bus_message *m, sd_bus_error *error, void *userdata) { From 06a5d24d78d9bc24028300ae82ba0ab8cd8363d6 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 24 Apr 2025 09:49:50 +0900 Subject: [PATCH 2/4] resolvectl: skip reading remaining elements when an invalid address is specified Previous code assumed that both family and address are read when -EINVAL, hence the following read of port and name could succeed. Let's drop the assumption, and skip reading remaining elements. No functional change, just refactoring and preparation for later change. --- src/resolve/resolvectl.c | 35 +++++++++++++++++++++++++---------- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/src/resolve/resolvectl.c b/src/resolve/resolvectl.c index fa6ace9bec..f517d149dd 100644 --- a/src/resolve/resolvectl.c +++ b/src/resolve/resolvectl.c @@ -1366,7 +1366,7 @@ static int read_dns_server_one(sd_bus_message *m, ReadDNSFlag flags, char **ret) union in_addr_union a; const char *name = NULL; int32_t ifindex = 0; - int family, r, k; + int family, r; uint16_t port = 0; assert(m); @@ -1386,9 +1386,30 @@ static int read_dns_server_one(sd_bus_message *m, ReadDNSFlag flags, char **ret) return r; } - k = bus_message_read_in_addr_auto(m, &error, &family, &a); - if (k < 0 && !sd_bus_error_has_name(&error, SD_BUS_ERROR_INVALID_ARGS)) - return k; + r = bus_message_read_in_addr_auto(m, &error, &family, &a); + if (r < 0) { + if (sd_bus_error_has_name(&error, SD_BUS_ERROR_INVALID_ARGS)) { + /* CurrentDNSServer provides AF_UNSPEC when no current server assigned. */ + log_debug("Invalid DNS server, ignoring: %s", bus_error_message(&error, r)); + + for (;;) { + r = sd_bus_message_skip(m, NULL); + if (r == -ENXIO) /* End of the container */ + break; + if (r < 0) + return r; + } + + r = sd_bus_message_exit_container(m); + if (r < 0) + return r; + + *ret = NULL; + return 1; + } + + return r; + } if (FLAGS_SET(flags, READ_DNS_EXTENDED)) { r = sd_bus_message_read(m, "q", &port); @@ -1404,12 +1425,6 @@ static int read_dns_server_one(sd_bus_message *m, ReadDNSFlag flags, char **ret) if (r < 0) return r; - if (k < 0) { - log_debug("Invalid DNS server, ignoring: %s", bus_error_message(&error, k)); - *ret = NULL; - return 1; - } - if (FLAGS_SET(flags, READ_DNS_ONLY_GLOBAL) && ifindex > 0 && ifindex != LOOPBACK_IFINDEX) { /* This one has an (non-loopback) ifindex set, and we were told to suppress those. Hence do so. */ *ret = NULL; From 176b887da4450b80287d11ea178635a93d15dc94 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Wed, 23 Apr 2025 15:43:43 +0200 Subject: [PATCH 3/4] bus-message-util: use bus_message_read_family() at one more place --- src/shared/bus-message-util.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/shared/bus-message-util.c b/src/shared/bus-message-util.c index 8da112cacc..1b855df293 100644 --- a/src/shared/bus-message-util.c +++ b/src/shared/bus-message-util.c @@ -82,7 +82,7 @@ int bus_message_read_in_addr_auto(sd_bus_message *message, sd_bus_error *error, assert(message); - r = sd_bus_message_read(message, "i", &family); + r = bus_message_read_family(message, error, &family); if (r < 0) return r; @@ -90,9 +90,6 @@ int bus_message_read_in_addr_auto(sd_bus_message *message, sd_bus_error *error, if (r < 0) return r; - if (!IN_SET(family, AF_INET, AF_INET6)) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Unknown address family %i", family); - if (sz != FAMILY_ADDRESS_SIZE(family)) return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid address size"); From 264749453c1c08b81ef71b34a2410d7525f60b90 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 24 Apr 2025 10:00:36 +0900 Subject: [PATCH 4/4] core/dbus-cgroup: use bus_message_read_in_addr_auto() at one more place --- src/core/dbus-cgroup.c | 33 ++++++++------------------------- 1 file changed, 8 insertions(+), 25 deletions(-) diff --git a/src/core/dbus-cgroup.c b/src/core/dbus-cgroup.c index 09692a93ec..6f5d2c0b9e 100644 --- a/src/core/dbus-cgroup.c +++ b/src/core/dbus-cgroup.c @@ -1636,46 +1636,29 @@ int bus_cgroup_set_property( return r; for (;;) { - const void *ap; - int32_t family; - uint32_t prefixlen; - size_t an; - r = sd_bus_message_enter_container(message, 'r', "iayu"); if (r < 0) return r; if (r == 0) break; - r = sd_bus_message_read(message, "i", &family); + struct in_addr_prefix prefix; + r = bus_message_read_in_addr_auto(message, error, &prefix.family, &prefix.address); if (r < 0) return r; - if (!IN_SET(family, AF_INET, AF_INET6)) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "%s= expects IPv4 or IPv6 addresses only.", name); - - r = sd_bus_message_read_array(message, 'y', &ap, &an); - if (r < 0) - return r; - - if (an != FAMILY_ADDRESS_SIZE(family)) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "IP address has wrong size for family (%s, expected %zu, got %zu)", - af_to_name(family), FAMILY_ADDRESS_SIZE(family), an); - + uint32_t prefixlen; r = sd_bus_message_read(message, "u", &prefixlen); if (r < 0) return r; - if (prefixlen > FAMILY_ADDRESS_SIZE(family)*8) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Prefix length %" PRIu32 " too large for address family %s.", prefixlen, af_to_name(family)); + if (prefixlen > FAMILY_ADDRESS_SIZE(prefix.family)*8) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Prefix length %" PRIu32 " too large for address family %s.", + prefixlen, af_to_name(prefix.family)); if (!UNIT_WRITE_FLAGS_NOOP(flags)) { - struct in_addr_prefix prefix = { - .family = family, - .prefixlen = prefixlen, - }; - - memcpy(&prefix.address, ap, an); + prefix.prefixlen = (uint8_t) prefixlen; r = in_addr_prefix_add(&new_prefixes, &prefix); if (r < 0)