[PATCH-v2] nl80211: Fix bug in match set count calculation

[PATCH-v2] nl80211: Fix bug in match set count calculation

[Raja Mani]

Match set count is calculated in nl80211_start_sched_scan() by counting
each attr available in NL80211_ATTR_SCHED_SCAN_MATCH. Some cases, RSSI
threshold limit NL80211_SCHED_SCAN_MATCH_ATTR_RSSI attr also can come
from user space along with NL80211_SCHED_SCAN_MATCH_ATTR_SSID attr.
In such cases, exiting code counts NL80211_SCHED_SCAN_MATCH_ATTR_RSSI
attr also as one of SSID and leads extra memory allocation for match set
array (request->match_sets).

Counting only NL80211_SCHED_SCAN_MATCH_ATTR_RSSI attr will help nl80211 to
allocate exact memory needed for match set array and also driver can know
the exact valid SSID available in match set array.

Signed-off-by: Raja Mani <rmani@qti.qualcomm.com>
---

V2 changes:
* Changed commit text and patch title as per johannas comments.

 net/wireless/nl80211.c |   12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index d0afd82..2d3a86f 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -5504,11 +5504,17 @@ static int nl80211_start_sched_scan(struct sk_buff *skb,
 	if (n_ssids > wiphy->max_sched_scan_ssids)
 		return -EINVAL;
 
-	if (info->attrs[NL80211_ATTR_SCHED_SCAN_MATCH])
+	if (info->attrs[NL80211_ATTR_SCHED_SCAN_MATCH]) {
 		nla_for_each_nested(attr,
 				    info->attrs[NL80211_ATTR_SCHED_SCAN_MATCH],
-				    tmp)
-			n_match_sets++;
+				    tmp) {
+			nla_parse(tb, NL80211_SCHED_SCAN_MATCH_ATTR_MAX,
+				  nla_data(attr), nla_len(attr),
+				  nl80211_match_policy);
+			if (tb[NL80211_SCHED_SCAN_MATCH_ATTR_SSID])
+				n_match_sets++;
+		}
+	}
 
 	if (n_match_sets > wiphy->max_match_sets)
 		return -EINVAL;
-- 
1.7.10.4

Re: [PATCH-v2] nl80211: Fix bug in match set count calculation

[Johannes Berg]

On Thu, 2014-01-23 at 17:04 +0530, Raja Mani wrote:
> Match set count is calculated in nl80211_start_sched_scan() by counting
> each attr available in NL80211_ATTR_SCHED_SCAN_MATCH. Some cases, RSSI
> threshold limit NL80211_SCHED_SCAN_MATCH_ATTR_RSSI attr also can come
> from user space along with NL80211_SCHED_SCAN_MATCH_ATTR_SSID attr.
> In such cases, exiting code counts NL80211_SCHED_SCAN_MATCH_ATTR_RSSI
> attr also as one of SSID and leads extra memory allocation for match set
> array (request->match_sets).
> 
> Counting only NL80211_SCHED_SCAN_MATCH_ATTR_RSSI attr will help nl80211 to
> allocate exact memory needed for match set array and also driver can know
> the exact valid SSID available in match set array.

Much better now. I still think you should explain *why* this is a
problem. It's not obvious at first glance that the sched scan match
thing is actually completely stupid, allowing the RSSI attribute in the
nested thing but then only using a single one of it ... Luca, any ideas
about that part? Was something else intended?

johannes

Re: [PATCH-v2] nl80211: Fix bug in match set count calculation

[Luca Coelho]

On Fri, 2014-01-24 at 10:21 +0100, Johannes Berg wrote:
> On Thu, 2014-01-23 at 17:04 +0530, Raja Mani wrote:
> > Match set count is calculated in nl80211_start_sched_scan() by counting
> > each attr available in NL80211_ATTR_SCHED_SCAN_MATCH. Some cases, RSSI
> > threshold limit NL80211_SCHED_SCAN_MATCH_ATTR_RSSI attr also can come
> > from user space along with NL80211_SCHED_SCAN_MATCH_ATTR_SSID attr.
> > In such cases, exiting code counts NL80211_SCHED_SCAN_MATCH_ATTR_RSSI
> > attr also as one of SSID and leads extra memory allocation for match set
> > array (request->match_sets).
> > 
> > Counting only NL80211_SCHED_SCAN_MATCH_ATTR_RSSI attr will help nl80211 to
> > allocate exact memory needed for match set array and also driver can know
> > the exact valid SSID available in match set array.
> 
> Much better now. I still think you should explain *why* this is a
> problem. It's not obvious at first glance that the sched scan match
> thing is actually completely stupid, allowing the RSSI attribute in the
> nested thing but then only using a single one of it ... Luca, any ideas
> about that part? Was something else intended?

As discussed on IRC, the problem was the implementation of the RSSI
attribute.  It should have been included in every matchset (ie. one for
each matchset that contains a BSSI attribute) to have the desired
effect.

Looked at your other patch and it looks like a good wait to solve the
problem, while keeping backwards compatibility.

--
Luca.