From: Edwin Peer <edwin.peer@broadcom•com>
To: netdev@vger•kernel.org
Cc: Edwin Peer <edwin.peer@broadcom•com>,
Jakub Kicinski <kuba@kernel•org>,
Andrew Gospodarek <andrew.gospodarek@broadcom•com>,
Michael Chan <michael.chan@broadcom•com>,
Stephen Hemminger <stephen@networkplumber•org>,
Michal Kubecek <mkubecek@suse•cz>,
David Ahern <dsahern@gmail•com>
Subject: [PATCH net-next 1/4] netlink: truncate overlength attribute list in nla_nest_end()
Date: Fri, 22 Jan 2021 20:53:18 -0800 [thread overview]
Message-ID: <20210123045321.2797360-2-edwin.peer@broadcom.com> (raw)
In-Reply-To: <20210123045321.2797360-1-edwin.peer@broadcom.com>
[-- Attachment #1: Type: text/plain, Size: 4950 bytes --]
If a nested list of attributes is too long, then the length will
exceed the 16-bit nla_len of the parent nlattr. In such cases,
determine how many whole attributes can fit and truncate the
message to this length. This properly maintains the nesting
hierarchy, keeping the entire message valid, while fitting more
subelements inside the nest range than may result if the length
is wrapped modulo 64KB.
Marking truncated attributes, such that user space can determine
the precise attribute truncated, by means of an additional bit in
the nla_type was considered and rejected. The NLA_F_NESTED and
NLA_F_NET_BYTEORDER flags are supposed to be mutually exclusive.
So, in theory, the latter bit could have been redefined for nested
attributes in order to indicate truncation, but user space tools
(most notably iproute2) cannot be relied on to honor NLA_TYPE_MASK,
resulting in alteration of the perceived nla_type and subsequent
catastrophic failure.
Failing the entire message with a hard error must also be rejected,
as this would break existing user space functionality. The trigger
issue is evident for IFLA_VFINFO_LIST and a hard error here would
cause iproute2 to fail to render an entire interface list even if
only a single interface warranted a truncated VF list. Instead, set
NLM_F_NEST_TRUNCATED in the netlink header to inform user space
about the incomplete data. In this particular case, however, user
space can better ascertain which instance is truncated by consulting
the associated IFLA_NUM_VF to determine how many VFs were expected.
Fixes: bfa83a9e03cf ("[NETLINK]: Type-safe netlink messages/attributes interface")
Signed-off-by: Edwin Peer <edwin.peer@broadcom•com>
---
include/net/netlink.h | 11 +++++++++--
include/uapi/linux/netlink.h | 1 +
lib/nlattr.c | 27 +++++++++++++++++++++++++++
3 files changed, 37 insertions(+), 2 deletions(-)
diff --git a/include/net/netlink.h b/include/net/netlink.h
index 1ceec518ab49..fc8c57dafb05 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -1785,19 +1785,26 @@ static inline struct nlattr *nla_nest_start(struct sk_buff *skb, int attrtype)
return nla_nest_start_noflag(skb, attrtype | NLA_F_NESTED);
}
+int __nla_nest_trunc_msg(struct sk_buff *skb, const struct nlattr *start);
+
/**
* nla_nest_end - Finalize nesting of attributes
* @skb: socket buffer the attributes are stored in
* @start: container attribute
*
* Corrects the container attribute header to include the all
- * appeneded attributes.
+ * appeneded attributes. The list of attributes will be truncated
+ * if too long to fit within the parent attribute's maximum reach.
*
* Returns the total data length of the skb.
*/
static inline int nla_nest_end(struct sk_buff *skb, struct nlattr *start)
{
- start->nla_len = skb_tail_pointer(skb) - (unsigned char *)start;
+ int len = skb_tail_pointer(skb) - (unsigned char *)start;
+
+ if (len > 0xffff)
+ len = __nla_nest_trunc_msg(skb, start);
+ start->nla_len = len;
return skb->len;
}
diff --git a/include/uapi/linux/netlink.h b/include/uapi/linux/netlink.h
index 3d94269bbfa8..44a250825c30 100644
--- a/include/uapi/linux/netlink.h
+++ b/include/uapi/linux/netlink.h
@@ -57,6 +57,7 @@ struct nlmsghdr {
#define NLM_F_ECHO 0x08 /* Echo this request */
#define NLM_F_DUMP_INTR 0x10 /* Dump was inconsistent due to sequence change */
#define NLM_F_DUMP_FILTERED 0x20 /* Dump was filtered as requested */
+#define NLM_F_NEST_TRUNCATED 0x40 /* Message contains truncated nested attribute */
/* Modifiers to GET request */
#define NLM_F_ROOT 0x100 /* specify tree root */
diff --git a/lib/nlattr.c b/lib/nlattr.c
index 5b6116e81f9f..2a267c0d3e16 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -1119,4 +1119,31 @@ int nla_append(struct sk_buff *skb, int attrlen, const void *data)
return 0;
}
EXPORT_SYMBOL(nla_append);
+
+/**
+ * __nla_nest_trunc_msg - Truncate list of nested netlink attributes to max len
+ * @skb: socket buffer with tail pointer positioned after end of nested list
+ * @start: container attribute designating the beginning of the list
+ *
+ * Trims the skb to fit only the attributes which are within the range of the
+ * containing nest attribute. This is a helper for nla_nest_end, to prevent
+ * adding unduly to the length of what is an inline function. It is not
+ * intended to be called from anywhere else.
+ *
+ * Returns the truncated length of the enclosing nest attribute in accordance
+ * with the number of whole attributes that can fit.
+ */
+int __nla_nest_trunc_msg(struct sk_buff *skb, const struct nlattr *start)
+{
+ struct nlattr *attr = nla_data(start);
+ int rem = 0xffff - NLA_HDRLEN;
+
+ while (nla_ok(attr, rem))
+ attr = nla_next(attr, &rem);
+ nlmsg_trim(skb, attr);
+ nlmsg_hdr(skb)->nlmsg_flags |= NLM_F_NEST_TRUNCATED;
+ return (unsigned char *)attr - (unsigned char *)start;
+}
+EXPORT_SYMBOL(__nla_nest_trunc_msg);
+
#endif
--
2.30.0
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4160 bytes --]
next prev parent reply other threads:[~2021-01-23 4:54 UTC|newest]
Thread overview: 25+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-23 4:53 [PATCH net-next 0/4] support for 256 VFs in RTM_GETLINK Edwin Peer
2021-01-23 4:53 ` Edwin Peer [this message]
2021-01-23 19:14 ` [PATCH net-next 1/4] netlink: truncate overlength attribute list in nla_nest_end() David Ahern
2021-01-23 20:42 ` Edwin Peer
2021-01-23 21:03 ` Edwin Peer
2021-01-26 4:56 ` David Ahern
2021-01-26 17:51 ` Edwin Peer
2023-06-05 7:28 ` Gal Pressman
2023-06-05 18:58 ` Jakub Kicinski
2023-06-05 19:27 ` Edwin Peer
2023-06-06 8:01 ` Gal Pressman
2023-06-06 16:17 ` Jakub Kicinski
2023-06-07 13:31 ` Gal Pressman
2023-06-07 16:33 ` Jakub Kicinski
2023-06-07 16:52 ` Stephen Hemminger
2023-06-07 17:29 ` Jakub Kicinski
2021-01-26 4:50 ` David Ahern
2021-01-26 1:43 ` Jakub Kicinski
2021-01-23 4:53 ` [PATCH net-next 2/4] rtnetlink: extend RTEXT_FILTER_SKIP_STATS to IFLA_VF_INFO Edwin Peer
2021-01-26 1:55 ` Jakub Kicinski
2021-01-26 22:48 ` Edwin Peer
2021-01-23 4:53 ` [PATCH net-next 3/4] rtnetlink: refactor IFLA_VF_INFO stats into rtnl_fill_vfstats() Edwin Peer
2021-01-23 4:53 ` [PATCH net-next 4/4] rtnetlink: promote IFLA_VF_STATS to same level as IFLA_VF_INFO Edwin Peer
2021-01-26 2:01 ` Jakub Kicinski
2021-01-26 14:50 ` Edwin Peer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210123045321.2797360-2-edwin.peer@broadcom.com \
--to=edwin.peer@broadcom$(echo .)com \
--cc=andrew.gospodarek@broadcom$(echo .)com \
--cc=dsahern@gmail$(echo .)com \
--cc=kuba@kernel$(echo .)org \
--cc=michael.chan@broadcom$(echo .)com \
--cc=mkubecek@suse$(echo .)cz \
--cc=netdev@vger$(echo .)kernel.org \
--cc=stephen@networkplumber$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox