* [patch] brcmfmac: use kcalloc() to prevent integer overflow
@ 2012-09-26 7:21 Dan Carpenter
2012-09-26 7:31 ` Dan Carpenter
2012-09-27 10:49 ` Arend van Spriel
0 siblings, 2 replies; 4+ messages in thread
From: Dan Carpenter @ 2012-09-26 7:21 UTC (permalink / raw)
To: Brett Rudley
Cc: Roland Vossen, Arend van Spriel, Franky (Zhenhui) Lin, Kan Yan,
John W. Linville, Hante Meuleman, linux-wireless,
brcm80211-dev-list, kernel-janitors
The multiplication here looks like it could overflow. I've changed it
to use kcalloc() to prevent that.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Only needed in linux-next. This was added in e58060723c "brcmfmac:
introduce scheduled scan support".
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
index af396e4..7983be1 100644
--- a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
+++ b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
@@ -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;
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [patch] brcmfmac: use kcalloc() to prevent integer overflow
2012-09-26 7:21 [patch] brcmfmac: use kcalloc() to prevent integer overflow Dan Carpenter
@ 2012-09-26 7:31 ` Dan Carpenter
2012-09-26 9:41 ` Arend van Spriel
2012-09-27 10:49 ` Arend van Spriel
1 sibling, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2012-09-26 7:31 UTC (permalink / raw)
To: Brett Rudley
Cc: Roland Vossen, Arend van Spriel, Franky (Zhenhui) Lin, Kan Yan,
John W. Linville, Hante Meuleman, linux-wireless,
brcm80211-dev-list, kernel-janitors
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [patch] brcmfmac: use kcalloc() to prevent integer overflow
2012-09-26 7:31 ` Dan Carpenter
@ 2012-09-26 9:41 ` Arend van Spriel
0 siblings, 0 replies; 4+ messages in thread
From: Arend van Spriel @ 2012-09-26 9:41 UTC (permalink / raw)
To: Dan Carpenter
Cc: Brett Rudley, Roland Vossen, Franky (Zhenhui) Lin, Kan Yan,
John W. Linville, Hante Meuleman, linux-wireless,
brcm80211-dev-list, kernel-janitors
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [patch] brcmfmac: use kcalloc() to prevent integer overflow
2012-09-26 7:21 [patch] brcmfmac: use kcalloc() to prevent integer overflow Dan Carpenter
2012-09-26 7:31 ` Dan Carpenter
@ 2012-09-27 10:49 ` Arend van Spriel
1 sibling, 0 replies; 4+ messages in thread
From: Arend van Spriel @ 2012-09-27 10:49 UTC (permalink / raw)
To: Dan Carpenter
Cc: Brett Rudley, Roland Vossen, Franky (Zhenhui) Lin, Kan Yan,
John W. Linville, Hante Meuleman, linux-wireless,
brcm80211-dev-list, kernel-janitors
On 09/26/2012 09:21 AM, Dan Carpenter wrote:
> The multiplication here looks like it could overflow. I've changed it
> to use kcalloc() to prevent that.
>
Acked-by: Arend van Spriel <arend@broadcom.com>
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Only needed in linux-next. This was added in e58060723c "brcmfmac:
> introduce scheduled scan support".
Better be safe than sorry. Thanks.
Gr. AvS
> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c b/drivers/net/wireless/brcm80211/brcmfmac/wl_cfg80211.c
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-09-27 10:49 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-26 7:21 [patch] brcmfmac: use kcalloc() to prevent integer overflow Dan Carpenter
2012-09-26 7:31 ` Dan Carpenter
2012-09-26 9:41 ` Arend van Spriel
2012-09-27 10:49 ` Arend van Spriel
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).