brcmfmac: use kcalloc() to prevent integer overflow
Subsystems (2)
PATCH 1/1 brcmfmac: use kcalloc() to prevent integer overflow
The multiplication here looks like it could overflow. I've changed it to use kcalloc() to prevent that.
---
Only needed in linux-next. This was added in e58060723c "brcmfmac:
introduce scheduled scan support".
drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c +2 -2
@@ -3299,8 +3299,8 @@ brcmf_notify_sched_scan_results(struct brcmf_cfg80211_priv *cfg_priv,int i; request =kzalloc (sizeof (*request),GFP_KERNEL ); - ssid =kzalloc (sizeof (*ssid) * result_count,GFP_KERNEL ); - channel =kzalloc (sizeof (*channel) * result_count,GFP_KERNEL ); + ssid =kcalloc (result_count,sizeof (*ssid),GFP_KERNEL ); + channel =kcalloc (result_count,sizeof (*channel),GFP_KERNEL );if (!request || !ssid || !channel) { err = -ENOMEM ; goto out_err;
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.
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.
regards,
dan carpenter
Quoted 4 lines · Dan Carpenter
Speaking of integer overflows, I had a couple other concerns in this file.
drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.cbrcmf_enq_event()
Quoted 8 lines
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.
Quoted 13 lines
4150 e = kzalloc(total_len, GFP_ATOMIC);
drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.cbrcmf_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);
Quoted 3 lines
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
Quoted 3 lines · Dan Carpenter
The multiplication here looks like it could overflow. I've changed it to use
kcalloc()to prevent that.
Quoted 4 lines
Signed-off-by Dan Carpenter <[email protected]>---
Only needed in linux-next. This was added ine58060723c"brcmfmac:
introduce scheduled scan support".
Better be safe than sorry. Thanks.
Gr. AvS
Quoted 1 line
drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c +0
-0