netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] netlink: add policy attribute range validation
@ 2018-09-26 20:06 Johannes Berg
  2018-09-26 20:07 ` [RFC] nl80211: use policy range validation where applicable Johannes Berg
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Johannes Berg @ 2018-09-26 20:06 UTC (permalink / raw)
  To: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA
  Cc: Johannes Berg

From: Johannes Berg <johannes.berg-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Without further bloating the policy structs, we can overload
the `validation_data' pointer with a struct of s16 min, max
and use those to validate ranges in NLA_{U,S}{8,16,32,64}
attributes.

It may sound strange to validate NLA_U32 with a s16 max, but
in many cases NLA_U32 is used for enums etc. since there's no
size benefit in using a smaller attribute width anyway, due
to netlink attribute alignment; in cases like that it's still
useful, particularly when the attribute really transports an
enum value.

Doing so lets us remove quite a bit of validation code, if we
can be sure that these attributes aren't used by userspace in
places where they're ignored today.

Signed-off-by: Johannes Berg <johannes.berg-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 include/net/netlink.h | 25 +++++++++++++++++++++++--
 lib/nlattr.c          | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 2 deletions(-)

diff --git a/include/net/netlink.h b/include/net/netlink.h
index 3698ca8ff92c..ddabc832febc 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -238,7 +238,23 @@ enum {
  *                         nested attributes directly inside, while an array has
  *                         the nested attributes at another level down and the
  *                         attributes directly in the nesting don't matter.
- *    All other            Unused
+ *    All other            Unused - but note that it's a union
+ *
+ * Meaning of `min' and `max' fields:
+ *    NLA_U8,
+ *    NLA_U16,
+ *    NLA_U32,
+ *    NLA_U64,
+ *    NLA_S8,
+ *    NLA_S16,
+ *    NLA_S32,
+ *    NLA_S64              If at least one of them is non-zero, they are the
+ *                         minimum and maximum value accepted for the attribute;
+ *                         note that in the interest of code simplicity and
+ *                         struct size both limits are s16, so you cannot
+ *                         enforce a range that doesn't fall within the range
+ *                         of s16 - do that as usual in the code instead.
+ *    All other            Unused - but note that it's a union
  *
  * Example:
  * static const struct nla_policy my_policy[ATTR_MAX+1] = {
@@ -251,7 +267,12 @@ enum {
 struct nla_policy {
 	u16		type;
 	u16		len;
-	const void     *validation_data;
+	union {
+		const void *validation_data;
+		struct {
+			s16 min, max;
+		};
+	};
 };
 
 #define NLA_POLICY_EXACT_LEN(_len)	{ .type = NLA_EXACT_LEN, .len = _len }
diff --git a/lib/nlattr.c b/lib/nlattr.c
index 2f8feff669a7..dd8d34c1ae19 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -230,6 +230,55 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
 			goto out_err;
 	}
 
+	/* validate range */
+	if (pt->min || pt->max) {
+		s64 value;
+		bool validate = true;
+
+		switch (pt->type) {
+		case NLA_U8:
+			value = nla_get_u8(nla);
+			break;
+		case NLA_U16:
+			value = nla_get_u16(nla);
+			break;
+		case NLA_U32:
+			value = nla_get_u32(nla);
+			break;
+		case NLA_S8:
+			value = nla_get_s8(nla);
+			break;
+		case NLA_S16:
+			value = nla_get_s16(nla);
+			break;
+		case NLA_S32:
+			value = nla_get_s32(nla);
+			break;
+		case NLA_S64:
+			value = nla_get_s64(nla);
+			break;
+		case NLA_U64:
+			/* treat this one specially, since it may not fit into s64 */
+			if (nla_get_u64(nla) < pt->min ||
+			    nla_get_u64(nla) > pt->max) {
+				NL_SET_ERR_MSG_ATTR(extack, nla,
+						    "integer out of range");
+				return -ERANGE;
+			}
+			/* fall through */
+		default:
+			/* no further validation */
+			validate = false;
+			break;
+		}
+
+		if (validate && (value < pt->min || value > pt->max)) {
+			NL_SET_ERR_MSG_ATTR(extack, nla,
+					    "integer out of range");
+			return -ERANGE;
+		}
+	}
+
 	return 0;
 out_err:
 	NL_SET_ERR_MSG_ATTR(extack, nla, "Attribute failed policy validation");
-- 
2.14.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-09-27 15:09 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-26 20:06 [PATCH] netlink: add policy attribute range validation Johannes Berg
2018-09-26 20:07 ` [RFC] nl80211: use policy range validation where applicable Johannes Berg
2018-09-26 20:09   ` Johannes Berg
2018-09-26 20:17 ` [PATCH] netlink: add policy attribute range validation Johannes Berg
     [not found]   ` <1537993066.28767.29.camel-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
2018-09-26 20:35     ` Johannes Berg
2018-09-27  7:16       ` Michal Kubecek
     [not found]         ` <20180927071621.GF30601-OEaqT8BN2ewCVLCxKZUutA@public.gmane.org>
2018-09-27  8:12           ` Johannes Berg
     [not found]             ` <1538035929.14416.21.camel-cdvu00un1VgdHxzADdlk8Q@public.gmane.org>
2018-09-27  8:48               ` Michal Kubecek
2018-09-27  8:51                 ` Johannes Berg
2018-09-26 20:24 ` Johannes Berg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).