Re: [PATCH] nl80211: fix nlmsg allocation in cfg80211_ft_event
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
RE: [PATCH] nl80211: fix nlmsg allocation in cfg80211_ft_event
> From: linux-wireless-ow...@vger.kernel.org > [mailto:linux-wireless-ow...@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.
Re: [PATCH] nl80211: fix nlmsg allocation in cfg80211_ft_event
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
Re: [PATCH] nl80211: fix nlmsg allocation in cfg80211_ft_event
On 2018-05-17 06:25, Dedy Lansky wrote: From: Dedy Lansky 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 --- 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"?
