Re: [PATCH 37/57] staging: rtl8188eu: os_dep: ioctl_linux: Move 2 large data buffers into the heap

2021-04-19 Thread Arnd Bergmann 
On Thu, Apr 15, 2021 at 7:29 AM Dan Carpenter  wrote:
>
> On Thu, Apr 15, 2021 at 08:20:16AM +0300, Dan Carpenter wrote:
> > On Wed, Apr 14, 2021 at 07:11:09PM +0100, Lee Jones wrote:
> > > ---
> > >  drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 12 +++-
> > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c 
> > > b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > > index c95ae4d6a3b6b..cc14f00947781 100644
> > > --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > > +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > > @@ -224,7 +224,7 @@ static char *translate_scan(struct adapter *padapter,
> > > /* parsing WPA/WPA2 IE */
> > > {
> > > u8 *buf;
> > > -   u8 wpa_ie[255], rsn_ie[255];
> > > +   u8 *wpa_ie, *rsn_ie;
> > > u16 wpa_len = 0, rsn_len = 0;
> > > u8 *p;
> > >
> > > @@ -232,6 +232,14 @@ static char *translate_scan(struct adapter *padapter,
> > > if (!buf)
> > > return start;
>
> Arnd, added this return...  I don't understand why we aren't returning
> -ENOMEM here.

I don't remember my patch, but I see that the function returns a pointer
that gets dereferenced afterwards. Changing this is probably a good idea
(the caller does return an error code), but it requires a few extra changes.

If there is a larger rework, I'd suggest using a single kzalloc to get all
three arrays at once to get simpler error handling.

   Arnd

Re: [PATCH 37/57] staging: rtl8188eu: os_dep: ioctl_linux: Move 2 large data buffers into the heap

2021-04-15 Thread Lee Jones 
On Thu, 15 Apr 2021, Dan Carpenter wrote:

> I screwed up my last email and dropped Lee and Arnd from the To: headers.
> Resending.
> 
> On Thu, Apr 15, 2021 at 08:20:16AM +0300, Dan Carpenter wrote:
> > On Wed, Apr 14, 2021 at 07:11:09PM +0100, Lee Jones wrote:
> > > ---
> > >  drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 12 +++-
> > >  1 file changed, 11 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c 
> > > b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > > index c95ae4d6a3b6b..cc14f00947781 100644
> > > --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > > +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > > @@ -224,7 +224,7 @@ static char *translate_scan(struct adapter *padapter,
> > >   /* parsing WPA/WPA2 IE */
> > >   {
> > >   u8 *buf;
> > > - u8 wpa_ie[255], rsn_ie[255];
> > > + u8 *wpa_ie, *rsn_ie;
> > >   u16 wpa_len = 0, rsn_len = 0;
> > >   u8 *p;
> > >  
> > > @@ -232,6 +232,14 @@ static char *translate_scan(struct adapter *padapter,
> > >   if (!buf)
> > >   return start;
> 
> Arnd added this return.  I think it should be -ENOMEM, though?

Will fix, thanks.

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 37/57] staging: rtl8188eu: os_dep: ioctl_linux: Move 2 large data buffers into the heap

2021-04-14 Thread Dan Carpenter 
I screwed up my last email and dropped Lee and Arnd from the To: headers.
Resending.

On Thu, Apr 15, 2021 at 08:20:16AM +0300, Dan Carpenter wrote:
> On Wed, Apr 14, 2021 at 07:11:09PM +0100, Lee Jones wrote:
> > ---
> >  drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 12 +++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c 
> > b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > index c95ae4d6a3b6b..cc14f00947781 100644
> > --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > @@ -224,7 +224,7 @@ static char *translate_scan(struct adapter *padapter,
> > /* parsing WPA/WPA2 IE */
> > {
> > u8 *buf;
> > -   u8 wpa_ie[255], rsn_ie[255];
> > +   u8 *wpa_ie, *rsn_ie;
> > u16 wpa_len = 0, rsn_len = 0;
> > u8 *p;
> >  
> > @@ -232,6 +232,14 @@ static char *translate_scan(struct adapter *padapter,
> > if (!buf)
> > return start;

Arnd added this return.  I think it should be -ENOMEM, though?

> >  
> > +   wpa_ie = kzalloc(255, GFP_ATOMIC);
> > +   if (!wpa_ie)
> > +   return start;
> 
> kfree(buf);
> 
> > +
> > +   rsn_ie = kzalloc(255, GFP_ATOMIC);
> > +   if (!rsn_ie)
> > +   return start;
> 

regards,
dan carpenter

Re: [PATCH 37/57] staging: rtl8188eu: os_dep: ioctl_linux: Move 2 large data buffers into the heap

2021-04-14 Thread Dan Carpenter 
On Thu, Apr 15, 2021 at 08:20:16AM +0300, Dan Carpenter wrote:
> On Wed, Apr 14, 2021 at 07:11:09PM +0100, Lee Jones wrote:
> > ---
> >  drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 12 +++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c 
> > b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > index c95ae4d6a3b6b..cc14f00947781 100644
> > --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> > @@ -224,7 +224,7 @@ static char *translate_scan(struct adapter *padapter,
> > /* parsing WPA/WPA2 IE */
> > {
> > u8 *buf;
> > -   u8 wpa_ie[255], rsn_ie[255];
> > +   u8 *wpa_ie, *rsn_ie;
> > u16 wpa_len = 0, rsn_len = 0;
> > u8 *p;
> >  
> > @@ -232,6 +232,14 @@ static char *translate_scan(struct adapter *padapter,
> > if (!buf)
> > return start;

Arnd, added this return...  I don't understand why we aren't returning
-ENOMEM here.

> >  
> > +   wpa_ie = kzalloc(255, GFP_ATOMIC);
> > +   if (!wpa_ie)
> > +   return start;
> 
> kfree(buf);
> 
> > +
> > +   rsn_ie = kzalloc(255, GFP_ATOMIC);
> > +   if (!rsn_ie)
> > +   return start;
> 

regards,
dan carpenter

Re: [PATCH 37/57] staging: rtl8188eu: os_dep: ioctl_linux: Move 2 large data buffers into the heap

2021-04-14 Thread Dan Carpenter 
On Wed, Apr 14, 2021 at 07:11:09PM +0100, Lee Jones wrote:
> ---
>  drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 12 +++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c 
> b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> index c95ae4d6a3b6b..cc14f00947781 100644
> --- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> +++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
> @@ -224,7 +224,7 @@ static char *translate_scan(struct adapter *padapter,
>   /* parsing WPA/WPA2 IE */
>   {
>   u8 *buf;
> - u8 wpa_ie[255], rsn_ie[255];
> + u8 *wpa_ie, *rsn_ie;
>   u16 wpa_len = 0, rsn_len = 0;
>   u8 *p;
>  
> @@ -232,6 +232,14 @@ static char *translate_scan(struct adapter *padapter,
>   if (!buf)
>   return start;
>  
> + wpa_ie = kzalloc(255, GFP_ATOMIC);
> + if (!wpa_ie)
> + return start;

kfree(buf);

> +
> + rsn_ie = kzalloc(255, GFP_ATOMIC);
> + if (!rsn_ie)
> + return start;

kfree(buf);
kfree(wpa_ie);

> +
>   rtw_get_sec_ie(pnetwork->network.ies, 
> pnetwork->network.ie_length, rsn_ie, _len, wpa_ie, _len);
>   RT_TRACE(_module_rtl871x_mlme_c_, _drv_info_, 
> ("rtw_wx_get_scan: ssid =%s\n", pnetwork->network.ssid.ssid));
>   RT_TRACE(_module_rtl871x_mlme_c_, _drv_info_, 
> ("rtw_wx_get_scan: wpa_len =%d rsn_len =%d\n", wpa_len, rsn_len));

regards,
dan carpenter

[PATCH 37/57] staging: rtl8188eu: os_dep: ioctl_linux: Move 2 large data buffers into the heap

2021-04-14 Thread Lee Jones 
Fixes the following W=1 kernel build warning(s):

 drivers/staging/rtl8188eu/os_dep/ioctl_linux.c: In function ‘translate_scan’:
 drivers/staging/rtl8188eu/os_dep/ioctl_linux.c:310:1: warning: the frame size 
of 1064 bytes is larger than 1024 bytes [-Wframe-larger-than=]
 drivers/staging/rtl8723bs/os_dep/ioctl_linux.c: In function ‘rtw_wx_set_mlme’:
 drivers/staging/rtl8723bs/os_dep/ioctl_linux.c:1128:6: warning: variable 
‘reason’ set but not used [-Wunused-but-set-variable]
 drivers/staging/rtl8723bs/os_dep/ioctl_linux.c: In function ‘rtw_dbg_port’:
 drivers/staging/rtl8723bs/os_dep/ioctl_linux.c:2548:33: warning: variable 
‘preorder_ctrl’ set but not used [-Wunused-but-set-variable]
 drivers/staging/rtl8723bs/os_dep/ioctl_linux.c:2573:33: warning: variable 
‘preorder_ctrl’ set but not used [-Wunused-but-set-variable]
 drivers/staging/rtl8723bs/os_dep/ioctl_linux.c:36:27: warning: 
‘iw_operation_mode’ defined but not used [-Wunused-const-variable=]

Cc: Larry Finger 
Cc: Greg Kroah-Hartman 
Cc: linux-stag...@lists.linux.dev
Signed-off-by: Lee Jones 
---
 drivers/staging/rtl8188eu/os_dep/ioctl_linux.c | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c 
b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
index c95ae4d6a3b6b..cc14f00947781 100644
--- a/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/rtl8188eu/os_dep/ioctl_linux.c
@@ -224,7 +224,7 @@ static char *translate_scan(struct adapter *padapter,
/* parsing WPA/WPA2 IE */
{
u8 *buf;
-   u8 wpa_ie[255], rsn_ie[255];
+   u8 *wpa_ie, *rsn_ie;
u16 wpa_len = 0, rsn_len = 0;
u8 *p;
 
@@ -232,6 +232,14 @@ static char *translate_scan(struct adapter *padapter,
if (!buf)
return start;
 
+   wpa_ie = kzalloc(255, GFP_ATOMIC);
+   if (!wpa_ie)
+   return start;
+
+   rsn_ie = kzalloc(255, GFP_ATOMIC);
+   if (!rsn_ie)
+   return start;
+
rtw_get_sec_ie(pnetwork->network.ies, 
pnetwork->network.ie_length, rsn_ie, _len, wpa_ie, _len);
RT_TRACE(_module_rtl871x_mlme_c_, _drv_info_, 
("rtw_wx_get_scan: ssid =%s\n", pnetwork->network.ssid.ssid));
RT_TRACE(_module_rtl871x_mlme_c_, _drv_info_, 
("rtw_wx_get_scan: wpa_len =%d rsn_len =%d\n", wpa_len, rsn_len));
@@ -268,6 +276,8 @@ static char *translate_scan(struct adapter *padapter,
start = iwe_stream_add_point(info, start, stop, , 
rsn_ie);
}
kfree(buf);
+   kfree(wpa_ie);
+   kfree(rsn_ie);
}
 
{/* parsing WPS IE */
-- 
2.27.0