| Message ID | 000901d3ede2$78a3aa20$69eafe60$@codeaurora.org (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Johannes Berg |
| Headers | show |
On 2018-05-17 06:25, Dedy Lansky wrote: > From: Dedy Lansky <dlansky@codeaurora.org> > > Allocation size of nlmsg in cfg80211_ft_event is based on ric_ies_len > and doesn't take into account ies_len. This leads to > NL80211_CMD_FT_EVENT message construction failure in case ft_event > contains large enough ies buffer. > Add ies_len to the nlmsg allocation size. > > Signed-off-by: Dedy Lansky <dlansky@codeaurora.org> > --- > net/wireless/nl80211.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c > index afbe510..64afd04 100644 > --- a/net/wireless/nl80211.c > +++ b/net/wireless/nl80211.c > @@ -15755,7 +15755,8 @@ void cfg80211_ft_event(struct net_device > *netdev, > if (!ft_event->target_ap) > return; > > - msg = nlmsg_new(100 + ft_event->ric_ies_len, GFP_KERNEL); > + msg = nlmsg_new(100 + ft_event->ies_len + ft_event->ric_ies_len, > + GFP_KERNEL); > if (!msg) > return; should these really be nla_total_size(ft_event->ies_len) + nla_total_size(ft_event->ric_ies_len) to properly account for the NLA header + padding? or do we consider that to be noise captured by the "100"?
On Thu, 2018-05-17 at 11:43 -0700, Jeff Johnson wrote: > > > - msg = nlmsg_new(100 + ft_event->ric_ies_len, GFP_KERNEL); > > + msg = nlmsg_new(100 + ft_event->ies_len + ft_event->ric_ies_len, > > + GFP_KERNEL); > > if (!msg) > > return; > > should these really be nla_total_size(ft_event->ies_len) + > nla_total_size(ft_event->ric_ies_len) to properly account for the NLA > header + padding? or do we consider that to be noise captured by the > "100"? We do, technically we should have something like nla_total_size() of various things including all those wiphy, ifindex, MAC attributes etc. so we just get lazy... johannes
> From: linux-wireless-owner@vger.kernel.org [mailto:linux-wireless-owner@vger.kernel.org] On Behalf Of Johannes Berg > > On Thu, 2018-05-17 at 11:43 -0700, Jeff Johnson wrote: > > > > > - msg = nlmsg_new(100 + ft_event->ric_ies_len, GFP_KERNEL); > > > + msg = nlmsg_new(100 + ft_event->ies_len + ft_event->ric_ies_len, > > > + GFP_KERNEL); > > > if (!msg) > > > return; > > > > should these really be nla_total_size(ft_event->ies_len) + > > nla_total_size(ft_event->ric_ies_len) to properly account for the NLA > > header + padding? or do we consider that to be noise captured by the > > "100"? > > We do, technically we should have something like nla_total_size() of various things including all those wiphy, ifindex, MAC attributes etc. > so we just get lazy... nla_total_size is currently not used in nl80211.c (actually not used in net\wireless\ for that matters). IMO, switching nl80211/cfg80211 to use nla_total_size should be done separately. This patch is for fixing a very specific and small bug. Using nla_total_size in a single function in the file (cfg80211_ft_event) would be awkward. Thanks, Dedy.
On Mon, 2018-05-21 at 10:23 +0300, Dedy Lansky wrote: > > We do, technically we should have something like nla_total_size() of various things including all those wiphy, ifindex, MAC attributes etc. > > so we just get lazy... > > nla_total_size is currently not used in nl80211.c (actually not used > in net\wireless\ for that matters). Like I said, we're lazy ;-) > IMO, switching nl80211/cfg80211 to use nla_total_size should be done > separately. I don't see much value in doing it at all, TBH. johannes
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index afbe510..64afd04 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -15755,7 +15755,8 @@ void cfg80211_ft_event(struct net_device *netdev, if (!ft_event->target_ap) return; - msg = nlmsg_new(100 + ft_event->ric_ies_len, GFP_KERNEL); + msg = nlmsg_new(100 + ft_event->ies_len + ft_event->ric_ies_len, + GFP_KERNEL); if (!msg) return;