From 179b4db439ce4c41d44a6b5de22244c9ab54b252 Mon Sep 17 00:00:00 2001 From: Benjamin Robin Date: Sat, 9 May 2020 18:12:21 +0200 Subject: [PATCH 1/5] netlink: Fix assert condition on n_containers --- src/libsystemd/sd-netlink/netlink-message.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/libsystemd/sd-netlink/netlink-message.c b/src/libsystemd/sd-netlink/netlink-message.c index a5871c7319..d6bf31efc7 100644 --- a/src/libsystemd/sd-netlink/netlink-message.c +++ b/src/libsystemd/sd-netlink/netlink-message.c @@ -15,8 +15,7 @@ #include "socket-util.h" #include "strv.h" -#define GET_CONTAINER(m, i) ((i) < (m)->n_containers ? (struct rtattr*)((uint8_t*)(m)->hdr + (m)->containers[i].offset) : NULL) -#define PUSH_CONTAINER(m, new) (m)->container_offsets[(m)->n_containers++] = (uint8_t*)(new) - (uint8_t*)(m)->hdr; +#define GET_CONTAINER(m, i) ((struct rtattr*)((uint8_t*)(m)->hdr + (m)->containers[i].offset)) #define RTA_TYPE(rta) ((rta)->rta_type & NLA_TYPE_MASK) #define RTA_FLAGS(rta) ((rta)->rta_type & ~NLA_TYPE_MASK) @@ -520,7 +519,8 @@ int sd_netlink_message_open_container(sd_netlink_message *m, unsigned short type assert_return(m, -EINVAL); assert_return(!m->sealed, -EPERM); - assert_return(m->n_containers < RTNL_CONTAINER_DEPTH, -ERANGE); + /* m->containers[m->n_containers + 1] is accessed both in read and write. Prevent access out of bound */ + assert_return(m->n_containers < (RTNL_CONTAINER_DEPTH - 1), -ERANGE); r = message_attribute_has_type(m, &size, type, NETLINK_TYPE_NESTED); if (r < 0) { @@ -567,6 +567,7 @@ int sd_netlink_message_open_container_union(sd_netlink_message *m, unsigned shor assert_return(m, -EINVAL); assert_return(!m->sealed, -EPERM); + assert_return(m->n_containers < (RTNL_CONTAINER_DEPTH - 1), -ERANGE); r = type_system_get_type_system_union(m->containers[m->n_containers].type_system, &type_system_union, type); if (r < 0) @@ -609,6 +610,7 @@ int sd_netlink_message_open_array(sd_netlink_message *m, uint16_t type) { assert_return(m, -EINVAL); assert_return(!m->sealed, -EPERM); + assert_return(m->n_containers < (RTNL_CONTAINER_DEPTH - 1), -ERANGE); r = add_rtattr(m, type | NLA_F_NESTED, NULL, 0); if (r < 0) @@ -1007,7 +1009,7 @@ int sd_netlink_message_enter_container(sd_netlink_message *m, unsigned short typ int r; assert_return(m, -EINVAL); - assert_return(m->n_containers < RTNL_CONTAINER_DEPTH, -EINVAL); + assert_return(m->n_containers < (RTNL_CONTAINER_DEPTH - 1), -EINVAL); r = type_system_get_type(m->containers[m->n_containers].type_system, &nl_type, @@ -1098,7 +1100,7 @@ int sd_netlink_message_enter_array(sd_netlink_message *m, unsigned short type_id int r; assert_return(m, -EINVAL); - assert_return(m->n_containers < RTNL_CONTAINER_DEPTH, -EINVAL); + assert_return(m->n_containers < (RTNL_CONTAINER_DEPTH - 1), -EINVAL); r = netlink_message_read_internal(m, type_id, &container, NULL); if (r < 0) From b9c54c46652d74c4bd3a55a87abcb82d317cbb16 Mon Sep 17 00:00:00 2001 From: Benjamin Robin Date: Sat, 9 May 2020 19:51:23 +0200 Subject: [PATCH 2/5] tree-wide: Initialize _cleanup_ variables if needed --- src/libsystemd/sd-bus/test-bus-chat.c | 4 ++-- src/libsystemd/sd-netlink/test-netlink.c | 2 +- src/network/netdev/macsec.c | 2 +- src/resolve/resolved-dnstls-gnutls.c | 2 +- src/shared/seccomp-util.c | 2 +- src/test/test-fstab-util.c | 2 +- src/udev/net/link-config.c | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/libsystemd/sd-bus/test-bus-chat.c b/src/libsystemd/sd-bus/test-bus-chat.c index 05127f0e0c..2c1fbbefc3 100644 --- a/src/libsystemd/sd-bus/test-bus-chat.c +++ b/src/libsystemd/sd-bus/test-bus-chat.c @@ -317,7 +317,7 @@ static void* client1(void *p) { finish: if (bus) { - _cleanup_(sd_bus_message_unrefp) sd_bus_message *q; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *q = NULL; r = sd_bus_message_new_method_call( bus, @@ -485,7 +485,7 @@ static void* client2(void *p) { finish: if (bus) { - _cleanup_(sd_bus_message_unrefp) sd_bus_message *q; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *q = NULL; r = sd_bus_message_new_method_call( bus, diff --git a/src/libsystemd/sd-netlink/test-netlink.c b/src/libsystemd/sd-netlink/test-netlink.c index e03302bbec..9598ad8e02 100644 --- a/src/libsystemd/sd-netlink/test-netlink.c +++ b/src/libsystemd/sd-netlink/test-netlink.c @@ -128,7 +128,7 @@ static void test_address_get(sd_netlink *rtnl, int ifindex) { } static void test_route(sd_netlink *rtnl) { - _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *req; + _cleanup_(sd_netlink_message_unrefp) sd_netlink_message *req = NULL; struct in_addr addr, addr_data; uint32_t index = 2, u32_data; int r; diff --git a/src/network/netdev/macsec.c b/src/network/netdev/macsec.c index 7d1fec3afe..3542f9652a 100644 --- a/src/network/netdev/macsec.c +++ b/src/network/netdev/macsec.c @@ -828,7 +828,7 @@ int config_parse_macsec_key_id( _cleanup_(macsec_transmit_association_free_or_set_invalidp) TransmitAssociation *a = NULL; _cleanup_(macsec_receive_association_free_or_set_invalidp) ReceiveAssociation *b = NULL; - _cleanup_free_ void *p; + _cleanup_free_ void *p = NULL; MACsec *s = userdata; uint8_t *dest; size_t l; diff --git a/src/resolve/resolved-dnstls-gnutls.c b/src/resolve/resolved-dnstls-gnutls.c index a5d9776427..6b7db7ef8c 100644 --- a/src/resolve/resolved-dnstls-gnutls.c +++ b/src/resolve/resolved-dnstls-gnutls.c @@ -28,7 +28,7 @@ static ssize_t dnstls_stream_writev(gnutls_transport_ptr_t p, const giovec_t *io } int dnstls_stream_connect_tls(DnsStream *stream, DnsServer *server) { - _cleanup_(gnutls_deinitp) gnutls_session_t gs; + _cleanup_(gnutls_deinitp) gnutls_session_t gs = NULL; int r; assert(stream); diff --git a/src/shared/seccomp-util.c b/src/shared/seccomp-util.c index 47a321ccd6..320b1767c5 100644 --- a/src/shared/seccomp-util.c +++ b/src/shared/seccomp-util.c @@ -1743,7 +1743,7 @@ int seccomp_restrict_archs(Set *archs) { } int parse_syscall_archs(char **l, Set **archs) { - _cleanup_set_free_ Set *_archs; + _cleanup_set_free_ Set *_archs = NULL; char **s; int r; diff --git a/src/test/test-fstab-util.c b/src/test/test-fstab-util.c index c1c7ec9114..187be69d15 100644 --- a/src/test/test-fstab-util.c +++ b/src/test/test-fstab-util.c @@ -21,7 +21,7 @@ static void do_fstab_filter_options(const char *opts, int r; const char *name; - _cleanup_free_ char *value, *filtered; + _cleanup_free_ char *value = NULL, *filtered = NULL; r = fstab_filter_options(opts, remove, &name, &value, &filtered); log_info("\"%s\" → %d, \"%s\", \"%s\", \"%s\", expected %d, \"%s\", \"%s\", \"%s\"", diff --git a/src/udev/net/link-config.c b/src/udev/net/link-config.c index 098aa59913..e4ace761b7 100644 --- a/src/udev/net/link-config.c +++ b/src/udev/net/link-config.c @@ -207,7 +207,7 @@ static int link_unsigned_attribute(sd_device *device, const char *attr, unsigned } int link_config_load(link_config_ctx *ctx) { - _cleanup_strv_free_ char **files; + _cleanup_strv_free_ char **files = NULL; char **f; int r; From 81c5a5961fe655b0a735775e74be701f1a473ceb Mon Sep 17 00:00:00 2001 From: Benjamin Robin Date: Sat, 9 May 2020 21:19:12 +0200 Subject: [PATCH 3/5] network: 'cur' variable cannot be null, so simplify code --- src/libsystemd-network/sd-radv.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/libsystemd-network/sd-radv.c b/src/libsystemd-network/sd-radv.c index e7ffe879a8..7ea6f0d561 100644 --- a/src/libsystemd-network/sd-radv.c +++ b/src/libsystemd-network/sd-radv.c @@ -590,8 +590,7 @@ _public_ int sd_radv_add_prefix(sd_radv *ra, sd_radv_prefix *p, int dynamic) { cur->valid_until = valid_until; cur->preferred_until = preferred_until; - log_radv("%s prefix %s/%u preferred %s valid %s", - cur? "Updated": "Added", + log_radv("Updated prefix %s/%u preferred %s valid %s", addr_p, p->opt.prefixlen, format_timespan(time_string_preferred, FORMAT_TIMESPAN_MAX, preferred, USEC_PER_SEC), @@ -691,8 +690,7 @@ _public_ int sd_radv_add_route_prefix(sd_radv *ra, sd_radv_route_prefix *p, int if (valid_until == USEC_INFINITY) return -EOVERFLOW; - log_radv("%s route prefix %s/%u valid %s", - cur? "Updated": "Added", + log_radv("Updated route prefix %s/%u valid %s", strempty(pretty), p->opt.prefixlen, format_timespan(time_string_valid, FORMAT_TIMESPAN_MAX, valid, USEC_PER_SEC)); From fbb0b665773690ab753d1428fd6b8a4621e58496 Mon Sep 17 00:00:00 2001 From: Benjamin Robin Date: Sat, 9 May 2020 21:19:17 +0200 Subject: [PATCH 4/5] systemctl: Check clean_or_freeze_unit(): Add assert if unknown method --- src/systemctl/systemctl.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/systemctl/systemctl.c b/src/systemctl/systemctl.c index 51cd0cf9d3..83d0e39738 100644 --- a/src/systemctl/systemctl.c +++ b/src/systemctl/systemctl.c @@ -3775,6 +3775,8 @@ static int clean_or_freeze_unit(int argc, char *argv[], void *userdata) { method = "FreezeUnit"; else if (streq(argv[0], "thaw")) method = "ThawUnit"; + else + assert_not_reached("Unhandled method"); STRV_FOREACH(name, names) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; From 243945e95e8cfdef85b44e98a127496fcec8675d Mon Sep 17 00:00:00 2001 From: Benjamin Robin Date: Sun, 10 May 2020 16:49:30 +0200 Subject: [PATCH 5/5] test: Add return 0 to main() function (even it is not strictly necessary) --- src/journal/test-audit-type.c | 1 + src/libsystemd-network/test-sd-dhcp-lease.c | 1 + src/network/test-network.c | 1 + 3 files changed, 3 insertions(+) diff --git a/src/journal/test-audit-type.c b/src/journal/test-audit-type.c index 20e654b599..a8315d0633 100644 --- a/src/journal/test-audit-type.c +++ b/src/journal/test-audit-type.c @@ -22,4 +22,5 @@ static void test_audit_type(void) { int main(int argc, char **argv) { test_audit_type(); + return 0; } diff --git a/src/libsystemd-network/test-sd-dhcp-lease.c b/src/libsystemd-network/test-sd-dhcp-lease.c index 0f881809ab..534cc78564 100644 --- a/src/libsystemd-network/test-sd-dhcp-lease.c +++ b/src/libsystemd-network/test-sd-dhcp-lease.c @@ -87,4 +87,5 @@ int main(int argc, char *argv[]) { test_dhcp_lease_parse_search_domains_no_data(); test_dhcp_lease_parse_search_domains_loops(); test_dhcp_lease_parse_search_domains_wrong_len(); + return 0; } diff --git a/src/network/test-network.c b/src/network/test-network.c index e23bec7680..1600966283 100644 --- a/src/network/test-network.c +++ b/src/network/test-network.c @@ -253,4 +253,5 @@ int main(void) { test_network_get(manager, loopback); assert_se(manager_rtnl_enumerate_links(manager) >= 0); + return 0; }