2023-10-21 15:52:30

by Ben Greear

[permalink] [raw]
Subject: [PATCH] wifi: wireless: Fix bad memory passed in inform_single_bss_data.

From: Ben Greear <[email protected]>

The sins of similar variable names and passing void pointers
are seen again in wireless-next tree.

Wrong data was passed into the rdev_inform_bss method causing
crashes.

Signed-off-by: Ben Greear <[email protected]>
---
net/wireless/scan.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/wireless/scan.c b/net/wireless/scan.c
index 7f8e831ef1dc..52b048675cc7 100644
--- a/net/wireless/scan.c
+++ b/net/wireless/scan.c
@@ -2155,7 +2155,7 @@ cfg80211_inform_single_bss_data(struct wiphy *wiphy,
if (!res)
goto drop;

- rdev_inform_bss(rdev, &res->pub, ies, data->drv_data);
+ rdev_inform_bss(rdev, &res->pub, ies, drv_data->drv_data);

if (data->bss_source == BSS_SOURCE_MBSSID) {
/* this is a nontransmitting bss, we need to add it to
--
2.40.0


2023-10-22 12:15:22

by James Dutton

[permalink] [raw]
Subject: Re: [PATCH] wifi: wireless: Fix bad memory passed in inform_single_bss_data.

On Sat, 21 Oct 2023 at 16:52, <[email protected]> wrote:
>
> From: Ben Greear <[email protected]>
>
> The sins of similar variable names and passing void pointers
> are seen again in wireless-next tree.
>
> Wrong data was passed into the rdev_inform_bss method causing
> crashes.
>

Is there any good reason for the void pointers?
The patch you propose fixes the immediate problem, but if the void
pointers were replaced with struct pointers, the compiler could catch
this sort of problem.
I imagine there could be similar confusion with this struct and
function having the same name:
0 scan.c 1999 struct cfg80211_inform_single_bss_data {
1 scan.c 2023 cfg80211_inform_single_bss_data(struct wiphy *wiphy,

Maybe renaming the following:
struct cfg80211_inform_bss *drv_data = data->drv_data;
to
struct cfg80211_inform_bss *c_inform_bss1 = data->drv_data;
would reduce the confusion.

2023-10-22 18:31:37

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH] wifi: wireless: Fix bad memory passed in inform_single_bss_data.

On 10/22/23 5:14 AM, James Dutton wrote:
> On Sat, 21 Oct 2023 at 16:52, <[email protected]> wrote:
>>
>> From: Ben Greear <[email protected]>
>>
>> The sins of similar variable names and passing void pointers
>> are seen again in wireless-next tree.
>>
>> Wrong data was passed into the rdev_inform_bss method causing
>> crashes.
>>
>
> Is there any good reason for the void pointers?
> The patch you propose fixes the immediate problem, but if the void
> pointers were replaced with struct pointers, the compiler could catch
> this sort of problem.
> I imagine there could be similar confusion with this struct and
> function having the same name:
> 0 scan.c 1999 struct cfg80211_inform_single_bss_data {
> 1 scan.c 2023 cfg80211_inform_single_bss_data(struct wiphy *wiphy,
>
> Maybe renaming the following:
> struct cfg80211_inform_bss *drv_data = data->drv_data;
> to
> struct cfg80211_inform_bss *c_inform_bss1 = data->drv_data;
> would reduce the confusion.

It takes a lot of time and patience to get more complicated patches upstreamed,
so maybe someone else has interested in pursuing that.

Probably a somewhat generic call-back struct could be used to pass info back to
the mac80211 stack instead of using the void*, but that would require touching
a larger set of drivers most likely.

I am tracking down more crash bugs in the mean time.

Thanks,
Ben

--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com