netlink: Relax attr validation for fixed length types

PATCH In review David Ahern 2 participants 2 messages Dec 7, 2017

Subsystems (3)

Networking Network drivers Library code

lore

Share & tools

lore

David Ahern

author

2017-12-07 04:09 UTC #0 lore raw

PATCH v2 1/1 netlink: Relax attr validation for fixed length types

Commit 28033ae4e0f5 ("net: netlink: Update attr validation to require exact length for some types") requires attributes using types NLA_U* and NLA_S* to have an exact length. This change is exposing bugs in various userspace commands that are sending attributes with an invalid length (e.g., attribute has type NLA_U8 and userspace sends NLA_U32). While the commands are clearly broken and need to be fixed, users are arguing that the sudden change in enforcement is breaking older commands on newer kernels for use cases that otherwise "worked".

Relax the validation to print a warning mesage similar to what is done for messages containing extra bytes after parsing.

Fixes 28033ae4e0f5 ("net: netlink: Update attr validation to require exact length for some types")

Signed-off-by David Ahern <[email protected]>

Reviewed-by Johannes Berg <[email protected]>

---
v2
- updated the comment before nla_attr_len and removed the outdated comment before use of nla_attr_len

 lib/nlattr.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

lib/nlattr.c +16 -6

index 8bf78b4b78f0..dfa55c873c13 100644
@@ -15,7 +15,11 @@
 #include <linux/types.h>
 #include <net/netlink.h>
 
-/* for these data types attribute length must be exactly given size */
+/* For these data types, attribute length should be exactly the given
+ * size. However, to maintain compatibility with broken commands, if the
+ * attribute length does not match the expected size a warning is emitted
+ * to the user that the command is sending invalid data and needs to be fixed.
+ */
 static const u8 nla_attr_len[NLA_TYPE_MAX+1] = {
 	[NLA_U8]	= sizeof(u8),
 	[NLA_U16]	= sizeof(u16),

@@ -28,8 +32,16 @@ static const u8 nla_attr_len[NLA_TYPE_MAX+1] = {
 };
 
 static const u8 nla_attr_minlen[NLA_TYPE_MAX+1] = {
+	[NLA_U8]	= sizeof(u8),
+	[NLA_U16]	= sizeof(u16),
+	[NLA_U32]	= sizeof(u32),
+	[NLA_U64]	= sizeof(u64),
 	[NLA_MSECS]	= sizeof(u64),
 	[NLA_NESTED]	= NLA_HDRLEN,
+	[NLA_S8]	= sizeof(s8),
+	[NLA_S16]	= sizeof(s16),
+	[NLA_S32]	= sizeof(s32),
+	[NLA_S64]	= sizeof(s64),
 };
 
 static int validate_nla_bitfield32(const struct nlattr *nla,

@@ -69,11 +81,9 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
 
 	BUG_ON(pt->type > NLA_TYPE_MAX);
 
-	/* for data types NLA_U* and NLA_S* require exact length */
-	if (nla_attr_len[pt->type]) {
-		if (attrlen != nla_attr_len[pt->type])
-			return -ERANGE;
-		return 0;
+	if (nla_attr_len[pt->type] && attrlen != nla_attr_len[pt->type]) {
+		pr_warn_ratelimited("netlink: '%s': attribute type %d has an invalid length.\n",
+				    current->comm, type);
 	}
 
 	switch (pt->type) {

Signature · 1 line

2.11.0

David Miller

maintainer

2017-12-07 19:01 UTC #1 lore raw

From: David Ahern <[email protected]>
Date: Wed, 6 Dec 2017 20:09:12 -0800

Quoted 12 lines

Commit 28033ae4e0f5 ("net: netlink: Update attr validation to require exact length for some types") requires attributes using types NLA_U* and NLA_S* to have an exact length. This change is exposing bugs in various userspace commands that are sending attributes with an invalid length (e.g., attribute has type NLA_U8 and userspace sends NLA_U32). While the commands are clearly broken and need to be fixed, users are arguing that the sudden change in enforcement is breaking older commands on newer kernels for use cases that otherwise "worked".
Relax the validation to print a warning mesage similar to what is done for messages containing extra bytes after parsing.

Quoted 7 lines

Fixes 28033ae4e0f5 ("net: netlink: Update attr validation to require exact length for some types")
Signed-off-by David Ahern <[email protected]>
Reviewed-by Johannes Berg <[email protected]>
---
v2
- updated the comment before nla_attr_len and removed the outdated comment before use of nla_attr_len

Applied and queued up for -stable, thanks David.