staging: r8723au: Add source files for new driver - part 1

Dan Carpenter dan.carpenter at oracle.com
Wed Apr 9 15:20:46 UTC 2014


Hi Larry and Jes,

The patch 5e93f3520957: "staging: r8723au: Add source files for new
driver - part 1" from Mar 28, 2014, leads to the following static
checker warning:

	drivers/staging/rtl8723au/core/rtw_ieee80211.c:1515 rtw_get_wfd_ie()
	error: memcpy() 'wfd_ie' too small (128 vs 257)

drivers/staging/rtl8723au/core/rtw_ieee80211.c
  1495  #ifdef CONFIG_8723AU_P2P
  1496  int rtw_get_wfd_ie(u8 *in_ie, int in_len, u8 *wfd_ie, uint *wfd_ielen)
  1497  {
  1498          int match;
  1499          uint cnt = 0;
  1500          u8 eid, wfd_oui[4] = {0x50, 0x6F, 0x9A, 0x0A};
  1501  
  1502          match = false;
  1503  
  1504          if (in_len < 0) {
  1505                  return match;
  1506          }
  1507  
  1508          while (cnt < in_len)
  1509          {
  1510                  eid = in_ie[cnt];
  1511  
  1512                  if ((eid == _VENDOR_SPECIFIC_IE_) &&
  1513                      !memcmp(&in_ie[cnt+2], wfd_oui, 4)) {
  1514                          if (wfd_ie != NULL) {
  1515                                  memcpy(wfd_ie, &in_ie[cnt], in_ie[cnt + 1] + 2);
                                               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

The concern here is when this is called from OnAssocReq23a().
wfd_ie[] is a 128 char buffer.
in_ie comes from skb->data so it's not trusted.  It can go up to 255 and
the "+ 2" makes 257.

You would need to have smatch cross function database configured to get
this warning in Smatch.

  1516  
  1517                          } else {
  1518                                  if (wfd_ielen != NULL) {
  1519                                          *wfd_ielen = 0;
  1520                                  }
  1521                          }
  1522  
  1523                          if (wfd_ielen != NULL) {
  1524                                  *wfd_ielen = in_ie[cnt + 1] + 2;
  1525                          }
  1526  
  1527                          cnt += in_ie[cnt + 1] + 2;
  1528  
  1529                          match = true;
  1530                          break;
  1531                  } else {
  1532                          cnt += in_ie[cnt + 1] +2; /* goto next */
  1533                  }
  1534          }

regards,
dan carpenter


More information about the devel mailing list