Skip to contentSkip to conversation

brcmfmac: use kcalloc() to prevent integer overflow

PATCH In review Dan Carpenter 2 participants 4 messages Sep 26, 2012 – Sep 27, 2012

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

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
index af396e4..7983be1 100644
@@ -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;
#1 lore raw

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

#2 lore raw
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.c brcmf_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.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);
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

Acked-by on v1 1/1
#3 lore raw
In reply to Dan Carpenter
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

---
Only needed in linux-next. This was added in e58060723c "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