* 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).