Commit 396fc59e authored by David S. Miller's avatar David S. Miller
Browse files

Merge branch 'netlink-allow-NLA_BINARY-length-range-validation' 



Johannes Berg says:

====================
netlink: allow NLA_BINARY length range validation

In quite a few places (perhaps particularly in wireless) we need to
validation an NLA_BINARY attribute with both a minimum and a maximum
length. Currently, we can do either of the two, but not both, given
that we have NLA_MIN_LEN (minimum length) and NLA_BINARY (maximum).

Extend the range mechanisms that we use for integer validation to
apply to NLA_BINARY as well.

After converting everything to use NLA_POLICY_MIN_LEN() we can thus
get rid of the NLA_MIN_LEN type since that's now a special case of
NLA_BINARY with a minimum length validation. Similarly, NLA_EXACT_LEN
can be specified using NLA_POLICY_EXACT_LEN() and also maps to the
new NLA_BINARY validation (min == max == desired length).

Finally, NLA_POLICY_EXACT_LEN_WARN() also gets to be a somewhat
special case of this.

I haven't included the patch here now that converts nl82011 to use
this because it doesn't apply without another cleanup patch, but
we can remove a number of hand-coded min/max length checks and get
better error messages from the general validation code while doing
that.

As I had originally built the netlink policy export to userspace in
a way that has min/max length for NLA_BINARY (for the types that we
used to call NLA_MIN_LEN, NLA_BINARY and NLA_EXACT_LEN) anyway, it
doesn't really change anything there except that now there's a chance
that userspace sees min length < max length, which previously wasn't
possible.

v2:
 * fix the min<max comment to correctly say min<=max
====================

Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 06a4ec1d 8aa26c57

drivers/net/macsec.c

+1 −1
Original line number Diff line number Diff line
@@ -1611,7 +1611,7 @@ static const struct nla_policy macsec_genl_rxsc_policy[NUM_MACSEC_RXSC_ATTR] = { 
static const struct nla_policy macsec_genl_sa_policy[NUM_MACSEC_SA_ATTR] = {

	[MACSEC_SA_ATTR_AN] = { .type = NLA_U8 },

	[MACSEC_SA_ATTR_ACTIVE] = { .type = NLA_U8 },

	[MACSEC_SA_ATTR_PN] = { .type = NLA_MIN_LEN, .len = 4 },

	[MACSEC_SA_ATTR_PN] = NLA_POLICY_MIN_LEN(4),

	[MACSEC_SA_ATTR_KEYID] = { .type = NLA_BINARY,

				   .len = MACSEC_KEYID_LEN, },

	[MACSEC_SA_ATTR_KEY] = { .type = NLA_BINARY,

drivers/net/wireguard/netlink.c

+7 −7
Original line number Diff line number Diff line
@@ -22,8 +22,8 @@ static struct genl_family genl_family; 
static const struct nla_policy device_policy[WGDEVICE_A_MAX + 1] = {

	[WGDEVICE_A_IFINDEX]		= { .type = NLA_U32 },

	[WGDEVICE_A_IFNAME]		= { .type = NLA_NUL_STRING, .len = IFNAMSIZ - 1 },

	[WGDEVICE_A_PRIVATE_KEY]	= { .type = NLA_EXACT_LEN, .len = NOISE_PUBLIC_KEY_LEN },

	[WGDEVICE_A_PUBLIC_KEY]		= { .type = NLA_EXACT_LEN, .len = NOISE_PUBLIC_KEY_LEN },

	[WGDEVICE_A_PRIVATE_KEY]	= NLA_POLICY_EXACT_LEN(NOISE_PUBLIC_KEY_LEN),

	[WGDEVICE_A_PUBLIC_KEY]		= NLA_POLICY_EXACT_LEN(NOISE_PUBLIC_KEY_LEN),

	[WGDEVICE_A_FLAGS]		= { .type = NLA_U32 },

	[WGDEVICE_A_LISTEN_PORT]	= { .type = NLA_U16 },

	[WGDEVICE_A_FWMARK]		= { .type = NLA_U32 },
@@ -31,12 +31,12 @@ static const struct nla_policy device_policy[WGDEVICE_A_MAX + 1] = { 
};



static const struct nla_policy peer_policy[WGPEER_A_MAX + 1] = {

	[WGPEER_A_PUBLIC_KEY]				= { .type = NLA_EXACT_LEN, .len = NOISE_PUBLIC_KEY_LEN },

	[WGPEER_A_PRESHARED_KEY]			= { .type = NLA_EXACT_LEN, .len = NOISE_SYMMETRIC_KEY_LEN },

	[WGPEER_A_PUBLIC_KEY]				= NLA_POLICY_EXACT_LEN(NOISE_PUBLIC_KEY_LEN),

	[WGPEER_A_PRESHARED_KEY]			= NLA_POLICY_EXACT_LEN(NOISE_SYMMETRIC_KEY_LEN),

	[WGPEER_A_FLAGS]				= { .type = NLA_U32 },

	[WGPEER_A_ENDPOINT]				= { .type = NLA_MIN_LEN, .len = sizeof(struct sockaddr) },

	[WGPEER_A_ENDPOINT]				= NLA_POLICY_MIN_LEN(sizeof(struct sockaddr)),

	[WGPEER_A_PERSISTENT_KEEPALIVE_INTERVAL]	= { .type = NLA_U16 },

	[WGPEER_A_LAST_HANDSHAKE_TIME]			= { .type = NLA_EXACT_LEN, .len = sizeof(struct __kernel_timespec) },

	[WGPEER_A_LAST_HANDSHAKE_TIME]			= NLA_POLICY_EXACT_LEN(sizeof(struct __kernel_timespec)),

	[WGPEER_A_RX_BYTES]				= { .type = NLA_U64 },

	[WGPEER_A_TX_BYTES]				= { .type = NLA_U64 },

	[WGPEER_A_ALLOWEDIPS]				= { .type = NLA_NESTED },
@@ -45,7 +45,7 @@ static const struct nla_policy peer_policy[WGPEER_A_MAX + 1] = { 


static const struct nla_policy allowedip_policy[WGALLOWEDIP_A_MAX + 1] = {

	[WGALLOWEDIP_A_FAMILY]		= { .type = NLA_U16 },

	[WGALLOWEDIP_A_IPADDR]		= { .type = NLA_MIN_LEN, .len = sizeof(struct in_addr) },

	[WGALLOWEDIP_A_IPADDR]		= NLA_POLICY_MIN_LEN(sizeof(struct in_addr)),

	[WGALLOWEDIP_A_CIDR_MASK]	= { .type = NLA_U8 }

};

include/net/netlink.h

+31 −27
Original line number Diff line number Diff line
@@ -181,8 +181,6 @@ enum { 
	NLA_S64,

	NLA_BITFIELD32,

	NLA_REJECT,

	NLA_EXACT_LEN,

	NLA_MIN_LEN,

	__NLA_TYPE_MAX,

};
@@ -199,11 +197,11 @@ struct netlink_range_validation_signed { 
enum nla_policy_validation {

	NLA_VALIDATE_NONE,

	NLA_VALIDATE_RANGE,

	NLA_VALIDATE_RANGE_WARN_TOO_LONG,

	NLA_VALIDATE_MIN,

	NLA_VALIDATE_MAX,

	NLA_VALIDATE_RANGE_PTR,

	NLA_VALIDATE_FUNCTION,

	NLA_VALIDATE_WARN_TOO_LONG,

};



/**
@@ -222,7 +220,7 @@ enum nla_policy_validation { 
 *    NLA_NUL_STRING       Maximum length of string (excluding NUL)

 *    NLA_FLAG             Unused

 *    NLA_BINARY           Maximum length of attribute payload

 *    NLA_MIN_LEN          Minimum length of attribute payload

 *                         (but see also below with the validation type)

 *    NLA_NESTED,

 *    NLA_NESTED_ARRAY     Length verification is done by checking len of

 *                         nested header (or empty); len field is used if
@@ -237,11 +235,6 @@ enum nla_policy_validation { 
 *                         just like "All other"

 *    NLA_BITFIELD32       Unused

 *    NLA_REJECT           Unused

 *    NLA_EXACT_LEN        Attribute should have exactly this length, otherwise

 *                         it is rejected or warned about, the latter happening

 *                         if and only if the `validation_type' is set to

 *                         NLA_VALIDATE_WARN_TOO_LONG.

 *    NLA_MIN_LEN          Minimum length of attribute payload

 *    All other            Minimum length of attribute payload

 *

 * Meaning of validation union:
@@ -296,6 +289,11 @@ enum nla_policy_validation { 
 *                         pointer to a struct netlink_range_validation_signed

 *                         that indicates the min/max values.

 *                         Use NLA_POLICY_FULL_RANGE_SIGNED().

 *

 *    NLA_BINARY           If the validation type is like the ones for integers

 *                         above, then the min/max length (not value like for

 *                         integers) of the attribute is enforced.

 *

 *    All other            Unused - but note that it's a union

 *

 * Meaning of `validate' field, use via NLA_POLICY_VALIDATE_FN:
@@ -309,7 +307,7 @@ enum nla_policy_validation { 
 * static const struct nla_policy my_policy[ATTR_MAX+1] = {

 * 	[ATTR_FOO] = { .type = NLA_U16 },

 *	[ATTR_BAR] = { .type = NLA_STRING, .len = BARSIZ },

 *	[ATTR_BAZ] = { .type = NLA_EXACT_LEN, .len = sizeof(struct mystruct) },

 *	[ATTR_BAZ] = NLA_POLICY_EXACT_LEN(sizeof(struct mystruct)),

 *	[ATTR_GOO] = NLA_POLICY_BITFIELD32(myvalidflags),

 * };

 */
@@ -335,9 +333,10 @@ struct nla_policy { 
		 * nesting validation starts here.

		 *

		 * Additionally, it means that NLA_UNSPEC is actually NLA_REJECT

		 * for any types >= this, so need to use NLA_MIN_LEN to get the

		 * previous pure { .len = xyz } behaviour. The advantage of this

		 * is that types not specified in the policy will be rejected.

		 * for any types >= this, so need to use NLA_POLICY_MIN_LEN() to

		 * get the previous pure { .len = xyz } behaviour. The advantage

		 * of this is that types not specified in the policy will be

		 * rejected.

		 *

		 * For completely new families it should be set to 1 so that the

		 * validation is enforced for all attributes. For existing ones
@@ -349,12 +348,6 @@ struct nla_policy { 
	};

};



#define NLA_POLICY_EXACT_LEN(_len)	{ .type = NLA_EXACT_LEN, .len = _len }

#define NLA_POLICY_EXACT_LEN_WARN(_len) \

	{ .type = NLA_EXACT_LEN, .len = _len, \

	  .validation_type = NLA_VALIDATE_WARN_TOO_LONG, }

#define NLA_POLICY_MIN_LEN(_len)	{ .type = NLA_MIN_LEN, .len = _len }



#define NLA_POLICY_ETH_ADDR		NLA_POLICY_EXACT_LEN(ETH_ALEN)

#define NLA_POLICY_ETH_ADDR_COMPAT	NLA_POLICY_EXACT_LEN_WARN(ETH_ALEN)
@@ -370,19 +363,21 @@ struct nla_policy { 
	{ .type = NLA_BITFIELD32, .bitfield32_valid = valid }



#define __NLA_ENSURE(condition) BUILD_BUG_ON_ZERO(!(condition))

#define NLA_ENSURE_UINT_TYPE(tp)			\

#define NLA_ENSURE_UINT_OR_BINARY_TYPE(tp)		\

	(__NLA_ENSURE(tp == NLA_U8 || tp == NLA_U16 ||	\

		      tp == NLA_U32 || tp == NLA_U64 ||	\

		      tp == NLA_MSECS) + tp)

		      tp == NLA_MSECS ||		\

		      tp == NLA_BINARY) + tp)

#define NLA_ENSURE_SINT_TYPE(tp)			\

	(__NLA_ENSURE(tp == NLA_S8 || tp == NLA_S16  ||	\

		      tp == NLA_S32 || tp == NLA_S64) + tp)

#define NLA_ENSURE_INT_TYPE(tp)				\

#define NLA_ENSURE_INT_OR_BINARY_TYPE(tp)		\

	(__NLA_ENSURE(tp == NLA_S8 || tp == NLA_U8 ||	\

		      tp == NLA_S16 || tp == NLA_U16 ||	\

		      tp == NLA_S32 || tp == NLA_U32 ||	\

		      tp == NLA_S64 || tp == NLA_U64 ||	\

		      tp == NLA_MSECS) + tp)

		      tp == NLA_MSECS ||		\

		      tp == NLA_BINARY) + tp)

#define NLA_ENSURE_NO_VALIDATION_PTR(tp)		\

	(__NLA_ENSURE(tp != NLA_BITFIELD32 &&		\

		      tp != NLA_REJECT &&		\
@@ -390,14 +385,14 @@ struct nla_policy { 
		      tp != NLA_NESTED_ARRAY) + tp)



#define NLA_POLICY_RANGE(tp, _min, _max) {		\

	.type = NLA_ENSURE_INT_TYPE(tp),		\

	.type = NLA_ENSURE_INT_OR_BINARY_TYPE(tp),	\

	.validation_type = NLA_VALIDATE_RANGE,		\

	.min = _min,					\

	.max = _max					\

}



#define NLA_POLICY_FULL_RANGE(tp, _range) {		\

	.type = NLA_ENSURE_UINT_TYPE(tp),		\

	.type = NLA_ENSURE_UINT_OR_BINARY_TYPE(tp),	\

	.validation_type = NLA_VALIDATE_RANGE_PTR,	\

	.range = _range,				\

}
@@ -409,13 +404,13 @@ struct nla_policy { 
}



#define NLA_POLICY_MIN(tp, _min) {			\

	.type = NLA_ENSURE_INT_TYPE(tp),		\

	.type = NLA_ENSURE_INT_OR_BINARY_TYPE(tp),	\

	.validation_type = NLA_VALIDATE_MIN,		\

	.min = _min,					\

}



#define NLA_POLICY_MAX(tp, _max) {			\

	.type = NLA_ENSURE_INT_TYPE(tp),		\

	.type = NLA_ENSURE_INT_OR_BINARY_TYPE(tp),	\

	.validation_type = NLA_VALIDATE_MAX,		\

	.max = _max,					\

}
@@ -427,6 +422,15 @@ struct nla_policy { 
	.len = __VA_ARGS__ + 0,				\

}



#define NLA_POLICY_EXACT_LEN(_len)	NLA_POLICY_RANGE(NLA_BINARY, _len, _len)

#define NLA_POLICY_EXACT_LEN_WARN(_len) {			\

	.type = NLA_BINARY,					\

	.validation_type = NLA_VALIDATE_RANGE_WARN_TOO_LONG,	\

	.min = _len,						\

	.max = _len						\

}

#define NLA_POLICY_MIN_LEN(_len)	NLA_POLICY_MIN(NLA_BINARY, _len)



/**

 * struct nl_info - netlink source information

 * @nlh: Netlink message header of original request

lib/nlattr.c

+39 −21
Original line number Diff line number Diff line
@@ -124,6 +124,7 @@ void nla_get_range_unsigned(const struct nla_policy *pt, 
		range->max = U8_MAX;

		break;

	case NLA_U16:

	case NLA_BINARY:

		range->max = U16_MAX;

		break;

	case NLA_U32:
@@ -140,6 +141,7 @@ void nla_get_range_unsigned(const struct nla_policy *pt, 


	switch (pt->validation_type) {

	case NLA_VALIDATE_RANGE:

	case NLA_VALIDATE_RANGE_WARN_TOO_LONG:

		range->min = pt->min;

		range->max = pt->max;

		break;
@@ -157,9 +159,10 @@ void nla_get_range_unsigned(const struct nla_policy *pt, 
	}

}



static int nla_validate_int_range_unsigned(const struct nla_policy *pt,

static int nla_validate_range_unsigned(const struct nla_policy *pt,

				       const struct nlattr *nla,

					   struct netlink_ext_ack *extack)

				       struct netlink_ext_ack *extack,

				       unsigned int validate)

{

	struct netlink_range_validation range;

	u64 value;
@@ -178,15 +181,39 @@ static int nla_validate_int_range_unsigned(const struct nla_policy *pt, 
	case NLA_MSECS:

		value = nla_get_u64(nla);

		break;

	case NLA_BINARY:

		value = nla_len(nla);

		break;

	default:

		return -EINVAL;

	}



	nla_get_range_unsigned(pt, &range);



	if (pt->validation_type == NLA_VALIDATE_RANGE_WARN_TOO_LONG &&

	    pt->type == NLA_BINARY && value > range.max) {

		pr_warn_ratelimited("netlink: '%s': attribute type %d has an invalid length.\n",

				    current->comm, pt->type);

		if (validate & NL_VALIDATE_STRICT_ATTRS) {

			NL_SET_ERR_MSG_ATTR(extack, nla,

					    "invalid attribute length");

			return -EINVAL;

		}



		/* this assumes min <= max (don't validate against min) */

		return 0;

	}



	if (value < range.min || value > range.max) {

		bool binary = pt->type == NLA_BINARY;



		if (binary)

			NL_SET_ERR_MSG_ATTR(extack, nla,

					    "binary attribute size out of range");

		else

			NL_SET_ERR_MSG_ATTR(extack, nla,

					    "integer out of range");



		return -ERANGE;

	}
@@ -274,7 +301,8 @@ static int nla_validate_int_range_signed(const struct nla_policy *pt, 


static int nla_validate_int_range(const struct nla_policy *pt,

				  const struct nlattr *nla,

				  struct netlink_ext_ack *extack)

				  struct netlink_ext_ack *extack,

				  unsigned int validate)

{

	switch (pt->type) {

	case NLA_U8:
@@ -282,7 +310,8 @@ static int nla_validate_int_range(const struct nla_policy *pt, 
	case NLA_U32:

	case NLA_U64:

	case NLA_MSECS:

		return nla_validate_int_range_unsigned(pt, nla, extack);

	case NLA_BINARY:

		return nla_validate_range_unsigned(pt, nla, extack, validate);

	case NLA_S8:

	case NLA_S16:

	case NLA_S32:
@@ -313,10 +342,7 @@ static int validate_nla(const struct nlattr *nla, int maxtype, 


	BUG_ON(pt->type > NLA_TYPE_MAX);



	if ((nla_attr_len[pt->type] && attrlen != nla_attr_len[pt->type]) ||

	    (pt->type == NLA_EXACT_LEN &&

	     pt->validation_type == NLA_VALIDATE_WARN_TOO_LONG &&

	     attrlen != pt->len)) {

	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);

		if (validate & NL_VALIDATE_STRICT_ATTRS) {
@@ -449,19 +475,10 @@ static int validate_nla(const struct nlattr *nla, int maxtype, 
					    "Unsupported attribute");

			return -EINVAL;

		}

		/* fall through */

	case NLA_MIN_LEN:

		if (attrlen < pt->len)

			goto out_err;

		break;



	case NLA_EXACT_LEN:

		if (pt->validation_type != NLA_VALIDATE_WARN_TOO_LONG) {

			if (attrlen != pt->len)

				goto out_err;

			break;

		}

		/* fall through */

	default:

		if (pt->len)

			minlen = pt->len;
@@ -479,9 +496,10 @@ static int validate_nla(const struct nlattr *nla, int maxtype, 
		break;

	case NLA_VALIDATE_RANGE_PTR:

	case NLA_VALIDATE_RANGE:

	case NLA_VALIDATE_RANGE_WARN_TOO_LONG:

	case NLA_VALIDATE_MIN:

	case NLA_VALIDATE_MAX:

		err = nla_validate_int_range(pt, nla, extack);

		err = nla_validate_int_range(pt, nla, extack, validate);

		if (err)

			return err;

		break;

net/bridge/br_netlink.c

+2 −2
Original line number Diff line number Diff line
@@ -1095,8 +1095,8 @@ static const struct nla_policy br_policy[IFLA_BR_MAX + 1] = { 
	[IFLA_BR_MCAST_IGMP_VERSION] = { .type = NLA_U8 },

	[IFLA_BR_MCAST_MLD_VERSION] = { .type = NLA_U8 },

	[IFLA_BR_VLAN_STATS_PER_PORT] = { .type = NLA_U8 },

	[IFLA_BR_MULTI_BOOLOPT] = { .type = NLA_EXACT_LEN,

				    .len = sizeof(struct br_boolopt_multi) },

	[IFLA_BR_MULTI_BOOLOPT] =

		NLA_POLICY_EXACT_LEN(sizeof(struct br_boolopt_multi)),

};



static int br_changelink(struct net_device *brdev, struct nlattr *tb[],