diff mbox series

wifi: wireless: Fix bad memory passed in inform_single_bss_data.

Message ID 20231021154827.1142734-1-greearb@candelatech.com
State New
Headers show
Series wifi: wireless: Fix bad memory passed in inform_single_bss_data. | expand

Commit Message

Ben Greear Oct. 21, 2023, 3:48 p.m. UTC
From: Ben Greear <greearb@candelatech.com>

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 <greearb@candelatech.com>
---
 net/wireless/scan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

James Dutton Oct. 22, 2023, 12:14 p.m. UTC | #1
On Sat, 21 Oct 2023 at 16:52, <greearb@candelatech.com> wrote:
>
> From: Ben Greear <greearb@candelatech.com>
>
> 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.
Ben Greear Oct. 22, 2023, 6:30 p.m. UTC | #2
On 10/22/23 5:14 AM, James Dutton wrote:
> On Sat, 21 Oct 2023 at 16:52, <greearb@candelatech.com> wrote:
>>
>> From: Ben Greear <greearb@candelatech.com>
>>
>> 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
diff mbox series

Patch

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