[PATCH] nl80211: fix nlmsg allocation in cfg80211_ft_event

[PATCH] nl80211: fix nlmsg allocation in cfg80211_ft_event

[Dedy Lansky]

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;
 
-- 
1.9.1

Re: [PATCH] nl80211: fix nlmsg allocation in cfg80211_ft_event

[Jeff Johnson]

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"?

Re: [PATCH] nl80211: fix nlmsg allocation in cfg80211_ft_event

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

johannes

RE: [PATCH] nl80211: fix nlmsg allocation in cfg80211_ft_event

[Dedy Lansky]

> 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:
> >=20
> > > -	msg =3D nlmsg_new(100 + ft_event->ric_ies_len, GFP_KERNEL);
> > > +	msg =3D nlmsg_new(100 + ft_event->ies_len + =
ft_event->ric_ies_len,
> > > +			GFP_KERNEL);
> > >  	if (!msg)
> > >  		return;
> >=20
> > 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=20
> > 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

[Johannes Berg]

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