From b89a3758e92894162e3c2dcb594a55acff3274d5 Mon Sep 17 00:00:00 2001 From: Yu Watanabe Date: Thu, 23 Sep 2021 14:58:28 +0900 Subject: [PATCH] sd-dhcp6-client: modernize dhcp6_option_parse() - merge dhcp6_option_parse() with option_parse_hdr(). - do not assign/update any values on error. - use assert() instead of assert_return(), as the assertions cannot be triggered by a library user. --- src/libsystemd-network/dhcp6-internal.h | 10 +++- src/libsystemd-network/dhcp6-option.c | 62 ++++++++++------------ src/libsystemd-network/sd-dhcp6-lease.c | 37 ++++++------- src/libsystemd-network/test-dhcp6-client.c | 50 ++++++++--------- 4 files changed, 77 insertions(+), 82 deletions(-) diff --git a/src/libsystemd-network/dhcp6-internal.h b/src/libsystemd-network/dhcp6-internal.h index 29c5c22ca0..8bcc826f42 100644 --- a/src/libsystemd-network/dhcp6-internal.h +++ b/src/libsystemd-network/dhcp6-internal.h @@ -101,8 +101,14 @@ int dhcp6_option_append_fqdn(uint8_t **buf, size_t *buflen, const char *fqdn); int dhcp6_option_append_user_class(uint8_t **buf, size_t *buflen, char * const *user_class); int dhcp6_option_append_vendor_class(uint8_t **buf, size_t *buflen, char * const *user_class); int dhcp6_option_append_vendor_option(uint8_t **buf, size_t *buflen, OrderedHashmap *vendor_options); -int dhcp6_option_parse(uint8_t **buf, size_t *buflen, uint16_t *optcode, - size_t *optlen, uint8_t **optvalue); + +int dhcp6_option_parse( + const uint8_t *buf, + size_t buflen, + size_t *offset, + uint16_t *ret_option_code, + size_t *ret_option_data_len, + const uint8_t **ret_option_data); int dhcp6_option_parse_status(DHCP6Option *option, size_t len); int dhcp6_option_parse_ia(sd_dhcp6_client *client, DHCP6Option *iaoption, be32_t iaid, DHCP6IA *ia, uint16_t *ret_status_code); int dhcp6_option_parse_ip6addrs(const uint8_t *optval, uint16_t optlen, diff --git a/src/libsystemd-network/dhcp6-option.c b/src/libsystemd-network/dhcp6-option.c index 0709cfd4fd..781d391c0c 100644 --- a/src/libsystemd-network/dhcp6-option.c +++ b/src/libsystemd-network/dhcp6-option.c @@ -370,47 +370,39 @@ int dhcp6_option_append_vendor_class(uint8_t **buf, size_t *buflen, char * const return dhcp6_option_append(buf, buflen, SD_DHCP6_OPTION_VENDOR_CLASS, total, p); } -static int option_parse_hdr(uint8_t **buf, size_t *buflen, uint16_t *optcode, size_t *optlen) { - DHCP6Option *option = (DHCP6Option*) *buf; - uint16_t len; +int dhcp6_option_parse( + const uint8_t *buf, + size_t buflen, + size_t *offset, + uint16_t *ret_option_code, + size_t *ret_option_data_len, + const uint8_t **ret_option_data) { - assert_return(buf, -EINVAL); - assert_return(optcode, -EINVAL); - assert_return(optlen, -EINVAL); + const DHCP6Option *option; + size_t len; - if (*buflen < offsetof(DHCP6Option, data)) - return -ENOMSG; + assert(buf); + assert(offset); + assert(ret_option_code); + assert(ret_option_data_len); + assert(ret_option_data); + if (buflen < offsetof(DHCP6Option, data)) + return -EBADMSG; + + if (*offset >= buflen - offsetof(DHCP6Option, data)) + return -EBADMSG; + + option = (const DHCP6Option*) (buf + *offset); len = be16toh(option->len); - if (len > *buflen) - return -ENOMSG; + if (len > buflen - offsetof(DHCP6Option, data) - *offset) + return -EBADMSG; - *optcode = be16toh(option->code); - *optlen = len; - - *buf += 4; - *buflen -= 4; - - return 0; -} - -int dhcp6_option_parse(uint8_t **buf, size_t *buflen, uint16_t *optcode, - size_t *optlen, uint8_t **optvalue) { - int r; - - assert_return(buf && buflen && optcode && optlen && optvalue, -EINVAL); - - r = option_parse_hdr(buf, buflen, optcode, optlen); - if (r < 0) - return r; - - if (*optlen > *buflen) - return -ENOBUFS; - - *optvalue = *buf; - *buflen -= *optlen; - *buf += *optlen; + *offset += offsetof(DHCP6Option, data) + len; + *ret_option_code = be16toh(option->code); + *ret_option_data_len = len; + *ret_option_data = option->data; return 0; } diff --git a/src/libsystemd-network/sd-dhcp6-lease.c b/src/libsystemd-network/sd-dhcp6-lease.c index 9082185bca..6375a22537 100644 --- a/src/libsystemd-network/sd-dhcp6-lease.c +++ b/src/libsystemd-network/sd-dhcp6-lease.c @@ -259,9 +259,6 @@ int sd_dhcp6_lease_get_domains(sd_dhcp6_lease *lease, char ***domains) { int dhcp6_lease_set_ntp(sd_dhcp6_lease *lease, uint8_t *optval, size_t optlen) { int r; - uint16_t subopt; - size_t sublen; - uint8_t *subval; assert_return(lease, -EINVAL); assert_return(optval, -EINVAL); @@ -269,10 +266,14 @@ int dhcp6_lease_set_ntp(sd_dhcp6_lease *lease, uint8_t *optval, size_t optlen) { lease->ntp = mfree(lease->ntp); lease->ntp_count = 0; - while ((r = dhcp6_option_parse(&optval, &optlen, &subopt, &sublen, - &subval)) >= 0) { - int s; - char **servers; + for (size_t offset = 0; offset < optlen;) { + const uint8_t *subval; + size_t sublen; + uint16_t subopt; + + r = dhcp6_option_parse(optval, optlen, &offset, &subopt, &sublen, &subval); + if (r < 0) + return r; switch(subopt) { case DHCP6_NTP_SUBOPTION_SRV_ADDR: @@ -280,19 +281,18 @@ int dhcp6_lease_set_ntp(sd_dhcp6_lease *lease, uint8_t *optval, size_t optlen) { if (sublen != 16) return 0; - s = dhcp6_option_parse_ip6addrs(subval, sublen, - &lease->ntp, - lease->ntp_count); - if (s < 0) - return s; + r = dhcp6_option_parse_ip6addrs(subval, sublen, &lease->ntp, lease->ntp_count); + if (r < 0) + return r; - lease->ntp_count = s; + lease->ntp_count = r; break; - case DHCP6_NTP_SUBOPTION_SRV_FQDN: - r = dhcp6_option_parse_domainname_list(subval, sublen, - &servers); + case DHCP6_NTP_SUBOPTION_SRV_FQDN: { + char **servers; + + r = dhcp6_option_parse_domainname_list(subval, sublen, &servers); if (r < 0) return 0; @@ -300,12 +300,9 @@ int dhcp6_lease_set_ntp(sd_dhcp6_lease *lease, uint8_t *optval, size_t optlen) { lease->ntp_fqdn_count = r; break; - } + }} } - if (r != -ENOMSG) - return r; - return 0; } diff --git a/src/libsystemd-network/test-dhcp6-client.c b/src/libsystemd-network/test-dhcp6-client.c index 22ab25b2b0..f057276476 100644 --- a/src/libsystemd-network/test-dhcp6-client.c +++ b/src/libsystemd-network/test-dhcp6-client.c @@ -170,47 +170,47 @@ static int test_option(sd_event *e) { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 'B', 'A', 'R', }; + size_t offset, pos, optlen, outlen = sizeof(result); + const uint8_t *optval; uint16_t optcode; - size_t optlen; - uint8_t *optval, *buf, *out; - size_t zero = 0, pos = 3; - size_t buflen = sizeof(packet), outlen = sizeof(result); + uint8_t *out; log_debug("/* %s */", __func__); - assert_se(buflen == outlen); + assert_se(sizeof(packet) == sizeof(result)); - assert_se(dhcp6_option_parse(&buf, &zero, &optcode, &optlen, - &optval) == -ENOMSG); + offset = 0; + assert_se(dhcp6_option_parse(packet, 0, &offset, &optcode, &optlen, &optval) == -EBADMSG); - buflen -= 3; - buf = &packet[3]; + offset = 3; + assert_se(dhcp6_option_parse(packet, 0, &offset, &optcode, &optlen, &optval) == -EBADMSG); + + offset = 3; + assert_se(dhcp6_option_parse(packet, sizeof(packet), &offset, &optcode, &optlen, &optval) >= 0); + + assert_se(optcode == SD_DHCP6_OPTION_ORO); + assert_se(optlen == 7); + assert_se(optval == packet + 7); + + pos = 3; outlen -= 3; out = &result[3]; - assert_se(dhcp6_option_parse(&buf, &buflen, &optcode, &optlen, - &optval) >= 0); - pos += 4 + optlen; - assert_se(buf == &packet[pos]); - assert_se(optcode == SD_DHCP6_OPTION_ORO); - assert_se(optlen == 7); - assert_se(buflen + pos == sizeof(packet)); + assert_se(dhcp6_option_append(&out, &outlen, optcode, optlen, optval) >= 0); - assert_se(dhcp6_option_append(&out, &outlen, optcode, optlen, - optval) >= 0); + pos += 4 + optlen; assert_se(out == &result[pos]); assert_se(*out == 0x00); - assert_se(dhcp6_option_parse(&buf, &buflen, &optcode, &optlen, - &optval) >= 0); - pos += 4 + optlen; - assert_se(buf == &packet[pos]); + assert_se(dhcp6_option_parse(packet, sizeof(packet), &offset, &optcode, &optlen, &optval) >= 0); + assert_se(optcode == SD_DHCP6_OPTION_VENDOR_CLASS); assert_se(optlen == 9); - assert_se(buflen + pos == sizeof(packet)); + assert_se(optval == packet + 18); - assert_se(dhcp6_option_append(&out, &outlen, optcode, optlen, - optval) >= 0); + assert_se(dhcp6_option_append(&out, &outlen, optcode, optlen, optval) >= 0); + + pos += 4 + optlen; assert_se(out == &result[pos]); assert_se(*out == 'B');