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