Re: [patch] brcmfmac: use kcalloc() to prevent integer overflow

["Arend van Spriel" <arend@broadcom.com>]

On 09/26/2012 09:31 AM, Dan Carpenter wrote:
> Speaking of integer overflows, I had a couple other concerns in this
> file.
>
> drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c brcmf_enq_event()
>    4144          total_len = sizeof(struct brcmf_cfg80211_event_q);
>    4145          if (data)
>    4146                  data_len = be32_to_cpu(msg->datalen);
>    4147          else
>    4148                  data_len = 0;
>    4149          total_len += data_len;
>                  ^^^^^^^^^^^^^^^^^^^^^
> This looks very suspicious like a remote exploitable overflow.

Hi Dan,

The event message is generated in our device so I believe there is 
little room for exploits.

>    4150          e = kzalloc(total_len, GFP_ATOMIC);
>
> drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c brcmf_run_escan()
>     882          if (request != NULL) {
>     883                  /* Allocate space for populating ssids in struct */
>     884                  params_size += sizeof(u32) * ((request->n_channels + 1) / 2);
>     885
>     886                  /* Allocate space for populating ssids in struct */
>     887                  params_size += sizeof(struct brcmf_ssid) * request->n_ssids;
>     888          }
>     889
>     890          params = kzalloc(params_size, GFP_KERNEL);
>
> I didn't track back where request comes from so I don't know if
> that's a problem or not.  I figured you would know better than I
> would.

This request comes from user-space, ie. nl80211. cfg80211 does make sure 
that amount channels and ssids are sane (see nl80211_trigger_scan() in 
net/wireless/nl80211.c).

Gr. AvS