Re: [PATCH 4.4, 4.9, 4.14, 4.19] nl80211: validate beacon head

[Greg KH <greg@kroah.com>]

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