netdev - RE: wireless: mwifiex: initial commit for Marvell mwifiex driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 25 Apr 2012 15:08:33 -0700
From:	Bing Zhao <bzhao@...vell.com>
To:	Dan Carpenter <dan.carpenter@...cle.com>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: wireless: mwifiex: initial commit for Marvell mwifiex driver

Hi Dan,

> Hi Bing,
> 
> The patch 5e6e3a92b9a4: "wireless: mwifiex: initial commit for
> Marvell mwifiex driver" from Mar 21, 2011, leads to the following
> static checker warning:
> 
> drivers/net/wireless/mwifiex/sta_ioctl.c:1410
> mwifiex_set_gen_ie_helper()
> 	 error: memcmp() 'pvendor_ie->oui' too small (3 vs 4)

Thanks for catching this error.

> 
>   1390  mwifiex_set_gen_ie_helper(struct mwifiex_private *priv, u8 *ie_data_ptr,
>   1391                            u16 ie_len)
>   1392  {
>   1393          int ret = 0;
>   1394          struct ieee_types_vendor_header *pvendor_ie;
>   1395          const u8 wpa_oui[] = { 0x00, 0x50, 0xf2, 0x01 };
>                                      ^^^^^^^^^^^^^^^^^^^^^^^^^^
> 4 byte array.

The last byte 0x01 is actually the oui_type.

	...
	u8 oui[3];
	u8 oui_type;
	...

> 
>   1396          const u8 wps_oui[] = { 0x00, 0x50, 0xf2, 0x04 };
>   1397
>   1398          /* If the passed length is zero, reset the buffer */
>   1399          if (!ie_len) {
>   1400                  priv->gen_ie_buf_len = 0;
>   1401                  priv->wps.session_enable = false;
>   1402
>   1403                  return 0;
>   1404          } else if (!ie_data_ptr) {
>   1405                  return -1;
>   1406          }
>   1407          pvendor_ie = (struct ieee_types_vendor_header *) ie_data_ptr;
>   1408          /* Test to see if it is a WPA IE, if not, then it is a gen IE */
>   1409          if (((pvendor_ie->element_id == WLAN_EID_WPA) &&
>   1410               (!memcmp(pvendor_ie->oui, wpa_oui, sizeof(wpa_oui)))) ||
>                               ^^^^^^^^^^^^^^^
> ->oui is only a 3 byte array so we're reading past the end for this
> comparison.

I will fix the misuse of memcmp's.

By the way, could you please share with me how you check for this kind of errors?
Is it coccinelle? I tried with coccicheck but got "spatch.opt unknown option -D" error.

Thanks,
Bing

> 
>   1411              (pvendor_ie->element_id == WLAN_EID_RSN)) {
> 
> 
> There are a couple other similar warnings as well:
> 
> drivers/net/wireless/mwifiex/sta_ioctl.c:1435 mwifiex_set_gen_ie_helper()
> 	error: memcmp() 'pvendor_ie->oui' too small (3 vs 4)
> drivers/net/wireless/mwifiex/scan.c:1177 mwifiex_update_bss_desc_with_ie()
> 	error: memcmp() 'vendor_ie->vend_hdr.oui' too small (3 vs 4)
> drivers/net/wireless/mwifiex/scan.c:1185 mwifiex_update_bss_desc_with_ie()
> 	error: memcmp() 'vendor_ie->vend_hdr.oui' too small (3 vs 4)
> 
> regards,
> dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html