[report] staging: r8723au: rtw_report_sec_ie23a() is buggy

[Dan Carpenter]

Hello Larry, Jes,

The rtw_report_sec_ie23a() is very buggy.

1) It uses GFP_KERNEL but the callers are holding a spinlock.

	rtw_select_and_join_from_scanned_queue23a() <- takes lock
	-> rtw_joinbss_cmd23a()
           -> rtw_restruct_sec_ie23a()
              -> rtw_report_sec_ie23a()

2) The sprintf() can overflow because we're putting over 512 characters
   into a IW_CUSTOM_MAX (256) character buffer.

3) It could actually be far worse than 512.  It could be a forever
   loop!  :P  The "i" variable is declared as u8 so it will always be
   less than IW_CUSTOM_MAX (256).

4) What is the point of this function?  It doesn't seem to store "buff"
   anywhere or do anything with "wrqu".

I have included my suggestion for how to start fixing the function but
it's not a complete solution because I don't know the answer to #4.

regards,
dan carpenter

diff --git a/drivers/staging/rtl8723au/os_dep/mlme_linux.c b/drivers/staging/rtl8723au/os_dep/mlme_linux.c
index b30d4d3..6927227 100644
--- a/drivers/staging/rtl8723au/os_dep/mlme_linux.c
+++ b/drivers/staging/rtl8723au/os_dep/mlme_linux.c
@@ -102,42 +102,30 @@ void rtw_os_indicate_disconnect23a(struct rtw_adapter *adapter)
 
 void rtw_report_sec_ie23a(struct rtw_adapter *adapter, u8 authmode, u8 *sec_ie)
 {
-	uint	len;
-	u8	*buff, *p, i;
+	char buf[IW_CUSTOM_MAX];
+	int len;
+	int i;
+	int printed = 0;
 	union iwreq_data wrqu;
 
 	RT_TRACE(_module_mlme_osdep_c_, _drv_info_,
 		 ("+rtw_report_sec_ie23a, authmode =%d\n", authmode));
 
-	buff = NULL;
-	if (authmode == _WPA_IE_ID_) {
-		RT_TRACE(_module_mlme_osdep_c_, _drv_info_,
-			 ("rtw_report_sec_ie23a, authmode =%d\n", authmode));
-
-		buff = kzalloc(IW_CUSTOM_MAX, GFP_KERNEL);
-		if (!buff)
-			return;
-		p = buff;
-
-		p += sprintf(p, "ASSOCINFO(ReqIEs =");
-
-		len = sec_ie[1]+2;
-		len =  (len < IW_CUSTOM_MAX) ? len : IW_CUSTOM_MAX;
-
-		for (i = 0; i < len; i++)
-			p += sprintf(p, "%02x", sec_ie[i]);
-
-		p += sprintf(p, ")");
-
-		memset(&wrqu, 0, sizeof(wrqu));
-
-		wrqu.data.length = p-buff;
-
-		wrqu.data.length = (wrqu.data.length < IW_CUSTOM_MAX) ?
-				   wrqu.data.length : IW_CUSTOM_MAX;
+	if (authmode != _WPA_IE_ID_)
+		return;
 
-		kfree(buff);
+	printed += sprintf(p, "ASSOCINFO(ReqIEs =");
+	len = min(sec_ie[1] + 2, IW_CUSTOM_MAX);
+	for (i = 0; i < len; i++) {
+		printed += scnprintf(buf + printed, sizeof(buf) - printed,
+				     "%02x", sec_ie[i]);
 	}
+	printed += scnprintf(buf + printed, sizeof(buf) - printed, ")");
+
+	memset(&wrqu, 0, sizeof(wrqu));
+	/* FIXME: store the buf somewhere */
+	wrqu.data.length = printed;
+	/* FIXME: do something with wrqu */
 }
 
 #ifdef CONFIG_8723AU_AP_MODE