* [PATCH v2 0/2] Fix some compile warnings in v5.18+ @ 2022-05-29 0:47 Larry Finger 2022-05-29 0:47 ` [PATCH v2 1/2] staging: r8188eu: Fix warning of array overflow in ioctl_linux.c Larry Finger 2022-05-29 0:47 ` [PATCH v2 2/2] staging: r8188eu: Fix undersized array in rtw_xmit.c Larry Finger 0 siblings, 2 replies; 5+ messages in thread From: Larry Finger @ 2022-05-29 0:47 UTC (permalink / raw) To: gregkh; +Cc: phil, linux-staging, linux-kernel, 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 Larry Finger (2): staging: r8188eu: Fix warning of array overflow in ioctl_linux.c staging: r8188eu: Fix undersized array in rtw_xmit.c drivers/staging/r8188eu/include/rtw_xmit.h | 2 +- drivers/staging/r8188eu/os_dep/ioctl_linux.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) -- 2.36.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] staging: r8188eu: Fix warning of array overflow in ioctl_linux.c 2022-05-29 0:47 [PATCH v2 0/2] Fix some compile warnings in v5.18+ Larry Finger @ 2022-05-29 0:47 ` Larry Finger 2022-05-30 7:44 ` Dan Carpenter 2022-05-29 0:47 ` [PATCH v2 2/2] staging: r8188eu: Fix undersized array in rtw_xmit.c Larry Finger 1 sibling, 1 reply; 5+ messages in thread From: Larry Finger @ 2022-05-29 0:47 UTC (permalink / raw) To: gregkh; +Cc: phil, linux-staging, linux-kernel, 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> --- v2 - Get To and Cc right --- 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 eb9375b0c660..ce3dcfc812e9 100644 --- a/drivers/staging/r8188eu/os_dep/ioctl_linux.c +++ b/drivers/staging/r8188eu/os_dep/ioctl_linux.c @@ -404,7 +404,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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] staging: r8188eu: Fix warning of array overflow in ioctl_linux.c 2022-05-29 0:47 ` [PATCH v2 1/2] staging: r8188eu: Fix warning of array overflow in ioctl_linux.c Larry Finger @ 2022-05-30 7:44 ` Dan Carpenter 0 siblings, 0 replies; 5+ messages in thread From: Dan Carpenter @ 2022-05-30 7:44 UTC (permalink / raw) To: Larry Finger; +Cc: gregkh, phil, linux-staging, linux-kernel On Sat, May 28, 2022 at 07:47:10PM -0500, Larry Finger wrote: > 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; I'm not totally sure I understand where the void[25] comes from... This is a false positive, though? Another fix would be to just declare the struct as a variable size array? I don't know where the 16 comes from either. :P I have not followed this logic to other functions. If the 16 is important then the bug is real. Your patch is harmless and less risky than changing the struct definition so I'm fine with that. But I'm just trying to understand the situation better. regards, dan carpenter diff --git a/drivers/staging/r8188eu/include/wlan_bssdef.h b/drivers/staging/r8188eu/include/wlan_bssdef.h index 9d1c9e763287..0d4fb1d0b22b 100644 --- a/drivers/staging/r8188eu/include/wlan_bssdef.h +++ b/drivers/staging/r8188eu/include/wlan_bssdef.h @@ -158,7 +158,7 @@ struct ndis_802_11_wep { u32 KeyIndex; /* 0 is the per-client key, * 1-N are the global keys */ u32 KeyLength; /* length of key in bytes */ - u8 KeyMaterial[16];/* variable len depending on above field */ + u8 KeyMaterial[];/* variable len depending on above field */ }; struct ndis_802_11_auth_req { ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] staging: r8188eu: Fix undersized array in rtw_xmit.c 2022-05-29 0:47 [PATCH v2 0/2] Fix some compile warnings in v5.18+ Larry Finger 2022-05-29 0:47 ` [PATCH v2 1/2] staging: r8188eu: Fix warning of array overflow in ioctl_linux.c Larry Finger @ 2022-05-29 0:47 ` Larry Finger 2022-05-30 8:05 ` Dan Carpenter 1 sibling, 1 reply; 5+ messages in thread From: Larry Finger @ 2022-05-29 0:47 UTC (permalink / raw) To: gregkh; +Cc: phil, linux-staging, linux-kernel, 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 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 <Larry.Finger@lwfinger.net> Cc: Phillip Potter <phil@philpotter.co.uk> --- 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] staging: r8188eu: Fix undersized array in rtw_xmit.c 2022-05-29 0:47 ` [PATCH v2 2/2] staging: r8188eu: Fix undersized array in rtw_xmit.c Larry Finger @ 2022-05-30 8:05 ` Dan Carpenter 0 siblings, 0 replies; 5+ messages in thread From: Dan Carpenter @ 2022-05-30 8:05 UTC (permalink / raw) To: Larry Finger; +Cc: gregkh, phil, linux-staging, linux-kernel 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; } ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-05-30 8:06 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-05-29 0:47 [PATCH v2 0/2] Fix some compile warnings in v5.18+ Larry Finger 2022-05-29 0:47 ` [PATCH v2 1/2] staging: r8188eu: Fix warning of array overflow in ioctl_linux.c Larry Finger 2022-05-30 7:44 ` Dan Carpenter 2022-05-29 0:47 ` [PATCH v2 2/2] staging: r8188eu: Fix undersized array in rtw_xmit.c Larry Finger 2022-05-30 8:05 ` Dan Carpenter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.