pkarashchenko commented on code in PR #8864:
URL: https://github.com/apache/nuttx/pull/8864#discussion_r1143147300
##########
include/netpacket/if_addr.h:
##########
@@ -98,6 +98,7 @@ enum
IFA_CACHEINFO,
IFA_MULTICAST,
IFA_FLAGS,
+ IFA_RT_PRIORITY, /* u32, priority/metric for prefix route */
Review Comment:
Maybe we should include the description in comment block above instead of
placing it here?
##########
net/netlink/Make.defs:
##########
@@ -26,7 +26,7 @@ SOCK_CSRCS += netlink_sockif.c
NET_CSRCS += netlink_conn.c netlink_notifier.c
ifeq ($(CONFIG_NETLINK_ROUTE),y)
-NET_CSRCS += netlink_route.c
+NET_CSRCS += netlink_route.c nlattr.c
Review Comment:
Should we name file `netlink_attr.c` to be consistent with other files in
the netlink folder?
##########
net/netlink/netlink.h:
##########
@@ -45,10 +45,138 @@
#ifndef CONFIG_NETLINK_ROUTE
#define netlink_device_notify(dev)
+ #define netlink_device_notify_ipaddr(dev, type, domain)
#endif
#ifdef CONFIG_NET_NETLINK
+/**
+ * nla_for_each_attr - iterate over a stream of attributes
+ * @pos: loop counter, set to current attribute
+ * @head: head of attribute stream
+ * @len: length of attribute stream
+ * @rem: initialized to len, holds bytes currently remaining in stream
+ */
+
+#define nla_for_each_attr(pos, head, len, rem) \
+ for (pos = head, rem = len; nla_ok(pos, rem); \
+ pos = nla_next(pos, &(rem)))
+
+/* Always use this macro, this allows later putting the
+ * message into a separate section or such for things
+ * like translation or listing all possible messages.
+ * Currently string formatting is not supported (due
+ * to the lack of an output buffer.)
+ */
+
+#define nl_set_err_msg_attr(extack, attr, msg) do { \
+ static const char __msg[] = msg; \
+ FAR struct netlink_ext_ack *__extack = (extack); \
+ if (__extack) \
+ { \
+ __extack->_msg = __msg; \
+ __extack->bad_attr = (attr); \
+ } \
+} while (0)
+
+/**
+ * nla_data - head of payload
+ * @nla: netlink attribute
+ */
+
+#define nla_data(nla) ((FAR void *)((FAR char *)(nla) + NLA_HDRLEN))
+
+/**
+ * nla_len - length of payload
+ * @nla: netlink attribute
+ */
+
+#define nla_len(nla) ((nla)->nla_len - NLA_HDRLEN)
+
+/**
+ * nla_type - attribute type
+ * @nla: netlink attribute
+ */
+
+#define nla_type(nla) ((nla)->nla_type & NLA_TYPE_MASK)
+
+/**
+ * nla_ok - check if the netlink attribute fits into the remaining bytes
+ * @nla: netlink attribute
+ * @remaining: number of bytes remaining in attribute stream
+ */
+
+#define nla_ok(nla, remaining) \
+ ((remaining) >= sizeof(*(nla)) && \
+ (nla)->nla_len >= sizeof(*(nla)) && \
+ (nla)->nla_len <= (remaining))
Review Comment:
```suggestion
#define nla_ok(nla, remaining) \
((remaining) >= sizeof(*(nla)) && \
(nla)->nla_len >= sizeof(*(nla)) && \
(nla)->nla_len <= (remaining))
```
##########
net/netlink/netlink.h:
##########
@@ -45,10 +45,138 @@
#ifndef CONFIG_NETLINK_ROUTE
#define netlink_device_notify(dev)
+ #define netlink_device_notify_ipaddr(dev, type, domain)
#endif
#ifdef CONFIG_NET_NETLINK
+/**
+ * nla_for_each_attr - iterate over a stream of attributes
+ * @pos: loop counter, set to current attribute
+ * @head: head of attribute stream
+ * @len: length of attribute stream
+ * @rem: initialized to len, holds bytes currently remaining in stream
+ */
+
+#define nla_for_each_attr(pos, head, len, rem) \
+ for (pos = head, rem = len; nla_ok(pos, rem); \
+ pos = nla_next(pos, &(rem)))
+
+/* Always use this macro, this allows later putting the
+ * message into a separate section or such for things
+ * like translation or listing all possible messages.
+ * Currently string formatting is not supported (due
+ * to the lack of an output buffer.)
+ */
+
+#define nl_set_err_msg_attr(extack, attr, msg) do { \
+ static const char __msg[] = msg; \
+ FAR struct netlink_ext_ack *__extack = (extack); \
+ if (__extack) \
+ { \
+ __extack->_msg = __msg; \
+ __extack->bad_attr = (attr); \
+ } \
+} while (0)
Review Comment:
```suggestion
#define nl_set_err_msg_attr(extack, attr, msg) \
do \
{ \
static const char __msg[] = msg; \
FAR struct netlink_ext_ack *__extack = (extack); \
if (__extack) \
{ \
__extack->_msg = __msg; \
__extack->bad_attr = (attr); \
} \
} \
while (0)
```
##########
net/netlink/netlink.h:
##########
@@ -45,10 +45,138 @@
#ifndef CONFIG_NETLINK_ROUTE
#define netlink_device_notify(dev)
+ #define netlink_device_notify_ipaddr(dev, type, domain)
Review Comment:
```suggestion
# define netlink_device_notify(dev)
# define netlink_device_notify_ipaddr(dev, type, domain)
```
##########
net/netlink/nlattr.c:
##########
@@ -0,0 +1,324 @@
+/****************************************************************************
+ * net/netlink/nlattr.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/types.h>
+#include <string.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+#include <unistd.h>
+
+#include <nuttx/net/netlink.h>
+
+#include "netlink.h"
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+#ifdef CONFIG_NETLINK_VALIDATE_POLICY
+static const uint8_t g_nla_attr_len[NLA_TYPE_MAX + 1] =
+{
+ 0,
+ sizeof(uint8_t),
+ sizeof(uint16_t),
+ sizeof(uint32_t),
+ sizeof(uint64_t),
+ 0, 0, 0, 0, 0, 0, 0,
+ sizeof(int8_t),
+ sizeof(int16_t),
+ sizeof(int32_t),
+ sizeof(int64_t),
+};
+
+static const uint8_t g_nla_attr_minlen[NLA_TYPE_MAX + 1] =
+{
+ 0,
+ sizeof(uint8_t),
+ sizeof(uint16_t),
+ sizeof(uint32_t),
+ sizeof(uint64_t),
+ 0, 0,
+ sizeof(uint64_t),
+ NLA_HDRLEN,
+ 0, 0, 0,
+ sizeof(int8_t),
+ sizeof(int16_t),
+ sizeof(int32_t),
+ sizeof(int64_t),
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static int validate_nla_bitfield32(FAR const struct nlattr *nla,
+ FAR uint32_t *valid_flags_mask)
+{
+ FAR const struct nla_bitfield32 *bf = nla_data(nla);
+
+ if (!valid_flags_mask)
+ {
+ return -EINVAL;
+ }
+
+ /* disallow invalid bit selector */
+
+ if (bf->selector & ~*valid_flags_mask)
+ {
+ return -EINVAL;
+ }
+
+ /* disallow invalid bit values */
+
+ if (bf->value & ~*valid_flags_mask)
+ {
+ return -EINVAL;
+ }
+
+ /* disallow valid bit values that are not selected */
+
+ if (bf->value & ~bf->selector)
+ {
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int validate_nla(FAR const struct nlattr *nla, int maxtype,
+ FAR const struct nla_policy *policy)
+{
+ FAR const struct nla_policy *pt;
+ int minlen = 0;
+ int attrlen = nla_len(nla);
+ int type = nla_type(nla);
+
+ if (type <= 0 || type > maxtype)
+ {
+ return -EINVAL;
+ }
+
+ pt = &policy[type];
+
+ DEBUGASSERT(pt->type <= NLA_TYPE_MAX);
+
+ if (g_nla_attr_len[pt->type] && attrlen != g_nla_attr_len[pt->type])
+ {
+ nwarn("netlink: '%d': attribute type %d has an invalid length.\n",
+ getpid(), type);
+ return -EINVAL;
+ }
+
+ switch (pt->type)
+ {
+ case NLA_FLAG:
+ if (attrlen > 0)
+ {
+ return -ERANGE;
+ }
+ break;
+
+ case NLA_BITFIELD32:
+ if (attrlen != sizeof(struct nla_bitfield32))
+ {
+ return -ERANGE;
+ }
+
+ return validate_nla_bitfield32(nla, pt->validation_data);
+
+ case NLA_NUL_STRING:
+ if (pt->len)
+ {
+ minlen = MIN(attrlen, pt->len + 1);
+ }
+ else
+ {
+ minlen = attrlen;
+ }
+
+ if (!minlen || memchr(nla_data(nla), '\0', minlen) == NULL)
+ {
+ return -EINVAL;
+ }
+
+ /* fall through */
+
+ case NLA_STRING:
+ if (attrlen < 1)
+ {
+ return -ERANGE;
+ }
+
+ if (pt->len)
+ {
+ FAR char *buf = nla_data(nla);
+
+ if (buf[attrlen - 1] == '\0')
+ attrlen--;
+
+ if (attrlen > pt->len)
+ return -ERANGE;
Review Comment:
```suggestion
if (buf[attrlen - 1] == '\0')
{
attrlen--;
}
if (attrlen > pt->len)
{
return -ERANGE;
}
```
##########
net/netlink/nlattr.c:
##########
@@ -0,0 +1,324 @@
+/****************************************************************************
+ * net/netlink/nlattr.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership. The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <sys/types.h>
+#include <string.h>
+#include <assert.h>
+#include <errno.h>
+#include <debug.h>
+#include <unistd.h>
+
+#include <nuttx/net/netlink.h>
+
+#include "netlink.h"
+
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+#ifdef CONFIG_NETLINK_VALIDATE_POLICY
+static const uint8_t g_nla_attr_len[NLA_TYPE_MAX + 1] =
+{
+ 0,
+ sizeof(uint8_t),
+ sizeof(uint16_t),
+ sizeof(uint32_t),
+ sizeof(uint64_t),
+ 0, 0, 0, 0, 0, 0, 0,
+ sizeof(int8_t),
+ sizeof(int16_t),
+ sizeof(int32_t),
+ sizeof(int64_t),
+};
+
+static const uint8_t g_nla_attr_minlen[NLA_TYPE_MAX + 1] =
+{
+ 0,
+ sizeof(uint8_t),
+ sizeof(uint16_t),
+ sizeof(uint32_t),
+ sizeof(uint64_t),
+ 0, 0,
+ sizeof(uint64_t),
+ NLA_HDRLEN,
+ 0, 0, 0,
+ sizeof(int8_t),
+ sizeof(int16_t),
+ sizeof(int32_t),
+ sizeof(int64_t),
+};
+
+/****************************************************************************
+ * Private Functions
+ ****************************************************************************/
+
+static int validate_nla_bitfield32(FAR const struct nlattr *nla,
+ FAR uint32_t *valid_flags_mask)
+{
+ FAR const struct nla_bitfield32 *bf = nla_data(nla);
+
+ if (!valid_flags_mask)
+ {
+ return -EINVAL;
+ }
+
+ /* disallow invalid bit selector */
+
+ if (bf->selector & ~*valid_flags_mask)
+ {
+ return -EINVAL;
+ }
+
+ /* disallow invalid bit values */
+
+ if (bf->value & ~*valid_flags_mask)
+ {
+ return -EINVAL;
+ }
+
+ /* disallow valid bit values that are not selected */
+
+ if (bf->value & ~bf->selector)
+ {
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int validate_nla(FAR const struct nlattr *nla, int maxtype,
+ FAR const struct nla_policy *policy)
+{
+ FAR const struct nla_policy *pt;
+ int minlen = 0;
+ int attrlen = nla_len(nla);
+ int type = nla_type(nla);
+
+ if (type <= 0 || type > maxtype)
+ {
+ return -EINVAL;
+ }
+
+ pt = &policy[type];
+
+ DEBUGASSERT(pt->type <= NLA_TYPE_MAX);
+
+ if (g_nla_attr_len[pt->type] && attrlen != g_nla_attr_len[pt->type])
+ {
+ nwarn("netlink: '%d': attribute type %d has an invalid length.\n",
+ getpid(), type);
+ return -EINVAL;
+ }
+
+ switch (pt->type)
+ {
+ case NLA_FLAG:
+ if (attrlen > 0)
+ {
+ return -ERANGE;
+ }
+ break;
+
+ case NLA_BITFIELD32:
+ if (attrlen != sizeof(struct nla_bitfield32))
+ {
+ return -ERANGE;
+ }
+
+ return validate_nla_bitfield32(nla, pt->validation_data);
+
+ case NLA_NUL_STRING:
+ if (pt->len)
+ {
+ minlen = MIN(attrlen, pt->len + 1);
+ }
+ else
+ {
+ minlen = attrlen;
+ }
+
+ if (!minlen || memchr(nla_data(nla), '\0', minlen) == NULL)
+ {
+ return -EINVAL;
+ }
+
+ /* fall through */
+
+ case NLA_STRING:
+ if (attrlen < 1)
+ {
+ return -ERANGE;
+ }
+
+ if (pt->len)
+ {
+ FAR char *buf = nla_data(nla);
+
+ if (buf[attrlen - 1] == '\0')
+ attrlen--;
+
+ if (attrlen > pt->len)
+ return -ERANGE;
+ }
+ break;
+
+ case NLA_BINARY:
+ if (pt->len && attrlen > pt->len)
+ {
+ return -ERANGE;
+ }
+ break;
+
+ case NLA_NESTED_COMPAT:
+ if (attrlen < pt->len)
+ {
+ return -ERANGE;
+ }
+
+ if (attrlen < NLA_ALIGN(pt->len))
+ {
+ break;
+ }
+
+ if (attrlen < NLA_ALIGN(pt->len) + NLA_HDRLEN)
+ {
+ return -ERANGE;
+ }
+
+ nla = nla_data(nla) + NLA_ALIGN(pt->len);
+ if (attrlen < NLA_ALIGN(pt->len) + NLA_HDRLEN + nla_len(nla))
+ {
+ return -ERANGE;
+ }
+ break;
+
+ case NLA_NESTED:
+ /* a nested attributes is allowed to be empty; if its not,
+ * it must have a size of at least NLA_HDRLEN.
+ */
+
+ if (attrlen == 0)
+ {
+ break;
+ }
+
+ default:
+ if (pt->len)
+ {
+ minlen = pt->len;
+ }
+ else if (pt->type != NLA_UNSPEC)
+ {
+ minlen = g_nla_attr_minlen[pt->type];
+ }
+
+ if (attrlen < minlen)
+ {
+ return -ERANGE;
+ }
+ }
+
+ return 0;
+}
+#else
+#define validate_nla(nla, maxtype, policy) 0
Review Comment:
```suggestion
# define validate_nla(nla, maxtype, policy) 0
```
##########
net/netlink/netlink_route.c:
##########
@@ -164,6 +185,54 @@ struct nlroute_info_s
FAR const struct nlroute_sendto_request_s *req;
};
+/****************************************************************************
+ * Private Data
+ ****************************************************************************/
+
+#ifdef CONFIG_NETLINK_VALIDATE_POLICY
+# ifdef CONFIG_NET_IPv4
+static const struct nla_policy g_ifa_ipv4_policy[] =
+{
+ {0}, /* IFA_UNSPEC */
+ {NLA_U32, 0, NULL}, /* IFA_ADDRESS */
+ {NLA_U32, 0, NULL}, /* IFA_LOCAL */
+ {NLA_STRING, IFNAMSIZ - 1, NULL}, /* IFA_LABEL */
+ {NLA_U32, 0, NULL}, /* IFA_BROADCAST */
+ {0}, /* IFA_ANYCAST */
+ {NLA_UNSPEC, sizeof(struct ifa_cacheinfo), NULL}, /* IFA_CACHEINFO */
+ {0}, /* IFA_MULTICAST */
+ {NLA_U32, 0, NULL}, /* IFA_FLAGS */
+ {NLA_U32, 0, NULL}, /* IFA_RT_PRIORITY */
+};
+
+static_assert(sizeof(g_ifa_ipv4_policy) / sizeof(g_ifa_ipv4_policy[0]) ==
+ IFA_MAX + 1, "The policy definition has changed,"
+ " please check it");
+# endif
+# ifdef CONFIG_NET_IPv6
+static const struct nla_policy g_ifa_ipv6_policy[] =
+{
+ {0}, /* IFA_UNSPEC */
+ {0, sizeof(struct in6_addr), NULL}, /* IFA_ADDRESS */
+ {0, sizeof(struct in6_addr), NULL}, /* IFA_LOCAL */
+ {0}, /* IFA_LABEL */
+ {0}, /* IFA_BROADCAST */
+ {0}, /* IFA_ANYCAST */
+ {NLA_UNSPEC, sizeof(struct ifa_cacheinfo), NULL}, /* IFA_CACHEINFO */
+ {0}, /* IFA_MULTICAST */
+ {0, sizeof(uint32_t), NULL}, /* IFA_FLAGS */
+ {0, sizeof(uint32_t), NULL}, /* IFA_RT_PRIORITY */
+};
+
+static_assert(sizeof(g_ifa_ipv6_policy) / sizeof(g_ifa_ipv6_policy[0]) ==
+ IFA_MAX + 1, "The policy definition has changed,"
+ " please check it");
+# endif
+#else
+#define g_ifa_ipv4_policy NULL
+#define g_ifa_ipv6_policy NULL
Review Comment:
```suggestion
# define g_ifa_ipv4_policy NULL
# define g_ifa_ipv6_policy NULL
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
