* [PATCH v2 1/2] netlink: add NLA_REJECT policy type
@ 2018-09-17 9:57 Johannes Berg
2018-09-17 9:57 ` [PATCH v2 2/2] netlink: add ethernet address policy types Johannes Berg
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Johannes Berg @ 2018-09-17 9:57 UTC (permalink / raw)
To: netdev; +Cc: Marcelo Ricardo Leitner, Michal Kubecek, Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
In some situations some netlink attributes may be used for output
only (kernel->userspace) or may be reserved for future use. It's
then helpful to be able to prevent userspace from using them in
messages sent to the kernel, since they'd otherwise be ignored and
any future will become impossible if this happens.
Add NLA_REJECT to the policy which does nothing but reject (with
EINVAL) validation of any messages containing this attribute.
Allow for returning a specific extended ACK error message in the
validation_data pointer.
While at it clear up the documentation a bit - the NLA_BITFIELD32
documentation was added to the list of len field descriptions.
Also, use NL_SET_BAD_ATTR() in one place where it's open-coded.
The specific case I have in mind now is a shared nested attribute
containing request/response data, and it would be pointless and
potentially confusing to have userspace include response data in
the messages that actually contain a request.
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
v2: preserve behaviour of overwriting the extack message, with
either the generic or the specific one now
---
include/net/netlink.h | 13 ++++++++++++-
lib/nlattr.c | 23 ++++++++++++++++-------
2 files changed, 28 insertions(+), 8 deletions(-)
diff --git a/include/net/netlink.h b/include/net/netlink.h
index 0c154f98e987..b318b0a9f6c3 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -180,6 +180,7 @@ enum {
NLA_S32,
NLA_S64,
NLA_BITFIELD32,
+ NLA_REJECT,
__NLA_TYPE_MAX,
};
@@ -208,9 +209,19 @@ enum {
* NLA_MSECS Leaving the length field zero will verify the
* given type fits, using it verifies minimum length
* just like "All other"
- * NLA_BITFIELD32 A 32-bit bitmap/bitselector attribute
+ * NLA_BITFIELD32 Unused
+ * NLA_REJECT Unused
* All other Minimum length of attribute payload
*
+ * Meaning of `validation_data' field:
+ * NLA_BITFIELD32 This is a 32-bit bitmap/bitselector attribute and
+ * validation data must point to a u32 value of valid
+ * flags
+ * NLA_REJECT This attribute is always rejected and validation data
+ * may point to a string to report as the error instead
+ * of the generic one in extended ACK.
+ * All other Unused
+ *
* Example:
* static const struct nla_policy my_policy[ATTR_MAX+1] = {
* [ATTR_FOO] = { .type = NLA_U16 },
diff --git a/lib/nlattr.c b/lib/nlattr.c
index e335bcafa9e4..36d74b079151 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -69,7 +69,8 @@ static int validate_nla_bitfield32(const struct nlattr *nla,
}
static int validate_nla(const struct nlattr *nla, int maxtype,
- const struct nla_policy *policy)
+ const struct nla_policy *policy,
+ const char **error_msg)
{
const struct nla_policy *pt;
int minlen = 0, attrlen = nla_len(nla), type = nla_type(nla);
@@ -87,6 +88,11 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
}
switch (pt->type) {
+ case NLA_REJECT:
+ if (pt->validation_data && error_msg)
+ *error_msg = pt->validation_data;
+ return -EINVAL;
+
case NLA_FLAG:
if (attrlen > 0)
return -ERANGE;
@@ -180,11 +186,10 @@ int nla_validate(const struct nlattr *head, int len, int maxtype,
int rem;
nla_for_each_attr(nla, head, len, rem) {
- int err = validate_nla(nla, maxtype, policy);
+ int err = validate_nla(nla, maxtype, policy, NULL);
if (err < 0) {
- if (extack)
- extack->bad_attr = nla;
+ NL_SET_BAD_ATTR(extack, nla);
return err;
}
}
@@ -250,11 +255,15 @@ int nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
u16 type = nla_type(nla);
if (type > 0 && type <= maxtype) {
+ static const char _msg[] = "Attribute failed policy validation";
+ const char *msg = _msg;
+
if (policy) {
- err = validate_nla(nla, maxtype, policy);
+ err = validate_nla(nla, maxtype, policy, &msg);
if (err < 0) {
- NL_SET_ERR_MSG_ATTR(extack, nla,
- "Attribute failed policy validation");
+ NL_SET_BAD_ATTR(extack, nla);
+ if (extack)
+ extack->_msg = msg;
goto errout;
}
}
--
2.14.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] netlink: add ethernet address policy types
2018-09-17 9:57 [PATCH v2 1/2] netlink: add NLA_REJECT policy type Johannes Berg
@ 2018-09-17 9:57 ` Johannes Berg
2018-09-17 20:26 ` Marcelo Ricardo Leitner
2018-09-19 2:51 ` David Miller
2018-09-17 20:23 ` [PATCH v2 1/2] netlink: add NLA_REJECT policy type Marcelo Ricardo Leitner
2018-09-19 2:51 ` David Miller
2 siblings, 2 replies; 6+ messages in thread
From: Johannes Berg @ 2018-09-17 9:57 UTC (permalink / raw)
To: netdev; +Cc: Marcelo Ricardo Leitner, Michal Kubecek, Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
Commonly, ethernet addresses are just using a policy of
{ .len = ETH_ALEN }
which leaves userspace free to send more data than it should,
which may hide bugs.
Introduce NLA_EXACT_LEN which checks for exact size, rejecting
the attribute if it's not exactly that length. Also add
NLA_EXACT_LEN_WARN which requires the minimum length and will
warn on longer attributes, for backward compatibility.
Use these to define NLA_POLICY_ETH_ADDR (new strict policy) and
NLA_POLICY_ETH_ADDR_COMPAT (compatible policy with warning);
these are used like this:
static const struct nla_policy <name>[...] = {
[NL_ATTR_NAME] = NLA_POLICY_ETH_ADDR,
...
};
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
v2: add only NLA_EXACT_LEN/NLA_EXACT_LEN_WARN and build on top
of that for ethernet address validation, so it can be extended
for other types (e.g. IPv6 addresses)
---
include/net/netlink.h | 13 +++++++++++++
lib/nlattr.c | 8 +++++++-
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/include/net/netlink.h b/include/net/netlink.h
index b318b0a9f6c3..318b1ded3833 100644
--- a/include/net/netlink.h
+++ b/include/net/netlink.h
@@ -181,6 +181,8 @@ enum {
NLA_S64,
NLA_BITFIELD32,
NLA_REJECT,
+ NLA_EXACT_LEN,
+ NLA_EXACT_LEN_WARN,
__NLA_TYPE_MAX,
};
@@ -211,6 +213,10 @@ enum {
* just like "All other"
* NLA_BITFIELD32 Unused
* NLA_REJECT Unused
+ * NLA_EXACT_LEN Attribute must have exactly this length, otherwise
+ * it is rejected.
+ * NLA_EXACT_LEN_WARN Attribute should have exactly this length, a warning
+ * is logged if it is longer, shorter is rejected.
* All other Minimum length of attribute payload
*
* Meaning of `validation_data' field:
@@ -236,6 +242,13 @@ struct nla_policy {
void *validation_data;
};
+#define NLA_POLICY_EXACT_LEN(_len) { .type = NLA_EXACT_LEN, .len = _len }
+#define NLA_POLICY_EXACT_LEN_WARN(_len) { .type = NLA_EXACT_LEN_WARN, \
+ .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)
+
/**
* struct nl_info - netlink source information
* @nlh: Netlink message header of original request
diff --git a/lib/nlattr.c b/lib/nlattr.c
index 36d74b079151..bb6fe5ed4ecf 100644
--- a/lib/nlattr.c
+++ b/lib/nlattr.c
@@ -82,12 +82,18 @@ 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]) {
+ if ((nla_attr_len[pt->type] && attrlen != nla_attr_len[pt->type]) ||
+ (pt->type == NLA_EXACT_LEN_WARN && attrlen != pt->len)) {
pr_warn_ratelimited("netlink: '%s': attribute type %d has an invalid length.\n",
current->comm, type);
}
switch (pt->type) {
+ case NLA_EXACT_LEN:
+ if (attrlen != pt->len)
+ return -ERANGE;
+ break;
+
case NLA_REJECT:
if (pt->validation_data && error_msg)
*error_msg = pt->validation_data;
--
2.14.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] netlink: add NLA_REJECT policy type
2018-09-17 9:57 [PATCH v2 1/2] netlink: add NLA_REJECT policy type Johannes Berg
2018-09-17 9:57 ` [PATCH v2 2/2] netlink: add ethernet address policy types Johannes Berg
@ 2018-09-17 20:23 ` Marcelo Ricardo Leitner
2018-09-19 2:51 ` David Miller
2 siblings, 0 replies; 6+ messages in thread
From: Marcelo Ricardo Leitner @ 2018-09-17 20:23 UTC (permalink / raw)
To: Johannes Berg; +Cc: netdev, Michal Kubecek, Johannes Berg
On Mon, Sep 17, 2018 at 11:57:28AM +0200, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> In some situations some netlink attributes may be used for output
> only (kernel->userspace) or may be reserved for future use. It's
> then helpful to be able to prevent userspace from using them in
> messages sent to the kernel, since they'd otherwise be ignored and
> any future will become impossible if this happens.
>
> Add NLA_REJECT to the policy which does nothing but reject (with
> EINVAL) validation of any messages containing this attribute.
> Allow for returning a specific extended ACK error message in the
> validation_data pointer.
>
> While at it clear up the documentation a bit - the NLA_BITFIELD32
> documentation was added to the list of len field descriptions.
>
> Also, use NL_SET_BAD_ATTR() in one place where it's open-coded.
>
> The specific case I have in mind now is a shared nested attribute
> containing request/response data, and it would be pointless and
> potentially confusing to have userspace include response data in
> the messages that actually contain a request.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> v2: preserve behaviour of overwriting the extack message, with
> either the generic or the specific one now
> ---
> include/net/netlink.h | 13 ++++++++++++-
> lib/nlattr.c | 23 ++++++++++++++++-------
> 2 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/include/net/netlink.h b/include/net/netlink.h
> index 0c154f98e987..b318b0a9f6c3 100644
> --- a/include/net/netlink.h
> +++ b/include/net/netlink.h
> @@ -180,6 +180,7 @@ enum {
> NLA_S32,
> NLA_S64,
> NLA_BITFIELD32,
> + NLA_REJECT,
> __NLA_TYPE_MAX,
> };
>
> @@ -208,9 +209,19 @@ enum {
> * NLA_MSECS Leaving the length field zero will verify the
> * given type fits, using it verifies minimum length
> * just like "All other"
> - * NLA_BITFIELD32 A 32-bit bitmap/bitselector attribute
> + * NLA_BITFIELD32 Unused
> + * NLA_REJECT Unused
> * All other Minimum length of attribute payload
> *
> + * Meaning of `validation_data' field:
> + * NLA_BITFIELD32 This is a 32-bit bitmap/bitselector attribute and
> + * validation data must point to a u32 value of valid
> + * flags
> + * NLA_REJECT This attribute is always rejected and validation data
> + * may point to a string to report as the error instead
> + * of the generic one in extended ACK.
> + * All other Unused
> + *
> * Example:
> * static const struct nla_policy my_policy[ATTR_MAX+1] = {
> * [ATTR_FOO] = { .type = NLA_U16 },
> diff --git a/lib/nlattr.c b/lib/nlattr.c
> index e335bcafa9e4..36d74b079151 100644
> --- a/lib/nlattr.c
> +++ b/lib/nlattr.c
> @@ -69,7 +69,8 @@ static int validate_nla_bitfield32(const struct nlattr *nla,
> }
>
> static int validate_nla(const struct nlattr *nla, int maxtype,
> - const struct nla_policy *policy)
> + const struct nla_policy *policy,
> + const char **error_msg)
> {
> const struct nla_policy *pt;
> int minlen = 0, attrlen = nla_len(nla), type = nla_type(nla);
> @@ -87,6 +88,11 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
> }
>
> switch (pt->type) {
> + case NLA_REJECT:
> + if (pt->validation_data && error_msg)
> + *error_msg = pt->validation_data;
> + return -EINVAL;
> +
> case NLA_FLAG:
> if (attrlen > 0)
> return -ERANGE;
> @@ -180,11 +186,10 @@ int nla_validate(const struct nlattr *head, int len, int maxtype,
> int rem;
>
> nla_for_each_attr(nla, head, len, rem) {
> - int err = validate_nla(nla, maxtype, policy);
> + int err = validate_nla(nla, maxtype, policy, NULL);
>
> if (err < 0) {
> - if (extack)
> - extack->bad_attr = nla;
> + NL_SET_BAD_ATTR(extack, nla);
> return err;
> }
> }
> @@ -250,11 +255,15 @@ int nla_parse(struct nlattr **tb, int maxtype, const struct nlattr *head,
> u16 type = nla_type(nla);
>
> if (type > 0 && type <= maxtype) {
> + static const char _msg[] = "Attribute failed policy validation";
> + const char *msg = _msg;
> +
> if (policy) {
> - err = validate_nla(nla, maxtype, policy);
> + err = validate_nla(nla, maxtype, policy, &msg);
> if (err < 0) {
> - NL_SET_ERR_MSG_ATTR(extack, nla,
> - "Attribute failed policy validation");
> + NL_SET_BAD_ATTR(extack, nla);
> + if (extack)
> + extack->_msg = msg;
> goto errout;
> }
> }
> --
> 2.14.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] netlink: add ethernet address policy types
2018-09-17 9:57 ` [PATCH v2 2/2] netlink: add ethernet address policy types Johannes Berg
@ 2018-09-17 20:26 ` Marcelo Ricardo Leitner
2018-09-19 2:51 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: Marcelo Ricardo Leitner @ 2018-09-17 20:26 UTC (permalink / raw)
To: Johannes Berg; +Cc: netdev, Michal Kubecek, Johannes Berg
On Mon, Sep 17, 2018 at 11:57:29AM +0200, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Commonly, ethernet addresses are just using a policy of
> { .len = ETH_ALEN }
> which leaves userspace free to send more data than it should,
> which may hide bugs.
>
> Introduce NLA_EXACT_LEN which checks for exact size, rejecting
> the attribute if it's not exactly that length. Also add
> NLA_EXACT_LEN_WARN which requires the minimum length and will
> warn on longer attributes, for backward compatibility.
>
> Use these to define NLA_POLICY_ETH_ADDR (new strict policy) and
> NLA_POLICY_ETH_ADDR_COMPAT (compatible policy with warning);
> these are used like this:
>
> static const struct nla_policy <name>[...] = {
> [NL_ATTR_NAME] = NLA_POLICY_ETH_ADDR,
> ...
> };
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> v2: add only NLA_EXACT_LEN/NLA_EXACT_LEN_WARN and build on top
> of that for ethernet address validation, so it can be extended
> for other types (e.g. IPv6 addresses)
> ---
> include/net/netlink.h | 13 +++++++++++++
> lib/nlattr.c | 8 +++++++-
> 2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/netlink.h b/include/net/netlink.h
> index b318b0a9f6c3..318b1ded3833 100644
> --- a/include/net/netlink.h
> +++ b/include/net/netlink.h
> @@ -181,6 +181,8 @@ enum {
> NLA_S64,
> NLA_BITFIELD32,
> NLA_REJECT,
> + NLA_EXACT_LEN,
> + NLA_EXACT_LEN_WARN,
> __NLA_TYPE_MAX,
> };
>
> @@ -211,6 +213,10 @@ enum {
> * just like "All other"
> * NLA_BITFIELD32 Unused
> * NLA_REJECT Unused
> + * NLA_EXACT_LEN Attribute must have exactly this length, otherwise
> + * it is rejected.
> + * NLA_EXACT_LEN_WARN Attribute should have exactly this length, a warning
> + * is logged if it is longer, shorter is rejected.
> * All other Minimum length of attribute payload
> *
> * Meaning of `validation_data' field:
> @@ -236,6 +242,13 @@ struct nla_policy {
> void *validation_data;
> };
>
> +#define NLA_POLICY_EXACT_LEN(_len) { .type = NLA_EXACT_LEN, .len = _len }
> +#define NLA_POLICY_EXACT_LEN_WARN(_len) { .type = NLA_EXACT_LEN_WARN, \
> + .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)
> +
> /**
> * struct nl_info - netlink source information
> * @nlh: Netlink message header of original request
> diff --git a/lib/nlattr.c b/lib/nlattr.c
> index 36d74b079151..bb6fe5ed4ecf 100644
> --- a/lib/nlattr.c
> +++ b/lib/nlattr.c
> @@ -82,12 +82,18 @@ 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]) {
> + if ((nla_attr_len[pt->type] && attrlen != nla_attr_len[pt->type]) ||
> + (pt->type == NLA_EXACT_LEN_WARN && attrlen != pt->len)) {
> pr_warn_ratelimited("netlink: '%s': attribute type %d has an invalid length.\n",
> current->comm, type);
> }
>
> switch (pt->type) {
> + case NLA_EXACT_LEN:
> + if (attrlen != pt->len)
> + return -ERANGE;
> + break;
> +
> case NLA_REJECT:
> if (pt->validation_data && error_msg)
> *error_msg = pt->validation_data;
> --
> 2.14.4
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] netlink: add NLA_REJECT policy type
2018-09-17 9:57 [PATCH v2 1/2] netlink: add NLA_REJECT policy type Johannes Berg
2018-09-17 9:57 ` [PATCH v2 2/2] netlink: add ethernet address policy types Johannes Berg
2018-09-17 20:23 ` [PATCH v2 1/2] netlink: add NLA_REJECT policy type Marcelo Ricardo Leitner
@ 2018-09-19 2:51 ` David Miller
2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2018-09-19 2:51 UTC (permalink / raw)
To: johannes; +Cc: netdev, marcelo.leitner, mkubecek, johannes.berg
From: Johannes Berg <johannes@sipsolutions.net>
Date: Mon, 17 Sep 2018 11:57:28 +0200
> From: Johannes Berg <johannes.berg@intel.com>
>
> In some situations some netlink attributes may be used for output
> only (kernel->userspace) or may be reserved for future use. It's
> then helpful to be able to prevent userspace from using them in
> messages sent to the kernel, since they'd otherwise be ignored and
> any future will become impossible if this happens.
>
> Add NLA_REJECT to the policy which does nothing but reject (with
> EINVAL) validation of any messages containing this attribute.
> Allow for returning a specific extended ACK error message in the
> validation_data pointer.
>
> While at it clear up the documentation a bit - the NLA_BITFIELD32
> documentation was added to the list of len field descriptions.
>
> Also, use NL_SET_BAD_ATTR() in one place where it's open-coded.
>
> The specific case I have in mind now is a shared nested attribute
> containing request/response data, and it would be pointless and
> potentially confusing to have userspace include response data in
> the messages that actually contain a request.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
> v2: preserve behaviour of overwriting the extack message, with
> either the generic or the specific one now
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] netlink: add ethernet address policy types
2018-09-17 9:57 ` [PATCH v2 2/2] netlink: add ethernet address policy types Johannes Berg
2018-09-17 20:26 ` Marcelo Ricardo Leitner
@ 2018-09-19 2:51 ` David Miller
1 sibling, 0 replies; 6+ messages in thread
From: David Miller @ 2018-09-19 2:51 UTC (permalink / raw)
To: johannes; +Cc: netdev, marcelo.leitner, mkubecek, johannes.berg
From: Johannes Berg <johannes@sipsolutions.net>
Date: Mon, 17 Sep 2018 11:57:29 +0200
> From: Johannes Berg <johannes.berg@intel.com>
>
> Commonly, ethernet addresses are just using a policy of
> { .len = ETH_ALEN }
> which leaves userspace free to send more data than it should,
> which may hide bugs.
>
> Introduce NLA_EXACT_LEN which checks for exact size, rejecting
> the attribute if it's not exactly that length. Also add
> NLA_EXACT_LEN_WARN which requires the minimum length and will
> warn on longer attributes, for backward compatibility.
>
> Use these to define NLA_POLICY_ETH_ADDR (new strict policy) and
> NLA_POLICY_ETH_ADDR_COMPAT (compatible policy with warning);
> these are used like this:
>
> static const struct nla_policy <name>[...] = {
> [NL_ATTR_NAME] = NLA_POLICY_ETH_ADDR,
> ...
> };
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
> v2: add only NLA_EXACT_LEN/NLA_EXACT_LEN_WARN and build on top
> of that for ethernet address validation, so it can be extended
> for other types (e.g. IPv6 addresses)
Applied.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-09-19 8:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-09-17 9:57 [PATCH v2 1/2] netlink: add NLA_REJECT policy type Johannes Berg
2018-09-17 9:57 ` [PATCH v2 2/2] netlink: add ethernet address policy types Johannes Berg
2018-09-17 20:26 ` Marcelo Ricardo Leitner
2018-09-19 2:51 ` David Miller
2018-09-17 20:23 ` [PATCH v2 1/2] netlink: add NLA_REJECT policy type Marcelo Ricardo Leitner
2018-09-19 2:51 ` David Miller
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).