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
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Reply via email to