[PATCH v3 0/2] Fix some compile warnings in v5.18+

[PATCH v3 0/2] Fix some compile warnings in v5.18+

[Larry Finger]

Building driver r8188eu in staging with -warray-bounds exposes two places
where arrays are too small.

Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
---
v2 - get proper To and Cc
v3 - Use Dan Carpenter's suggestion for correct patch.

Larry Finger (2):
  staging: r8188eu: Fix undersized array in rtw_xmit.c
  staging: r8188eu: Fix warning of array overflow in ioctl_linux.c

 drivers/staging/r8188eu/core/rtw_xmit.c      | 17 ++++-------------
 drivers/staging/r8188eu/os_dep/ioctl_linux.c |  2 +-
 2 files changed, 5 insertions(+), 14 deletions(-)

-- 
2.36.1

[PATCH v3 1/2] staging: r8188eu: Fix undersized array in rtw_xmit.c

[Larry Finger]

Compiling with -warray-bounds yields the following warning:

drivers/staging/r8188eu/core/rtw_xmit.c: In function ‘rtw_alloc_hwxmits’:
drivers/staging/r8188eu/core/rtw_xmit.c:1493:24: warning: array subscript 4 is outside array bounds of ‘void[64]’ [-Warray-bounds]
 1493 |                 hwxmits[4] .sta_queue = &pxmitpriv->be_pending;
      |                 ~~~~~~~^~~
In file included from drivers/staging/r8188eu/core/../include/osdep_service.h:19,
                 from drivers/staging/r8188eu/core/rtw_xmit.c:6:
In function ‘kmalloc’,
    inlined from ‘kzalloc’ at ./include/linux/slab.h:733:9,
    inlined from ‘rtw_alloc_hwxmits’ at drivers/staging/r8188eu/core/rtw_xmit.c:1484:23:
./include/linux/slab.h:600:24: note: at offset 64 into object of size 64 allocated by ‘kmem_cache_alloc_trace’
  600 |                 return kmem_cache_alloc_trace(
      |                        ^~~~~~~~~~~~~~~~~~~~~~~
  601 |                                 kmalloc_caches[kmalloc_type(flags)][index],
      |                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  602 |                                 flags, size);
      |

This warning arises because the test for (pxmitpriv->hwxmit_entry == 5) is totally
bogus. There are only 4 queues available. Thanks to Dan Carpenter for suggesting the
correct patch. Thanks to Dan Carperter for suggesting the correct fix.

Fixes commit 7884fc0a1473 ("staging: r8188eu: introduce new include dir
for RTL8188eu driver")

Fixes: 7884fc0a1473 ("staging: r8188eu: introduce new include dir for RTL8188eu driver")
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Phillip Potter <phil@philpotter.co.uk>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
--
v2 - get proper To and Cc
v3 - Use Dan Carpenter's suggestion for correct fix
---
 drivers/staging/r8188eu/core/rtw_xmit.c | 17 ++++-------------
 1 file changed, 4 insertions(+), 13 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_xmit.c b/drivers/staging/r8188eu/core/rtw_xmit.c
index 3d8e9dea7651..066308150b42 100644
--- a/drivers/staging/r8188eu/core/rtw_xmit.c
+++ b/drivers/staging/r8188eu/core/rtw_xmit.c
@@ -1483,19 +1483,10 @@ int rtw_alloc_hwxmits(struct adapter *padapter)
 
 	hwxmits = pxmitpriv->hwxmits;
 
-	if (pxmitpriv->hwxmit_entry == 5) {
-		hwxmits[0] .sta_queue = &pxmitpriv->bm_pending;
-		hwxmits[1] .sta_queue = &pxmitpriv->vo_pending;
-		hwxmits[2] .sta_queue = &pxmitpriv->vi_pending;
-		hwxmits[3] .sta_queue = &pxmitpriv->bk_pending;
-		hwxmits[4] .sta_queue = &pxmitpriv->be_pending;
-	} else if (pxmitpriv->hwxmit_entry == 4) {
-		hwxmits[0] .sta_queue = &pxmitpriv->vo_pending;
-		hwxmits[1] .sta_queue = &pxmitpriv->vi_pending;
-		hwxmits[2] .sta_queue = &pxmitpriv->be_pending;
-		hwxmits[3] .sta_queue = &pxmitpriv->bk_pending;
-	} else {
-	}
+	hwxmits[0].sta_queue = &pxmitpriv->vo_pending;
+	hwxmits[1].sta_queue = &pxmitpriv->vi_pending;
+	hwxmits[2].sta_queue = &pxmitpriv->be_pending;
+	hwxmits[3].sta_queue = &pxmitpriv->bk_pending;
 
 	return 0;
 }
-- 
2.36.1

[PATCH v3 2/2] staging: r8188eu: Fix warning of array overflow in ioctl_linux.c

[Larry Finger]

Building with -Warray-bounds results in the following warning plus others
related to the same problem:

CC [M]  drivers/staging/r8188eu/os_dep/ioctl_linux.o
In function ‘wpa_set_encryption’,
    inlined from ‘rtw_wx_set_enc_ext’ at drivers/staging/r8188eu/os_dep/ioctl_linux.c:1868:9:
drivers/staging/r8188eu/os_dep/ioctl_linux.c:412:41: warning: array subscript ‘struct ndis_802_11_wep[0]’ is partly outside array bounds of ‘void[25]’ [-Warray-bounds]
  412 |                         pwep->KeyLength = wep_key_len;
      |                         ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
In file included from drivers/staging/r8188eu/os_dep/../include/osdep_service.h:19,
                 from drivers/staging/r8188eu/os_dep/ioctl_linux.c:4:
In function ‘kmalloc’,
    inlined from ‘kzalloc’ at ./include/linux/slab.h:733:9,
    inlined from ‘wpa_set_encryption’ at drivers/staging/r8188eu/os_dep/ioctl_linux.c:408:11,
    inlined from ‘rtw_wx_set_enc_ext’ at drivers/staging/r8188eu/os_dep/ioctl_linux.c:1868:9:
./include/linux/slab.h:605:16: note: object of size [17, 25] allocated by ‘__kmalloc’
  605 |         return __kmalloc(size, flags);
      |                ^~~~~~~~~~~~~~~~~~~~~~
./include/linux/slab.h:600:24: note: object of size [17, 25] allocated by ‘kmem_cache_alloc_trace’
  600 |                 return kmem_cache_alloc_trace(
      |                        ^~~~~~~~~~~~~~~~~~~~~~~
  601 |                                 kmalloc_caches[kmalloc_type(flags)][index],
      |                                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  602 |                                 flags, size);
      |                                 ~~~~~~~~~~~~

Although it is unlikely that anyone is still using WEP encryption, the
size of the allocation needs to be increased just in case.

Fixes commit 2b42bd58b321 ("staging: r8188eu: introduce new os_dep dir for RTL8188eu driver")

Fixes: 2b42bd58b321 ("staging: r8188eu: introduce new os_dep dir for RTL8188eu driver")
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Phillip Potter <phil@philpotter.co.uk>
Cc: Dan Carpenter <dan.carpenter@oracle.com>
---
v2 - get proper To and Cc
v3 - no changes
---
 drivers/staging/r8188eu/os_dep/ioctl_linux.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/r8188eu/os_dep/ioctl_linux.c b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
index 1b09462ca908..8dd280e2739a 100644
--- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c
+++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c
@@ -403,7 +403,7 @@ static int wpa_set_encryption(struct net_device *dev, struct ieee_param *param,
 
 		if (wep_key_len > 0) {
 			wep_key_len = wep_key_len <= 5 ? 5 : 13;
-			wep_total_len = wep_key_len + FIELD_OFFSET(struct ndis_802_11_wep, KeyMaterial);
+			wep_total_len = wep_key_len + sizeof(*pwep);
 			pwep = kzalloc(wep_total_len, GFP_KERNEL);
 			if (!pwep)
 				goto exit;
-- 
2.36.1

Re: [PATCH v3 0/2] Fix some compile warnings in v5.18+

[Phillip Potter]

On Mon, May 30, 2022 at 08:31:01PM -0500, Larry Finger wrote:
> Building driver r8188eu in staging with -warray-bounds exposes two places
> where arrays are too small.
> 
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
> v2 - get proper To and Cc
> v3 - Use Dan Carpenter's suggestion for correct patch.
> 
> Larry Finger (2):
>   staging: r8188eu: Fix undersized array in rtw_xmit.c
>   staging: r8188eu: Fix warning of array overflow in ioctl_linux.c
> 
>  drivers/staging/r8188eu/core/rtw_xmit.c      | 17 ++++-------------
>  drivers/staging/r8188eu/os_dep/ioctl_linux.c |  2 +-
>  2 files changed, 5 insertions(+), 14 deletions(-)
> 
> -- 
> 2.36.1
> 

Hi Larry,

Both patches look good to me, however I've already submitted a patch 10
days ago that does exactly what your rtw_xmit.c patch does, in
addition to fixing the changed error handling semantics for the relevant
function (in f94b47c6bde6) which broke the driver.

Regards,
Phil

Re: [PATCH v3 1/2] staging: r8188eu: Fix undersized array in rtw_xmit.c

[Dan Carpenter]

On Mon, May 30, 2022 at 08:31:02PM -0500, Larry Finger wrote:
> --
> v2 - get proper To and Cc
> v3 - Use Dan Carpenter's suggestion for correct fix

Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>

regards,
dan carpenter

Re: [PATCH v3 0/2] Fix some compile warnings in v5.18+

[Larry Finger]

On 5/31/22 01:40, Phillip Potter wrote:
> 
> Hi Larry,
> 
> Both patches look good to me, however I've already submitted a patch 10
> days ago that does exactly what your rtw_xmit.c patch does, in
> addition to fixing the changed error handling semantics for the relevant
> function (in f94b47c6bde6) which broke the driver.

Phil,

Sorry I missed your patch. I have been really busy the past month on another 
problem, and I was not paying much attention to r8188eu.

When I built a mainline kernel in the middle of the merge from 5.18 to 5.19, 
those two warnings stood out.

Your patch should be ahead of mine in Greg's queue, thus mine should fail, and I 
will get the "does not apply" message. At that time, I will send the other 
patch. As both of us missed the 5.19 merge, the build warnings will persist 
until 5.20, but no harm.

Larry

Re: [PATCH v3 0/2] Fix some compile warnings in v5.18+

[Phillip Potter]

On Tue, May 31, 2022 at 12:16:58PM -0500, Larry Finger wrote:
> On 5/31/22 01:40, Phillip Potter wrote:
> > 
> > Hi Larry,
> > 
> > Both patches look good to me, however I've already submitted a patch 10
> > days ago that does exactly what your rtw_xmit.c patch does, in
> > addition to fixing the changed error handling semantics for the relevant
> > function (in f94b47c6bde6) which broke the driver.
> 
> Phil,
> 
> Sorry I missed your patch. I have been really busy the past month on another
> problem, and I was not paying much attention to r8188eu.
> 
> When I built a mainline kernel in the middle of the merge from 5.18 to 5.19,
> those two warnings stood out.
> 
> Your patch should be ahead of mine in Greg's queue, thus mine should fail,
> and I will get the "does not apply" message. At that time, I will send the
> other patch. As both of us missed the 5.19 merge, the build warnings will
> persist until 5.20, but no harm.
> 
> Larry
> 

Please don't be sorry - just wanted you to know in advance :-).

Figured if I said nothing I would probably aggravate the situation. I
don't have much time to spend on r8188eu myself either so don't worry.

All the best,
Phil

Re: [PATCH v3 0/2] Fix some compile warnings in v5.18+

[Greg KH]

On Tue, May 31, 2022 at 12:16:58PM -0500, Larry Finger wrote:
> On 5/31/22 01:40, Phillip Potter wrote:
> > 
> > Hi Larry,
> > 
> > Both patches look good to me, however I've already submitted a patch 10
> > days ago that does exactly what your rtw_xmit.c patch does, in
> > addition to fixing the changed error handling semantics for the relevant
> > function (in f94b47c6bde6) which broke the driver.
> 
> Phil,
> 
> Sorry I missed your patch. I have been really busy the past month on another
> problem, and I was not paying much attention to r8188eu.
> 
> When I built a mainline kernel in the middle of the merge from 5.18 to 5.19,
> those two warnings stood out.
> 
> Your patch should be ahead of mine in Greg's queue, thus mine should fail,
> and I will get the "does not apply" message. At that time, I will send the
> other patch. As both of us missed the 5.19 merge, the build warnings will
> persist until 5.20, but no harm.

I've taken Phillip's patch, and your second patch here, both for
5.19-final, so all should be good now.

thanks,

greg k-h