[PATCH][next] staging: rtl8723bs: Replace one-element array with flexible-array member in struct ndis_80211_var_ie

Subsystems: staging subsystem, the rest

STALE2082d LANDED

Landed in mainline as bc8392e33d9a on 2021-02-11.

2 messages, 2 authors, 2021-02-11 · open the first message on its own page

[PATCH][next] staging: rtl8723bs: Replace one-element array with flexible-array member in struct ndis_80211_var_ie

From: Gustavo A. R. Silva <gustavoars@kernel.org>
Date: 2021-02-10 22:50:34

There is a regular need in the kernel to provide a way to declare having
a dynamically sized set of trailing elements in a structure. Kernel code
should always use “flexible array members”[1] for these cases. The older
style of one-element or zero-length arrays should no longer be used[2].

Refactor the code according to the use of a flexible-array member in
struct ndis_80211_var_ie, instead of a one-element array.

Also, this helps with the ongoing efforts to enable -Warray-bounds and
fix the following warnings:

  CC [M]  drivers/staging/rtl8723bs/core/rtw_wlan_util.o
In file included from ./drivers/staging/rtl8723bs/include/drv_types.h:20,
                 from drivers/staging/rtl8723bs/core/rtw_wlan_util.c:9:
drivers/staging/rtl8723bs/core/rtw_wlan_util.c: In function ‘HT_caps_handler’:
./drivers/staging/rtl8723bs/include/basic_types.h:108:11: warning: array subscript 1 is above array bounds of ‘u8[1]’ {aka ‘unsigned char[1]’} [-Warray-bounds]
  108 |  (EF1BYTE(*((u8 *)(__pstart))))
      |           ^
./drivers/staging/rtl8723bs/include/basic_types.h:42:8: note: in definition of macro ‘EF1BYTE’
   42 |  ((u8)(_val))
      |        ^~~~
./drivers/staging/rtl8723bs/include/basic_types.h:127:4: note: in expansion of macro ‘LE_P1BYTE_TO_HOST_1BYTE’
  127 |   (LE_P1BYTE_TO_HOST_1BYTE(__pstart) >> (__bitoffset)) & \
      |    ^~~~~~~~~~~~~~~~~~~~~~~
./drivers/staging/rtl8723bs/include/rtw_ht.h:97:55: note: in expansion of macro ‘LE_BITS_TO_1BYTE’
   97 | #define GET_HT_CAPABILITY_ELE_RX_STBC(_pEleStart)     LE_BITS_TO_1BYTE((_pEleStart)+1, 0, 2)
      |                                                       ^~~~~~~~~~~~~~~~
drivers/staging/rtl8723bs/core/rtw_wlan_util.c:1104:58: note: in expansion of macro ‘GET_HT_CAPABILITY_ELE_RX_STBC’
 1104 |   if (TEST_FLAG(phtpriv->stbc_cap, STBC_HT_ENABLE_TX) && GET_HT_CAPABILITY_ELE_RX_STBC(pIE->data)) {
      |                                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/staging/rtl8723bs/core/rtw_wlan_util.c:1051:75: warning: array subscript 2 is above array bounds of ‘u8[1]’ {aka ‘unsigned char[1]’} [-Warray-bounds]
 1051 |    if ((pmlmeinfo->HT_caps.u.HT_cap_element.AMPDU_para & 0x3) > (pIE->data[i] & 0x3))
      |                                                                  ~~~~~~~~~^~~
drivers/staging/rtl8723bs/core/rtw_wlan_util.c: In function ‘check_assoc_AP’:
drivers/staging/rtl8723bs/core/rtw_wlan_util.c:1606:19: warning: array subscript 4 is above array bounds of ‘u8[1]’ {aka ‘unsigned char[1]’} [-Warray-bounds]
 1606 |      if (pIE->data[4] == 1)
      |          ~~~~~~~~~^~~
drivers/staging/rtl8723bs/core/rtw_wlan_util.c:1609:20: warning: array subscript 5 is above array bounds of ‘u8[1]’ {aka ‘unsigned char[1]’} [-Warray-bounds]
 1609 |       if (pIE->data[5] & RT_HT_CAP_USE_92SE)
      |           ~~~~~~~~~^~~

19 more lines

diff --git a/drivers/staging/rtl8723bs/include/wlan_bssdef.h b/drivers/staging/rtl8723bs/include/wlan_bssdef.h
index ea370b2bb8db..27cd2c5d90af 100644
--- a/drivers/staging/rtl8723bs/include/wlan_bssdef.h
+++ b/drivers/staging/rtl8723bs/include/wlan_bssdef.h
@@ -68,7 +68,7 @@ struct ndis_802_11_fix_ie {
 struct ndis_80211_var_ie {
 	u8  ElementID;
 	u8  Length;
-	u8  data[1];
+	u8  data[];
 };
 
 /* Length is the 4 bytes multiples of the sum of
-- 
2.27.0

Re: [PATCH][next] staging: rtl8723bs: Replace one-element array with flexible-array member in struct ndis_80211_var_ie

From: Dan Carpenter <hidden>
Date: 2021-02-11 11:38:24

On Wed, Feb 10, 2021 at 04:49:37PM -0600, Gustavo A. R. Silva wrote:
There is a regular need in the kernel to provide a way to declare having
a dynamically sized set of trailing elements in a structure. Kernel code
should always use “flexible array members”[1] for these cases. The older
style of one-element or zero-length arrays should no longer be used[2].

Refactor the code according to the use of a flexible-array member in
struct ndis_80211_var_ie, instead of a one-element array.

Also, this helps with the ongoing efforts to enable -Warray-bounds and
fix the following warnings:

  CC [M]  drivers/staging/rtl8723bs/core/rtw_wlan_util.o
In file included from ./drivers/staging/rtl8723bs/include/drv_types.h:20,
                 from drivers/staging/rtl8723bs/core/rtw_wlan_util.c:9:
drivers/staging/rtl8723bs/core/rtw_wlan_util.c: In function ‘HT_caps_handler’:
./drivers/staging/rtl8723bs/include/basic_types.h:108:11: warning: array subscript 1 is above array bounds of ‘u8[1]’ {aka ‘unsigned char[1]’} [-Warray-bounds]
  108 |  (EF1BYTE(*((u8 *)(__pstart))))
      |           ^
./drivers/staging/rtl8723bs/include/basic_types.h:42:8: note: in definition of macro ‘EF1BYTE’
   42 |  ((u8)(_val))
      |        ^~~~
./drivers/staging/rtl8723bs/include/basic_types.h:127:4: note: in expansion of macro ‘LE_P1BYTE_TO_HOST_1BYTE’
  127 |   (LE_P1BYTE_TO_HOST_1BYTE(__pstart) >> (__bitoffset)) & \
      |    ^~~~~~~~~~~~~~~~~~~~~~~
./drivers/staging/rtl8723bs/include/rtw_ht.h:97:55: note: in expansion of macro ‘LE_BITS_TO_1BYTE’
   97 | #define GET_HT_CAPABILITY_ELE_RX_STBC(_pEleStart)     LE_BITS_TO_1BYTE((_pEleStart)+1, 0, 2)
      |                                                       ^~~~~~~~~~~~~~~~
drivers/staging/rtl8723bs/core/rtw_wlan_util.c:1104:58: note: in expansion of macro ‘GET_HT_CAPABILITY_ELE_RX_STBC’
 1104 |   if (TEST_FLAG(phtpriv->stbc_cap, STBC_HT_ENABLE_TX) && GET_HT_CAPABILITY_ELE_RX_STBC(pIE->data)) {
      |                                                          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/staging/rtl8723bs/core/rtw_wlan_util.c:1051:75: warning: array subscript 2 is above array bounds of ‘u8[1]’ {aka ‘unsigned char[1]’} [-Warray-bounds]
 1051 |    if ((pmlmeinfo->HT_caps.u.HT_cap_element.AMPDU_para & 0x3) > (pIE->data[i] & 0x3))
      |                                                                  ~~~~~~~~~^~~
drivers/staging/rtl8723bs/core/rtw_wlan_util.c: In function ‘check_assoc_AP’:
drivers/staging/rtl8723bs/core/rtw_wlan_util.c:1606:19: warning: array subscript 4 is above array bounds of ‘u8[1]’ {aka ‘unsigned char[1]’} [-Warray-bounds]
 1606 |      if (pIE->data[4] == 1)
      |          ~~~~~~~~~^~~
drivers/staging/rtl8723bs/core/rtw_wlan_util.c:1609:20: warning: array subscript 5 is above array bounds of ‘u8[1]’ {aka ‘unsigned char[1]’} [-Warray-bounds]
 1609 |       if (pIE->data[5] & RT_HT_CAP_USE_92SE)
      |           ~~~~~~~~~^~~

15 more quoted lines

Looks okay to me.  I looked for potential issues with changing the
sizeof the struct but couldn't find any.

Reviewed-by: Dan Carpenter <redacted>

regards,
dan carpenter
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help