From 0adc4fc4f6b8fc5a445060be0e6a32038c7642cf Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 10 Aug 2021 16:36:13 +0200 Subject: [PATCH 1/8] platform/netlink: drop unused NLA type enums --- src/libnm-platform/nm-netlink.h | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/libnm-platform/nm-netlink.h b/src/libnm-platform/nm-netlink.h index bcc321194d..28c2541319 100644 --- a/src/libnm-platform/nm-netlink.h +++ b/src/libnm-platform/nm-netlink.h @@ -35,15 +35,8 @@ enum { NLA_U64, /* 64 bit integer */ NLA_STRING, /* NUL terminated character string */ NLA_FLAG, /* Flag */ - NLA_MSECS, /* Micro seconds (64bit) */ NLA_NESTED, /* Nested attributes */ - NLA_NESTED_COMPAT, NLA_NUL_STRING, - NLA_BINARY, - NLA_S8, - NLA_S16, - NLA_S32, - NLA_S64, __NLA_TYPE_MAX, }; From 386b367bfa41cb160c23c9fa37ee7729eda73b1e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 10 Aug 2021 16:37:28 +0200 Subject: [PATCH 2/8] platform/netlink: cleanup handling of nla_attr_minlen - make nla_attr_minlen[] and array of uint8_t. That is large enough for all values we have. - don't handle NLA_UNSPEC specially. nla_attr_minlen[NLA_UNSPEC] returns zero just fine. --- src/libnm-platform/nm-netlink.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/libnm-platform/nm-netlink.c b/src/libnm-platform/nm-netlink.c index c45d73891d..efe41bca07 100644 --- a/src/libnm-platform/nm-netlink.c +++ b/src/libnm-platform/nm-netlink.c @@ -585,21 +585,20 @@ nla_nest_end(struct nl_msg *msg, struct nlattr *start) return _nest_end(msg, start, 0); } -static const uint16_t nla_attr_minlen[NLA_TYPE_MAX + 1] = { +static const uint8_t nla_attr_minlen[NLA_TYPE_MAX + 1] = { [NLA_U8] = sizeof(uint8_t), [NLA_U16] = sizeof(uint16_t), [NLA_U32] = sizeof(uint32_t), [NLA_U64] = sizeof(uint64_t), [NLA_STRING] = 1, - [NLA_FLAG] = 0, }; static int validate_nla(const struct nlattr *nla, int maxtype, const struct nla_policy *policy) { const struct nla_policy *pt; - unsigned int minlen = 0; - int type = nla_type(nla); + uint16_t minlen; + int type = nla_type(nla); if (type < 0 || type > maxtype) return 0; @@ -611,7 +610,7 @@ validate_nla(const struct nlattr *nla, int maxtype, const struct nla_policy *pol if (pt->minlen) minlen = pt->minlen; - else if (pt->type != NLA_UNSPEC) + else minlen = nla_attr_minlen[pt->type]; if (nla_len(nla) < minlen) From 6f1274caeab6a0113cad60d2ab9b681ffae43e8b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 10 Aug 2021 17:06:51 +0200 Subject: [PATCH 3/8] platform/netlink: return uint16_t type from nla_len() nla_len() cannot return anything larger or smaller than range uint16_t. Change the return type of nla_len(). --- src/libnm-platform/nm-netlink.h | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/libnm-platform/nm-netlink.h b/src/libnm-platform/nm-netlink.h index 28c2541319..a3644d49bd 100644 --- a/src/libnm-platform/nm-netlink.h +++ b/src/libnm-platform/nm-netlink.h @@ -105,28 +105,24 @@ nla_padlen(int payload) struct nlattr *nla_reserve(struct nl_msg *msg, int attrtype, int attrlen); -static inline int +static inline uint16_t nla_len(const struct nlattr *nla) { nm_assert(nla); nm_assert(nla->nla_len >= NLA_HDRLEN); - return ((int) nla->nla_len) - NLA_HDRLEN; + return nla->nla_len - ((uint16_t) NLA_HDRLEN); } static inline int nla_type(const struct nlattr *nla) { - nm_assert(nla_len(nla) >= 0); - return nla->nla_type & NLA_TYPE_MASK; } static inline void * nla_data(const struct nlattr *nla) { - nm_assert(nla_len(nla) >= 0); - return &(((char *) nla)[NLA_HDRLEN]); } @@ -212,9 +208,7 @@ nla_get_be64(const struct nlattr *nla) static inline char * nla_get_string(const struct nlattr *nla) { - nm_assert(nla_len(nla) >= 0); - - return (char *) nla_data(nla); + return nla_data(nla); } size_t nla_strlcpy(char *dst, const struct nlattr *nla, size_t dstsize); From 68a5d1cfe5126241892fc26736daad2c5b5f553d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 10 Aug 2021 17:14:04 +0200 Subject: [PATCH 4/8] platform/netlink: refactor handling length in validate_nla() --- src/libnm-platform/nm-netlink.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libnm-platform/nm-netlink.c b/src/libnm-platform/nm-netlink.c index efe41bca07..d836e4c9ff 100644 --- a/src/libnm-platform/nm-netlink.c +++ b/src/libnm-platform/nm-netlink.c @@ -598,6 +598,7 @@ validate_nla(const struct nlattr *nla, int maxtype, const struct nla_policy *pol { const struct nla_policy *pt; uint16_t minlen; + uint16_t len; int type = nla_type(nla); if (type < 0 || type > maxtype) @@ -613,10 +614,12 @@ validate_nla(const struct nlattr *nla, int maxtype, const struct nla_policy *pol else minlen = nla_attr_minlen[pt->type]; - if (nla_len(nla) < minlen) + len = nla_len(nla); + + if (len < minlen) return -NME_UNSPEC; - if (pt->maxlen && nla_len(nla) > pt->maxlen) + if (pt->maxlen && len > pt->maxlen) return -NME_UNSPEC; if (pt->type == NLA_STRING) { @@ -625,7 +628,7 @@ validate_nla(const struct nlattr *nla, int maxtype, const struct nla_policy *pol nm_assert(minlen > 0); data = nla_data(nla); - if (data[nla_len(nla) - 1] != '\0') + if (data[len - 1u] != '\0') return -NME_UNSPEC; } From f7635c9ffe738f1b9376d04d8c4acec2fc3caf8b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 10 Aug 2021 17:26:07 +0200 Subject: [PATCH 5/8] platform/netlink: use switch for type check in validate_nla() --- src/libnm-platform/nm-netlink.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/libnm-platform/nm-netlink.c b/src/libnm-platform/nm-netlink.c index d836e4c9ff..c7dfb3664a 100644 --- a/src/libnm-platform/nm-netlink.c +++ b/src/libnm-platform/nm-netlink.c @@ -622,14 +622,17 @@ validate_nla(const struct nlattr *nla, int maxtype, const struct nla_policy *pol if (pt->maxlen && len > pt->maxlen) return -NME_UNSPEC; - if (pt->type == NLA_STRING) { - const char *data; + switch (pt->type) { + case NLA_STRING: + { + const char *data = nla_data(nla); nm_assert(minlen > 0); - data = nla_data(nla); if (data[len - 1u] != '\0') return -NME_UNSPEC; + break; + } } return 0; From fa745181dcc595660cc9c4eac17df2fb5be1259c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 10 Aug 2021 17:27:59 +0200 Subject: [PATCH 6/8] platform/netlink: drop unused NLA_NUL_STRING type Kernel implemente NLA_NUL_STRING type, but we don't implement exactly the same type checks. Drop NLA_NUL_STRING and use a plain NLA_STRING instead. --- src/libnm-platform/nm-linux-platform.c | 2 +- src/libnm-platform/nm-netlink.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libnm-platform/nm-linux-platform.c b/src/libnm-platform/nm-linux-platform.c index d27e457814..3d57b7ff3f 100644 --- a/src/libnm-platform/nm-linux-platform.c +++ b/src/libnm-platform/nm-linux-platform.c @@ -2305,7 +2305,7 @@ _wireguard_get_device_cb(struct nl_msg *msg, void *arg) { static const struct nla_policy policy[] = { [WGDEVICE_A_IFINDEX] = {.type = NLA_U32}, - [WGDEVICE_A_IFNAME] = {.type = NLA_NUL_STRING, .maxlen = IFNAMSIZ}, + [WGDEVICE_A_IFNAME] = {.type = NLA_STRING, .maxlen = IFNAMSIZ}, [WGDEVICE_A_PRIVATE_KEY] = {}, [WGDEVICE_A_PUBLIC_KEY] = {}, [WGDEVICE_A_FLAGS] = {.type = NLA_U32}, diff --git a/src/libnm-platform/nm-netlink.h b/src/libnm-platform/nm-netlink.h index a3644d49bd..3946286716 100644 --- a/src/libnm-platform/nm-netlink.h +++ b/src/libnm-platform/nm-netlink.h @@ -36,7 +36,6 @@ enum { NLA_STRING, /* NUL terminated character string */ NLA_FLAG, /* Flag */ NLA_NESTED, /* Nested attributes */ - NLA_NUL_STRING, __NLA_TYPE_MAX, }; From 57d626c18209f72da54df67133199d33081cec14 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 11 Aug 2021 10:32:13 +0200 Subject: [PATCH 7/8] platform/netlink: rework static check in _nl_static_assert_tb() to use _Generic() Depending on sizeof(policy) to be sizeof(NULL) is not a good check whether the macro argument may be NULL. That is, because the size of the policy array might accidentally be the same as the size of a pointer. Use _Generic() instead. --- src/libnm-platform/nm-netlink.h | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/libnm-platform/nm-netlink.h b/src/libnm-platform/nm-netlink.h index 3946286716..b9f1f6f4f3 100644 --- a/src/libnm-platform/nm-netlink.h +++ b/src/libnm-platform/nm-netlink.h @@ -67,18 +67,24 @@ struct nla_policy { /*****************************************************************************/ /* static asserts that @tb and @policy are suitable arguments to nla_parse(). */ -#define _nl_static_assert_tb(tb, policy) \ - G_STMT_START \ - { \ - G_STATIC_ASSERT_EXPR(G_N_ELEMENTS(tb) > 0); \ - \ +#if _NM_CC_SUPPORT_GENERIC +#define _nl_static_assert_tb(tb, policy) \ + G_STMT_START \ + { \ + G_STATIC_ASSERT_EXPR(G_N_ELEMENTS(tb) > 0); \ + \ /* We allow @policy to be either a C array or NULL. The sizeof() - * must either match the expected array size or the sizeof(NULL), - * but not both. */ \ - G_STATIC_ASSERT_EXPR((sizeof(policy) == G_N_ELEMENTS(tb) * sizeof(struct nla_policy)) \ - ^ (sizeof(policy) == sizeof(NULL))); \ - } \ + * must either match the expected array size or we check that + * "policy" has typeof(NULL). This isn't a perfect compile time check, + * but good enough. */ \ + G_STATIC_ASSERT_EXPR( \ + _Generic((policy), typeof(NULL) : 1, default \ + : (sizeof(policy) == G_N_ELEMENTS(tb) * sizeof(struct nla_policy)))); \ + } \ G_STMT_END +#else +#define _nl_static_assert_tb(tb, policy) G_STATIC_ASSERT_EXPR(G_N_ELEMENTS(tb) > 0) +#endif /*****************************************************************************/ From 76bcd7871062ce66909cf1ff8aba68d222d54ab9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 10 Aug 2021 17:32:47 +0200 Subject: [PATCH 8/8] platform/netlink: use appropriate integer types in nla_policy --- src/libnm-platform/nm-netlink.c | 6 +++--- src/libnm-platform/nm-netlink.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libnm-platform/nm-netlink.c b/src/libnm-platform/nm-netlink.c index c7dfb3664a..56cde50ca7 100644 --- a/src/libnm-platform/nm-netlink.c +++ b/src/libnm-platform/nm-netlink.c @@ -597,7 +597,7 @@ static int validate_nla(const struct nlattr *nla, int maxtype, const struct nla_policy *policy) { const struct nla_policy *pt; - uint16_t minlen; + uint8_t minlen; uint16_t len; int type = nla_type(nla); @@ -609,7 +609,7 @@ validate_nla(const struct nlattr *nla, int maxtype, const struct nla_policy *pol if (pt->type > NLA_TYPE_MAX) g_return_val_if_reached(-NME_BUG); - if (pt->minlen) + if (pt->minlen > 0) minlen = pt->minlen; else minlen = nla_attr_minlen[pt->type]; @@ -619,7 +619,7 @@ validate_nla(const struct nlattr *nla, int maxtype, const struct nla_policy *pol if (len < minlen) return -NME_UNSPEC; - if (pt->maxlen && len > pt->maxlen) + if (pt->maxlen > 0 && len > pt->maxlen) return -NME_UNSPEC; switch (pt->type) { diff --git a/src/libnm-platform/nm-netlink.h b/src/libnm-platform/nm-netlink.h index b9f1f6f4f3..00d817cdbb 100644 --- a/src/libnm-platform/nm-netlink.h +++ b/src/libnm-platform/nm-netlink.h @@ -55,10 +55,10 @@ const char *nl_nlmsghdr_to_str(const struct nlmsghdr *hdr, char *buf, gsize len) struct nla_policy { /* Type of attribute or NLA_UNSPEC */ - uint16_t type; + uint8_t type; /* Minimal length of payload required */ - uint16_t minlen; + uint8_t minlen; /* Maximal length of payload allowed */ uint16_t maxlen;