* re: mwifiex: add AES_CMAC support in key_material cmd
@ 2012-08-08 14:12 Dan Carpenter
2012-08-09 1:45 ` Bing Zhao
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2012-08-08 14:12 UTC (permalink / raw)
To: luoy; +Cc: linux-wireless
Hello Ying Luo,
The patch b877f4cf5af8: "mwifiex: add AES_CMAC support in
key_material cmd" from Aug 3, 2012, leads to the following warning:
drivers/net/wireless/mwifiex/sta_cmd.c:692
mwifiex_cmd_802_11_key_material()
error: memcpy() 'param->key' too small (16 vs 32)
656 } else if (enc_key->key_len == WLAN_KEY_LEN_TKIP) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
We assume a 32 byte key_len here.
657 dev_dbg(priv->adapter->dev, "cmd: WPA_TKIP\n");
658 key_material->key_param_set.key_type_id =
659 cpu_to_le16(KEY_TYPE_ID_TKIP);
660 key_material->key_param_set.key_info =
661 cpu_to_le16(KEY_ENABLED);
[snip]
686 if (le16_to_cpu(key_material->key_param_set.key_type_id) ==
687 KEY_TYPE_ID_AES_CMAC) {
688 struct mwifiex_cmac_param *param =
689 (void *)key_material->key_param_set.key;
690
691 memcpy(param->ipn, enc_key->pn, IGTK_PN_LEN);
692 memcpy(param->key, enc_key->key_material,
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
693 enc_key->key_len);
^^^^^^^^^^^^^^^^
"param->key" only has space for 16 bytes.
694
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread* RE: mwifiex: add AES_CMAC support in key_material cmd 2012-08-08 14:12 mwifiex: add AES_CMAC support in key_material cmd Dan Carpenter @ 2012-08-09 1:45 ` Bing Zhao 2012-08-09 7:09 ` Dan Carpenter 0 siblings, 1 reply; 3+ messages in thread From: Bing Zhao @ 2012-08-09 1:45 UTC (permalink / raw) To: Dan Carpenter, Ying Luo; +Cc: linux-wireless@vger.kernel.org Hi Dan, Thanks for reporting this warning. > The patch b877f4cf5af8: "mwifiex: add AES_CMAC support in > key_material cmd" from Aug 3, 2012, leads to the following warning: > drivers/net/wireless/mwifiex/sta_cmd.c:692 > mwifiex_cmd_802_11_key_material() > error: memcpy() 'param->key' too small (16 vs 32) I pulled latest 'smatch' code (top commit 130ba3ad) but it cannot detect this error though. > > 656 } else if (enc_key->key_len == WLAN_KEY_LEN_TKIP) { > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > We assume a 32 byte key_len here. For TKIP, yes, key_len has been set to 32. > > 657 dev_dbg(priv->adapter->dev, "cmd: WPA_TKIP\n"); > 658 key_material->key_param_set.key_type_id = > 659 cpu_to_le16(KEY_TYPE_ID_TKIP); > 660 key_material->key_param_set.key_info = > 661 cpu_to_le16(KEY_ENABLED); > > [snip] > > 686 if (le16_to_cpu(key_material->key_param_set.key_type_id) == > 687 KEY_TYPE_ID_AES_CMAC) { > 688 struct mwifiex_cmac_param *param = > 689 (void *)key_material->key_param_set.key; > 690 > 691 memcpy(param->ipn, enc_key->pn, IGTK_PN_LEN); > 692 memcpy(param->key, enc_key->key_material, > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > 693 enc_key->key_len); > ^^^^^^^^^^^^^^^^ > "param->key" only has space for 16 bytes. For AES_CMAC, here key_len should have been set to 16, but compiler doesn't know that. The following change should fix the warning. (I will send a formal patch shortly.) diff --git a/drivers/net/wireless/mwifiex/sta_cmd.c b/drivers/net/wireless/mwifiex/ --- a/drivers/net/wireless/mwifiex/sta_cmd.c +++ b/drivers/net/wireless/mwifiex/sta_cmd.c @@ -690,7 +690,7 @@ mwifiex_cmd_802_11_key_material(struct mwifiex_private *priv, memcpy(param->ipn, enc_key->pn, IGTK_PN_LEN); memcpy(param->key, enc_key->key_material, - enc_key->key_len); + WLAN_KEY_LEN_AES_CMAC); key_param_len = sizeof(struct mwifiex_cmac_param); key_material->key_param_set.key_len = Thanks, Bing ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: mwifiex: add AES_CMAC support in key_material cmd 2012-08-09 1:45 ` Bing Zhao @ 2012-08-09 7:09 ` Dan Carpenter 0 siblings, 0 replies; 3+ messages in thread From: Dan Carpenter @ 2012-08-09 7:09 UTC (permalink / raw) To: Bing Zhao; +Cc: Ying Luo, linux-wireless@vger.kernel.org On Wed, Aug 08, 2012 at 06:45:36PM -0700, Bing Zhao wrote: > Hi Dan, > > Thanks for reporting this warning. > > > The patch b877f4cf5af8: "mwifiex: add AES_CMAC support in > > key_material cmd" from Aug 3, 2012, leads to the following warning: > > drivers/net/wireless/mwifiex/sta_cmd.c:692 > > mwifiex_cmd_802_11_key_material() > > error: memcpy() 'param->key' too small (16 vs 32) > > I pulled latest 'smatch' code (top commit 130ba3ad) but it cannot detect this error though. > Hm. It should. I'm not sure what the problem is. > > > > 656 } else if (enc_key->key_len == WLAN_KEY_LEN_TKIP) { > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > > > We assume a 32 byte key_len here. > > For TKIP, yes, key_len has been set to 32. > > > > > 657 dev_dbg(priv->adapter->dev, "cmd: WPA_TKIP\n"); > > 658 key_material->key_param_set.key_type_id = > > 659 cpu_to_le16(KEY_TYPE_ID_TKIP); > > 660 key_material->key_param_set.key_info = > > 661 cpu_to_le16(KEY_ENABLED); > > > > [snip] > > > > 686 if (le16_to_cpu(key_material->key_param_set.key_type_id) == > > 687 KEY_TYPE_ID_AES_CMAC) { > > 688 struct mwifiex_cmac_param *param = > > 689 (void *)key_material->key_param_set.key; > > 690 > > 691 memcpy(param->ipn, enc_key->pn, IGTK_PN_LEN); > > 692 memcpy(param->key, enc_key->key_material, > > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > > 693 enc_key->key_len); > > ^^^^^^^^^^^^^^^^ > > "param->key" only has space for 16 bytes. > > For AES_CMAC, here key_len should have been set to 16, but compiler doesn't know that. > > The following change should fix the warning. (I will send a formal patch shortly.) > It does. Normally I say that it's not worth silencing false positives, but this change doesn't make the code less readable so thanks for that. regards, dan carpenter > diff --git a/drivers/net/wireless/mwifiex/sta_cmd.c b/drivers/net/wireless/mwifiex/ > --- a/drivers/net/wireless/mwifiex/sta_cmd.c > +++ b/drivers/net/wireless/mwifiex/sta_cmd.c > @@ -690,7 +690,7 @@ mwifiex_cmd_802_11_key_material(struct mwifiex_private *priv, > > memcpy(param->ipn, enc_key->pn, IGTK_PN_LEN); > memcpy(param->key, enc_key->key_material, > - enc_key->key_len); > + WLAN_KEY_LEN_AES_CMAC); > > key_param_len = sizeof(struct mwifiex_cmac_param); > key_material->key_param_set.key_len = > > Thanks, > Bing ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-08-09 7:09 UTC | newest] Thread overview: 3+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-08-08 14:12 mwifiex: add AES_CMAC support in key_material cmd Dan Carpenter 2012-08-09 1:45 ` Bing Zhao 2012-08-09 7:09 ` Dan Carpenter
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).