re: mwifiex: parse TDLS action frames during RX

* re: mwifiex: parse TDLS action frames during RX
@ 2014-02-14  9:02 Dan Carpenter
  2014-02-14  9:57 ` Avinash Patil
  0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2014-02-14  9:02 UTC (permalink / raw)
  To: patila; +Cc: linux-wireless, Bing Zhao

Hello Avinash Patil,

The patch 5f2caaf32bc6: "mwifiex: parse TDLS action frames during RX"
from Feb 7, 2014, leads to the following static checker warning:

	drivers/net/wireless/mwifiex/tdls.c:820 mwifiex_process_tdls_action_frame()
	error: memcpy() '&sta_ptr->tdls_cap.rsn_ie' too small (256 vs 257)

drivers/net/wireless/mwifiex/tdls.c
   814                  case WLAN_EID_EXT_CAPABILITY:
   815                          memcpy((u8 *)&sta_ptr->tdls_cap.extcap, pos,
   816                                 sizeof(struct ieee_types_header) +
   817                                 min_t(u8, pos[1], 8));
   818                          break;
   819                  case WLAN_EID_RSN:
   820                          memcpy((u8 *)&sta_ptr->tdls_cap.rsn_ie, pos,
   821                                 sizeof(struct ieee_types_header) + pos[1]);

Smatch thinks pos[] is untrusted data because it comes from skb->data
in mwifiex_process_rx_packet().

sta_ptr->tdls_cap.rsn_ie is defined like:

struct ieee_types_generic {
        struct ieee_types_header ieee_hdr;
        u8 data[IEEE_MAX_IE_SIZE - sizeof(struct ieee_types_header)];
} __packed;

So it is IEEE_MAX_IE_SIZE (256) bytes long.  Meanwhile the memcpy()
limit is 2 + 0xff, so it's 257 and we are corrupting a byte past the end
of the struct.

   822                          break;
   823                  case WLAN_EID_QOS_CAPA:
   824                          sta_ptr->tdls_cap.qos_info = pos[2];
   825                          break;

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 5+ messages in thread