* [PATCH 4.4, 4.9, 4.14, 4.19] nl80211: validate beacon head
@ 2019-10-09 6:41 Johannes Berg
2019-10-09 9:27 ` Greg KH
2019-10-09 9:43 ` Greg KH
0 siblings, 2 replies; 6+ messages in thread
From: Johannes Berg @ 2019-10-09 6:41 UTC (permalink / raw)
To: linux-wireless, stable; +Cc: Johannes Berg
From: Johannes Berg <johannes.berg@intel.com>
Commit 8a3347aa110c76a7f87771999aed491d1d8779a8 upstream.
We currently don't validate the beacon head, i.e. the header,
fixed part and elements that are to go in front of the TIM
element. This means that the variable elements there can be
malformed, e.g. have a length exceeding the buffer size, but
most downstream code from this assumes that this has already
been checked.
Add the necessary checks to the netlink policy.
Cc: stable@vger.kernel.org
Fixes: ed1b6cc7f80f ("cfg80211/nl80211: add beacon settings")
Link: https://lore.kernel.org/r/1569009255-I7ac7fbe9436e9d8733439eab8acbbd35e55c74ef@changeid
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
net/wireless/nl80211.c | 38 ++++++++++++++++++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 6168db3c35e4..4a10ab388e0b 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -200,6 +200,38 @@ cfg80211_get_dev_from_info(struct net *netns, struct genl_info *info)
return __cfg80211_rdev_from_attrs(netns, info->attrs);
}
+static int validate_beacon_head(const struct nlattr *attr,
+ struct netlink_ext_ack *extack)
+{
+ const u8 *data = nla_data(attr);
+ unsigned int len = nla_len(attr);
+ const struct element *elem;
+ const struct ieee80211_mgmt *mgmt = (void *)data;
+ unsigned int fixedlen = offsetof(struct ieee80211_mgmt,
+ u.beacon.variable);
+
+ if (len < fixedlen)
+ goto err;
+
+ if (ieee80211_hdrlen(mgmt->frame_control) !=
+ offsetof(struct ieee80211_mgmt, u.beacon))
+ goto err;
+
+ data += fixedlen;
+ len -= fixedlen;
+
+ for_each_element(elem, data, len) {
+ /* nothing */
+ }
+
+ if (for_each_element_completed(elem, data, len))
+ return 0;
+
+err:
+ NL_SET_ERR_MSG_ATTR(extack, attr, "malformed beacon head");
+ return -EINVAL;
+}
+
/* policy for the attributes */
static const struct nla_policy nl80211_policy[NUM_NL80211_ATTR] = {
[NL80211_ATTR_WIPHY] = { .type = NLA_U32 },
@@ -4014,6 +4046,12 @@ static int nl80211_parse_beacon(struct nlattr *attrs[],
memset(bcn, 0, sizeof(*bcn));
if (attrs[NL80211_ATTR_BEACON_HEAD]) {
+ int ret = validate_beacon_head(attrs[NL80211_ATTR_BEACON_HEAD],
+ NULL);
+
+ if (ret)
+ return ret;
+
bcn->head = nla_data(attrs[NL80211_ATTR_BEACON_HEAD]);
bcn->head_len = nla_len(attrs[NL80211_ATTR_BEACON_HEAD]);
if (!bcn->head_len)
--
2.20.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 4.4, 4.9, 4.14, 4.19] nl80211: validate beacon head
2019-10-09 6:41 [PATCH 4.4, 4.9, 4.14, 4.19] nl80211: validate beacon head Johannes Berg
@ 2019-10-09 9:27 ` Greg KH
2019-10-09 9:29 ` Johannes Berg
2019-10-09 9:43 ` Greg KH
1 sibling, 1 reply; 6+ messages in thread
From: Greg KH @ 2019-10-09 9:27 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, stable, Johannes Berg
On Wed, Oct 09, 2019 at 08:41:09AM +0200, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Commit 8a3347aa110c76a7f87771999aed491d1d8779a8 upstream.
I don't see that commit in Linus's tree :(
Is this f88eb7c0d002 ("nl80211: validate beacon head")?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 4.4, 4.9, 4.14, 4.19] nl80211: validate beacon head
2019-10-09 9:27 ` Greg KH
@ 2019-10-09 9:29 ` Johannes Berg
0 siblings, 0 replies; 6+ messages in thread
From: Johannes Berg @ 2019-10-09 9:29 UTC (permalink / raw)
To: Greg KH; +Cc: linux-wireless, stable
On Wed, 2019-10-09 at 11:27 +0200, Greg KH wrote:
> On Wed, Oct 09, 2019 at 08:41:09AM +0200, Johannes Berg wrote:
> > From: Johannes Berg <johannes.berg@intel.com>
> >
> > Commit 8a3347aa110c76a7f87771999aed491d1d8779a8 upstream.
>
> I don't see that commit in Linus's tree :(
Hmmm. Not sure what happened there. I had created this in reply to
Sasha's bot, perhaps it told me a different commit ID or something.
Or maybe ... yes I think the bot had actually applied a *patch* rather
than picked one up, and I didn't really know what to do, but forgot
about this then.
> Is this f88eb7c0d002 ("nl80211: validate beacon head")?
Yes.
johannes
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 4.4, 4.9, 4.14, 4.19] nl80211: validate beacon head
2019-10-09 6:41 [PATCH 4.4, 4.9, 4.14, 4.19] nl80211: validate beacon head Johannes Berg
2019-10-09 9:27 ` Greg KH
@ 2019-10-09 9:43 ` Greg KH
2019-10-09 9:44 ` Johannes Berg
1 sibling, 1 reply; 6+ messages in thread
From: Greg KH @ 2019-10-09 9:43 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, stable, Johannes Berg
On Wed, Oct 09, 2019 at 08:41:09AM +0200, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Commit 8a3347aa110c76a7f87771999aed491d1d8779a8 upstream.
>
> We currently don't validate the beacon head, i.e. the header,
> fixed part and elements that are to go in front of the TIM
> element. This means that the variable elements there can be
> malformed, e.g. have a length exceeding the buffer size, but
> most downstream code from this assumes that this has already
> been checked.
>
> Add the necessary checks to the netlink policy.
>
> Cc: stable@vger.kernel.org
> Fixes: ed1b6cc7f80f ("cfg80211/nl80211: add beacon settings")
> Link: https://lore.kernel.org/r/1569009255-I7ac7fbe9436e9d8733439eab8acbbd35e55c74ef@changeid
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
> net/wireless/nl80211.c | 38 ++++++++++++++++++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
>
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 6168db3c35e4..4a10ab388e0b 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -200,6 +200,38 @@ cfg80211_get_dev_from_info(struct net *netns, struct genl_info *info)
> return __cfg80211_rdev_from_attrs(netns, info->attrs);
> }
>
> +static int validate_beacon_head(const struct nlattr *attr,
> + struct netlink_ext_ack *extack)
> +{
> + const u8 *data = nla_data(attr);
> + unsigned int len = nla_len(attr);
> + const struct element *elem;
> + const struct ieee80211_mgmt *mgmt = (void *)data;
> + unsigned int fixedlen = offsetof(struct ieee80211_mgmt,
> + u.beacon.variable);
> +
> + if (len < fixedlen)
> + goto err;
> +
> + if (ieee80211_hdrlen(mgmt->frame_control) !=
> + offsetof(struct ieee80211_mgmt, u.beacon))
> + goto err;
> +
> + data += fixedlen;
> + len -= fixedlen;
> +
> + for_each_element(elem, data, len) {
> + /* nothing */
> + }
for_each_element() is not in 4.4, 4.9, 4.14, or 4.19, so this breaks the
build :(
I'll drop this from my queues for now.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 4.4, 4.9, 4.14, 4.19] nl80211: validate beacon head
2019-10-09 9:43 ` Greg KH
@ 2019-10-09 9:44 ` Johannes Berg
2019-10-09 13:36 ` Greg KH
0 siblings, 1 reply; 6+ messages in thread
From: Johannes Berg @ 2019-10-09 9:44 UTC (permalink / raw)
To: Greg KH; +Cc: linux-wireless, stable
On Wed, 2019-10-09 at 11:43 +0200, Greg KH wrote:
>
> > + for_each_element(elem, data, len) {
> > + /* nothing */
> > + }
>
> for_each_element() is not in 4.4, 4.9, 4.14, or 4.19, so this breaks the
> build :(
Oh, right. I had previously also said:
You also need to cherry-pick
0f3b07f027f8 ("cfg80211: add and use strongly typed element iteration macros")
7388afe09143 ("cfg80211: Use const more consistently in for_each_element macros")
as dependencies - the latter doesn't apply cleanly as it has a change
that doesn't apply, but that part of the change isn't necessary.
johannes
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 4.4, 4.9, 4.14, 4.19] nl80211: validate beacon head
2019-10-09 9:44 ` Johannes Berg
@ 2019-10-09 13:36 ` Greg KH
0 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2019-10-09 13:36 UTC (permalink / raw)
To: Johannes Berg; +Cc: linux-wireless, stable
On Wed, Oct 09, 2019 at 11:44:03AM +0200, Johannes Berg wrote:
> On Wed, 2019-10-09 at 11:43 +0200, Greg KH wrote:
> >
> > > + for_each_element(elem, data, len) {
> > > + /* nothing */
> > > + }
> >
> > for_each_element() is not in 4.4, 4.9, 4.14, or 4.19, so this breaks the
> > build :(
>
> Oh, right. I had previously also said:
>
> You also need to cherry-pick
> 0f3b07f027f8 ("cfg80211: add and use strongly typed element iteration macros")
> 7388afe09143 ("cfg80211: Use const more consistently in for_each_element macros")
>
> as dependencies - the latter doesn't apply cleanly as it has a change
> that doesn't apply, but that part of the change isn't necessary.
Ok, that worked, thanks. Well, almost worked, I had to do a bit more
effort for 4.4.y, but all looks good now.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-10-09 13:37 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-09 6:41 [PATCH 4.4, 4.9, 4.14, 4.19] nl80211: validate beacon head Johannes Berg
2019-10-09 9:27 ` Greg KH
2019-10-09 9:29 ` Johannes Berg
2019-10-09 9:43 ` Greg KH
2019-10-09 9:44 ` Johannes Berg
2019-10-09 13:36 ` Greg KH
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).