| Message ID | 20250306124057.6ae37b21922b.I79b010ea9aab47893e4f22c266362fde30b7f9ac@changeid (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | wifi: mac80211/cfg80211: updates - 2025-03-06 | expand |
Miri Korenblit <miriam.rachel.korenblit@intel.com> wrote: > From: Ilan Peer <ilan.peer@intel.com> > > When links are added, update the wireless device link addresses based > on the information provided by the driver. > > Signed-off-by: Ilan Peer <ilan.peer@intel.com> > Signed-off-by: Miri Korenblit <miriam.rachel.korenblit@intel.com> > --- > include/net/cfg80211.h | 1 + > net/wireless/mlme.c | 4 ++++ > 2 files changed, 5 insertions(+) > > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h > index 6f76059c0aa5..558dc88b9f07 100644 > --- a/include/net/cfg80211.h > +++ b/include/net/cfg80211.h > @@ -9771,6 +9771,7 @@ struct cfg80211_mlo_reconf_done_data { > u16 added_links; > struct { > struct cfg80211_bss *bss; > + u8 *addr; Should swap order of patch 12/15, which does assign addr? At first glance, this patch doesn't set addr and callee does memcpy(), which kernel will raise NULL pointer dereference exception. And there are two callers, but patch 12/15 only set one of them. mac80211/mlme.c:3896: cfg80211_mlo_reconf_add_done(sdata->dev, &done_data); mac80211/mlme.c:10125: cfg80211_mlo_reconf_add_done(sdata->dev, &done_data); > } links[IEEE80211_MLD_MAX_NUM_LINKS]; > }; > > diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c > index 956d33b219df..05d44a443518 100644 > --- a/net/wireless/mlme.c > +++ b/net/wireless/mlme.c > @@ -1360,6 +1360,10 @@ void cfg80211_mlo_reconf_add_done(struct net_device *dev, > if (data->added_links & BIT(link_id)) { > wdev->links[link_id].client.current_bss = > bss_from_pub(bss); > + > + memcpy(wdev->links[link_id].addr, > + data->links[link_id].addr, > + ETH_ALEN); > } else { > cfg80211_unhold_bss(bss_from_pub(bss)); > cfg80211_put_bss(wiphy, bss); > -- > 2.34.1 >
> > Miri Korenblit <miriam.rachel.korenblit@intel.com> wrote: > > From: Ilan Peer <ilan.peer@intel.com> > > > > When links are added, update the wireless device link addresses based > > on the information provided by the driver. > > > > Signed-off-by: Ilan Peer <ilan.peer@intel.com> > > Signed-off-by: Miri Korenblit <miriam.rachel.korenblit@intel.com> > > --- > > include/net/cfg80211.h | 1 + > > net/wireless/mlme.c | 4 ++++ > > 2 files changed, 5 insertions(+) > > > > diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index > > 6f76059c0aa5..558dc88b9f07 100644 > > --- a/include/net/cfg80211.h > > +++ b/include/net/cfg80211.h > > @@ -9771,6 +9771,7 @@ struct cfg80211_mlo_reconf_done_data { > > u16 added_links; > > struct { > > struct cfg80211_bss *bss; > > + u8 *addr; > > Should swap order of patch 12/15, which does assign addr? I can't swap the order, because it won't build? (the caller will set a data member that does not exist) I could have a check that addr is not NULL before memcpy'ing it But this feature is disabled anyway... > > At first glance, this patch doesn't set addr and callee does memcpy(), which > kernel will raise NULL pointer dereference exception. > > And there are two callers, but patch 12/15 only set one of them. Note that the addr field is only used if there is something set in done_data->added_links. But this is not the case for the first caller, so it is ok. > > mac80211/mlme.c:3896: cfg80211_mlo_reconf_add_done(sdata->dev, > &done_data); > mac80211/mlme.c:10125: cfg80211_mlo_reconf_add_done(sdata->dev, > &done_data); > > > } links[IEEE80211_MLD_MAX_NUM_LINKS]; > > }; > > > > diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c index > > 956d33b219df..05d44a443518 100644 > > --- a/net/wireless/mlme.c > > +++ b/net/wireless/mlme.c > > @@ -1360,6 +1360,10 @@ void cfg80211_mlo_reconf_add_done(struct > net_device *dev, > > if (data->added_links & BIT(link_id)) { > > wdev->links[link_id].client.current_bss = > > bss_from_pub(bss); > > + > > + memcpy(wdev->links[link_id].addr, > > + data->links[link_id].addr, > > + ETH_ALEN); > > } else { > > cfg80211_unhold_bss(bss_from_pub(bss)); > > cfg80211_put_bss(wiphy, bss); > > -- > > 2.34.1 > >
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index 6f76059c0aa5..558dc88b9f07 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -9771,6 +9771,7 @@ struct cfg80211_mlo_reconf_done_data { u16 added_links; struct { struct cfg80211_bss *bss; + u8 *addr; } links[IEEE80211_MLD_MAX_NUM_LINKS]; }; diff --git a/net/wireless/mlme.c b/net/wireless/mlme.c index 956d33b219df..05d44a443518 100644 --- a/net/wireless/mlme.c +++ b/net/wireless/mlme.c @@ -1360,6 +1360,10 @@ void cfg80211_mlo_reconf_add_done(struct net_device *dev, if (data->added_links & BIT(link_id)) { wdev->links[link_id].client.current_bss = bss_from_pub(bss); + + memcpy(wdev->links[link_id].addr, + data->links[link_id].addr, + ETH_ALEN); } else { cfg80211_unhold_bss(bss_from_pub(bss)); cfg80211_put_bss(wiphy, bss);