From: Yu Watanabe Date: Wed, 14 Oct 2020 03:47:58 +0000 (+0900) Subject: sd-dhcp-client: make sd_dhcp_client_set_request_option() not return -EEXIST X-Git-Tag: v247-rc1~83^2~1 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=4081756a63eb1cc37117f8be6c0b0c4b62a5640e;p=thirdparty%2Fsystemd.git sd-dhcp-client: make sd_dhcp_client_set_request_option() not return -EEXIST Fixes #16964. --- diff --git a/src/basic/macro.h b/src/basic/macro.h index d3a53489013..e0fe990beff 100644 --- a/src/basic/macro.h +++ b/src/basic/macro.h @@ -444,6 +444,9 @@ static inline int __coverity_check_and_return__(int condition) { #define PTR_TO_ULONG(p) ((unsigned long) ((uintptr_t) (p))) #define ULONG_TO_PTR(u) ((void *) ((uintptr_t) (u))) +#define PTR_TO_UINT8(p) ((uint8_t) ((uintptr_t) (p))) +#define UINT8_TO_PTR(u) ((void *) ((uintptr_t) (u))) + #define PTR_TO_INT32(p) ((int32_t) ((intptr_t) (p))) #define INT32_TO_PTR(u) ((void *) ((intptr_t) (u))) #define PTR_TO_UINT32(p) ((uint32_t) ((uintptr_t) (p))) diff --git a/src/libsystemd-network/sd-dhcp-client.c b/src/libsystemd-network/sd-dhcp-client.c index 1b622ce5a8f..6eb3c1b7a0c 100644 --- a/src/libsystemd-network/sd-dhcp-client.c +++ b/src/libsystemd-network/sd-dhcp-client.c @@ -25,6 +25,8 @@ #include "io-util.h" #include "memory-util.h" #include "random-util.h" +#include "set.h" +#include "sort-util.h" #include "string-util.h" #include "strv.h" #include "utf8.h" @@ -75,9 +77,7 @@ struct sd_dhcp_client { union sockaddr_union link; sd_event_source *receive_message; bool request_broadcast; - uint8_t *req_opts; - size_t req_opts_allocated; - size_t req_opts_size; + Set *req_opts; bool anonymize; be32_t last_addr; uint8_t mac_addr[MAX_MAC_ADDR_LEN]; @@ -230,8 +230,6 @@ int sd_dhcp_client_set_request_broadcast(sd_dhcp_client *client, int broadcast) } int sd_dhcp_client_set_request_option(sd_dhcp_client *client, uint8_t option) { - size_t i; - assert_return(client, -EINVAL); assert_return(IN_SET(client->state, DHCP_STATE_INIT, DHCP_STATE_STOPPED), -EBUSY); @@ -248,17 +246,7 @@ int sd_dhcp_client_set_request_option(sd_dhcp_client *client, uint8_t option) { break; } - for (i = 0; i < client->req_opts_size; i++) - if (client->req_opts[i] == option) - return -EEXIST; - - if (!GREEDY_REALLOC(client->req_opts, client->req_opts_allocated, - client->req_opts_size + 1)) - return -ENOMEM; - - client->req_opts[client->req_opts_size++] = option; - - return 0; + return set_ensure_put(&client->req_opts, NULL, UINT8_TO_PTR(option)); } int sd_dhcp_client_set_request_address( @@ -725,6 +713,10 @@ static void client_stop(sd_dhcp_client *client, int error) { client_initialize(client); } +static int cmp_uint8(const uint8_t *a, const uint8_t *b) { + return CMP(*a, *b); +} + static int client_message_init( sd_dhcp_client *client, DHCPPacket **ret, @@ -835,10 +827,26 @@ static int client_message_init( MAY contain the Parameter Request List option. */ /* NOTE: in case that there would be an option to do not send * any PRL at all, the size should be checked before sending */ - if (client->req_opts_size > 0 && type != DHCP_RELEASE) { + if (!set_isempty(client->req_opts) && type != DHCP_RELEASE) { + _cleanup_free_ uint8_t *opts = NULL; + size_t n_opts, i = 0; + void *val; + + n_opts = set_size(client->req_opts); + opts = new(uint8_t, n_opts); + if (!opts) + return -ENOMEM; + + SET_FOREACH(val, client->req_opts) + opts[i++] = PTR_TO_UINT8(val); + assert(i == n_opts); + + /* For anonymizing the request, let's sort the options. */ + typesafe_qsort(opts, n_opts, cmp_uint8); + r = dhcp_option_append(&packet->dhcp, optlen, &optoffset, 0, SD_DHCP_OPTION_PARAMETER_REQUEST_LIST, - client->req_opts_size, client->req_opts); + n_opts, opts); if (r < 0) return r; } @@ -2187,7 +2195,7 @@ static sd_dhcp_client *dhcp_client_free(sd_dhcp_client *client) { sd_dhcp_lease_unref(client->lease); - free(client->req_opts); + set_free(client->req_opts); free(client->hostname); free(client->vendor_class_identifier); free(client->mudurl); @@ -2200,6 +2208,10 @@ static sd_dhcp_client *dhcp_client_free(sd_dhcp_client *client) { DEFINE_TRIVIAL_REF_UNREF_FUNC(sd_dhcp_client, sd_dhcp_client, dhcp_client_free); int sd_dhcp_client_new(sd_dhcp_client **ret, int anonymize) { + const uint8_t *opts; + size_t n_opts; + int r; + assert_return(ret, -EINVAL); _cleanup_(sd_dhcp_client_unrefp) sd_dhcp_client *client = new(sd_dhcp_client, 1); @@ -2219,14 +2231,18 @@ int sd_dhcp_client_new(sd_dhcp_client **ret, int anonymize) { }; /* NOTE: this could be moved to a function. */ if (anonymize) { - client->req_opts_size = ELEMENTSOF(default_req_opts_anonymize); - client->req_opts = memdup(default_req_opts_anonymize, client->req_opts_size); + n_opts = ELEMENTSOF(default_req_opts_anonymize); + opts = default_req_opts_anonymize; } else { - client->req_opts_size = ELEMENTSOF(default_req_opts); - client->req_opts = memdup(default_req_opts, client->req_opts_size); + n_opts = ELEMENTSOF(default_req_opts); + opts = default_req_opts; + } + + for (size_t i = 0; i < n_opts; i++) { + r = sd_dhcp_client_set_request_option(client, opts[i]); + if (r < 0) + return r; } - if (!client->req_opts) - return -ENOMEM; *ret = TAKE_PTR(client); diff --git a/src/libsystemd-network/test-dhcp-client.c b/src/libsystemd-network/test-dhcp-client.c index 4a0b71be1cf..4d748ea66d0 100644 --- a/src/libsystemd-network/test-dhcp-client.c +++ b/src/libsystemd-network/test-dhcp-client.c @@ -71,40 +71,29 @@ static void test_request_basic(sd_event *e) { assert_se(sd_dhcp_client_set_hostname(client, "~host") == -EINVAL); assert_se(sd_dhcp_client_set_hostname(client, "~host.domain") == -EINVAL); - assert_se(sd_dhcp_client_set_request_option(client, - SD_DHCP_OPTION_SUBNET_MASK) == -EEXIST); - assert_se(sd_dhcp_client_set_request_option(client, - SD_DHCP_OPTION_ROUTER) == -EEXIST); + assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_SUBNET_MASK) == 0); + assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_ROUTER) == 0); /* This PRL option is not set when using Anonymize, but in this test * Anonymize settings are not being used. */ - assert_se(sd_dhcp_client_set_request_option(client, - SD_DHCP_OPTION_HOST_NAME) == -EEXIST); - assert_se(sd_dhcp_client_set_request_option(client, - SD_DHCP_OPTION_DOMAIN_NAME) == -EEXIST); - assert_se(sd_dhcp_client_set_request_option(client, - SD_DHCP_OPTION_DOMAIN_NAME_SERVER) == -EEXIST); - - assert_se(sd_dhcp_client_set_request_option(client, - SD_DHCP_OPTION_PAD) == -EINVAL); - assert_se(sd_dhcp_client_set_request_option(client, - SD_DHCP_OPTION_END) == -EINVAL); - assert_se(sd_dhcp_client_set_request_option(client, - SD_DHCP_OPTION_MESSAGE_TYPE) == -EINVAL); - assert_se(sd_dhcp_client_set_request_option(client, - SD_DHCP_OPTION_OVERLOAD) == -EINVAL); - assert_se(sd_dhcp_client_set_request_option(client, - SD_DHCP_OPTION_PARAMETER_REQUEST_LIST) - == -EINVAL); + assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_HOST_NAME) == 0); + assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_DOMAIN_NAME) == 0); + assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_DOMAIN_NAME_SERVER) == 0); + + assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_PAD) == -EINVAL); + assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_END) == -EINVAL); + assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_MESSAGE_TYPE) == -EINVAL); + assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_OVERLOAD) == -EINVAL); + assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_PARAMETER_REQUEST_LIST) == -EINVAL); /* RFC7844: option 33 (SD_DHCP_OPTION_STATIC_ROUTE) is set in the * default PRL when using Anonymize, so it is changed to other option * that is not set by default, to check that it was set successfully. * Options not set by default (using or not anonymize) are option 17 * (SD_DHCP_OPTION_ROOT_PATH) and 42 (SD_DHCP_OPTION_NTP_SERVER) */ + assert_se(sd_dhcp_client_set_request_option(client, 17) == 1); + assert_se(sd_dhcp_client_set_request_option(client, 17) == 0); + assert_se(sd_dhcp_client_set_request_option(client, 42) == 1); assert_se(sd_dhcp_client_set_request_option(client, 17) == 0); - assert_se(sd_dhcp_client_set_request_option(client, 17) == -EEXIST); - assert_se(sd_dhcp_client_set_request_option(client, 42) == 0); - assert_se(sd_dhcp_client_set_request_option(client, 17) == -EEXIST); sd_dhcp_client_unref(client); } @@ -126,19 +115,15 @@ static void test_request_anonymize(sd_event *e) { r = sd_dhcp_client_attach_event(client, e, 0); assert_se(r >= 0); - assert_se(sd_dhcp_client_set_request_option(client, - SD_DHCP_OPTION_NETBIOS_NAMESERVER) == -EEXIST); + assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_NETBIOS_NAMESERVER) == 0); /* This PRL option is not set when using Anonymize */ - assert_se(sd_dhcp_client_set_request_option(client, - SD_DHCP_OPTION_HOST_NAME) == 0); - assert_se(sd_dhcp_client_set_request_option(client, - SD_DHCP_OPTION_PARAMETER_REQUEST_LIST) - == -EINVAL); + assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_HOST_NAME) == 1); + assert_se(sd_dhcp_client_set_request_option(client, SD_DHCP_OPTION_PARAMETER_REQUEST_LIST) == -EINVAL); /* RFC7844: option 101 (SD_DHCP_OPTION_NEW_TZDB_TIMEZONE) is not set in the * default PRL when using Anonymize, */ + assert_se(sd_dhcp_client_set_request_option(client, 101) == 1); assert_se(sd_dhcp_client_set_request_option(client, 101) == 0); - assert_se(sd_dhcp_client_set_request_option(client, 101) == -EEXIST); sd_dhcp_client_unref(client); }