Messages in this thread Patch in this message | | | Date | Mon, 18 Mar 2024 18:07:34 +0100 | Subject | Re: [RFC] staging: wlan-ng: Driver broken since kernel 5.15 | From | Philipp Hortmann <> |
| |
On 3/18/24 09:01, Dan Carpenter wrote: > On Sun, Mar 17, 2024 at 09:20:34PM +0100, Arnd Bergmann wrote: >> On Sun, Mar 17, 2024, at 21:07, Philipp Hortmann wrote: >>> On 3/11/24 08:04, Dan Carpenter wrote: >>>> On Sat, Mar 09, 2024 at 11:09:24PM +0100, Philipp Hortmann wrote: >>> You are right with the statement that it is this commit. >>> commit ea82ff749587807fa48e3277c977ff3cec266f25 (HEAD) >>> Author: Lee Jones <lee.jones@linaro.org> >>> Date: Wed Apr 14 19:10:39 2021 +0100 >>> >>> staging: wlan-ng: cfg80211: Move large struct onto the heap >>> >>> Fixes the following W=1 kernel build warning(s): >>> >>> drivers/staging/wlan-ng/cfg80211.c: In function ‘prism2_scan’: >>> drivers/staging/wlan-ng/cfg80211.c:388:1: warning: the frame size >>> of 1296 bytes is larger than 1024 bytes [-Wframe-larger-than=] >>> >>> But It is not depending on the line you pointed to. >> >> Right, the kzalloc() already clears the data, so the memset >> is not needed. >> > > No, it's inside a loop so it needs to be cleared on each iteration. > >>> I need another week to look into this. >> >> I'm fairly sure this fixes the bug, the problem here was that >> the cast to (u8 *) hides the incorrect conversion: >> >> diff --git a/drivers/staging/wlan-ng/cfg80211.c b/drivers/staging/wlan-ng/cfg80211.c >> index 471bb310176f..9d6a2dd35ba9 100644 >> --- a/drivers/staging/wlan-ng/cfg80211.c >> +++ b/drivers/staging/wlan-ng/cfg80211.c >> @@ -350,7 +350,7 @@ static int prism2_scan(struct wiphy *wiphy, >> msg2->msgcode = DIDMSG_DOT11REQ_SCAN_RESULTS; >> msg2->bssindex.data = i; >> >> - result = p80211req_dorequest(wlandev, (u8 *)&msg2); >> + result = p80211req_dorequest(wlandev, (u8 *)msg2); > > Ah, well done. > > It feels like this is the kind of bug which should be caught with > static analysis. One of the things that people want from static > analysis is looking at what a patch does. So if we pass &msg2 and the > patch moved msg from the stack to be kmalloc()ed, then print a warning. > It's not something that Smatch does. > > I have my rename_rev.pl script (https://github.com/error27/rename_rev) > which I use to filter out variable renames or see if (1 << foo) is > converted to BIT(foo) correctly. Maybe I could extend that to check > "move stack to heap" patches... > > regards, > dan carpenter >
Hi,
This change alone.
+++ b/drivers/staging/wlan-ng/cfg80211.c @@ -350,7 +350,7 @@ static int prism2_scan(struct wiphy *wiphy, msg2->msgcode = DIDMSG_DOT11REQ_SCAN_RESULTS; msg2->bssindex.data = i;
- result = p80211req_dorequest(wlandev, (u8 *)&msg2); + result = p80211req_dorequest(wlandev, (u8 *)msg2); if ((result != 0) || (msg2->resultcode.data != P80211ENUM_resultcode_success)) { break; makes the driver work again under latest kernel. I can only use unencrypted transmission. Throughput is 200 to 800kByte/s WEP would need further investigation.
Thanks for your support.
Bye Philipp
| |