On Sunday, June 6, 2021 10:09:39 AM CEST Wenli Looi wrote:

On Sun, Jun 6, 2021 at 1:00 AM Fabio M. De Francesco

[off-list ref] wrote:

quoted

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

quoted

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

quoted

quoted

quoted

quoted

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..

quoted

7308e1185

quoted

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

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?

quoted

quoted

quoted

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

thanks,
greg k-h

[CUT]

quoted

quoted

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.

The stack variable could be replaced with code like:

struct station_info *sinfo;if (!sinfo)

Why that "if (!sinfo" before kzalloc?

sinfo = kzalloc(sizeof(*sinfo), GFP_KERNEL);
...
cfg80211_new_sta(..., sinfo, ...);
kfree(sinfo);

which is what the linked code basically does. I'm not sure if this is 

preferred?

I don't know that code, but I think that:

(1) It is generally preferred that big structures should be allocated 
dinamically: kernel stack is a scarce resource.
(2) Passing address of variables from the stack to other functions is 
generally a good source of troubles.

I think you are closer to the proper solution.

Fabio 

quoted

quoted

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