2022-05-29 00:48:43

by Larry Finger

[permalink] [raw]
Subject: [PATCH v2 2/2] staging: r8188eu: Fix undersized array in rtw_xmit.c

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 macro HWXMIT_ENTRY is too small.

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 <[email protected]>
Cc: Phillip Potter <[email protected]>
---
v2 = Get To and Cc right
---
drivers/staging/r8188eu/include/rtw_xmit.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/r8188eu/include/rtw_xmit.h b/drivers/staging/r8188eu/include/rtw_xmit.h
index b2df1480d66b..27fa536f51ee 100644
--- a/drivers/staging/r8188eu/include/rtw_xmit.h
+++ b/drivers/staging/r8188eu/include/rtw_xmit.h
@@ -69,7 +69,7 @@ do { \
dot11txpn.val = dot11txpn.val == 0xffffffffffffULL ? 0 : (dot11txpn.val+1);\
} while (0)

-#define HWXMIT_ENTRY 4
+#define HWXMIT_ENTRY 5

#define TXDESC_SIZE 32

--
2.36.1



2022-06-01 13:58:51

by Dan Carpenter

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] staging: r8188eu: Fix undersized array in rtw_xmit.c

On Sat, May 28, 2022 at 07:47:11PM -0500, Larry Finger wrote:
> 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;
> | ~~~~~~~^~~

This patch is not correct. The hwxmits[4] is nonsense dead code. It
has an extra element and the other four element are in the wrong order.
If we applied this patch then we would have to update rtw_get_sta_pending()
to re-order the *(ac) = 2; assignments to match the format. And to set
*ac = 4 on one path.

The correct fix is to just delete the dead code.

regards,
dan carpenter

diff --git a/drivers/staging/r8188eu/core/rtw_xmit.c b/drivers/staging/r8188eu/core/rtw_xmit.c
index 3d8e9dea7651..2a93e7fa1142 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;
}