On Sunday, June 6, 2021 9:51:35 AM CEST Wenli Looi wrote:

On Sun, Jun 6, 2021 at 12:13 AM Greg Kroah-Hartman

[off-list ref] wrote:

quoted

On Sun, Jun 06, 2021 at 12:00:21AM -0700, Wenli Looi wrote:

quoted

Uninitialized struct with invalid pointer causes BUG and prevents access
point from working. Access point works once I apply this patch.

https://forum.armbian.com/topic/14727-wifi-ap-kernel-bug-in-kernel-5444/
has more details.

Signed-off-by: Wenli Looi <redacted>
---

 drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c

b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c index 2fb80b6eb..

7308e1185

quoted

quoted

100644

--- a/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c
+++ b/drivers/staging/rtl8723bs/os_dep/ioctl_cfg80211.c

@@ -2384,7 +2384,7 @@ void rtw_cfg80211_indicate_sta_assoc(struct 

adapter *padapter,

quoted

quoted

u8 *pmgmt_frame,> > 
      DBG_871X(FUNC_ADPT_FMT"\n", FUNC_ADPT_ARG(padapter));
      
      {

-             struct station_info sinfo;
+             struct station_info sinfo = {};

What caused this bug to show up?  Did it happen from some other commit?

Are you sure that all of the fields are being cleared properly here,
what about any "holes" in the structure?

thanks,
greg k-h

I believe this bug has been present since the driver was added to
staging:
https://github.com/torvalds/linux/commit/

554c0a3abf216c991c5ebddcdb2c08689ecd290b

It's probably not necessary to zero the entire struct, only
sinfo->pertid, which causes problems with the code here:
https://github.com/torvalds/linux/blob/

f5b6eb1e018203913dfefcf6fa988649ad11ad6e/net/wire

less/nl80211.c#L5919

You can see the following proposed OpenWrt patch
(700-wifi-8723bs-ap-bugfix.patch in
https://github.com/openwrt/openwrt/pull/4053/files) which sets
sinfo.pertid = 0; instead of zeroing the struct.

Looking at similar code in a non-staging driver, we can see that the
code there zeros the struct using kzalloc():
https://github.com/torvalds/linux/blob/

f5b6eb1e018203913dfefcf6fa988649ad11ad6e/drivers/

net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c#L6064

Do you think kzalloc() would be preferable?

You cannot use kzalloc there: 'sinfo' is instantiated automatically on the 
stack. The example you took had a pointer to the struct. 

Fabio

Sorry, I'm not familiar with "holes" in the struct.