[PATCH] nl80211: Fix unfiltered GET_INTERFACE dumps

[PATCH] nl80211: Fix unfiltered GET_INTERFACE dumps

[Denis Kenzior]

dump_wiphy_parse only assigns filter_wiphy if one of the supported
NL80211 attributes is present.  So for unfiltered dumps, filter_wiphy
was always initialized to 0, and only interface 0 was dumped.

This was introduced in commit 2d75da13fbb957e955d212555b91101cef36f0ce.

Reported-by: Arend Van Spriel <arend.vanspriel@broadcom.com>
Signed-off-by: Denis Kenzior <denkenz@gmail.com>
---
 net/wireless/nl80211.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 95d55d2..ddc994a 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -2534,6 +2534,7 @@ static int nl80211_dump_interface(struct sk_buff *skb, struct netlink_callback *
 		struct nl80211_dump_wiphy_state state = {};
 		int ret;
 
+		state.filter_wiphy = -1;
 		ret = nl80211_dump_wiphy_parse(skb, cb, &state);
 		if (ret)
 			return ret;
-- 
2.7.3

Re: [PATCH] nl80211: Fix unfiltered GET_INTERFACE dumps

[Arend van Spriel]

On 25-08-16 22:44, Denis Kenzior wrote:
> dump_wiphy_parse only assigns filter_wiphy if one of the supported
> NL80211 attributes is present.  So for unfiltered dumps, filter_wiphy
> was always initialized to 0, and only interface 0 was dumped.
> 
> This was introduced in commit 2d75da13fbb957e955d212555b91101cef36f0ce.
> 
> Reported-by: Arend Van Spriel <arend.vanspriel@broadcom.com>

Actually sent a patch for this issue a little earlier. I should have
Cc'ed you explicitly, I guess.

Regards,
Arend

> Signed-off-by: Denis Kenzior <denkenz@gmail.com>
> ---
>  net/wireless/nl80211.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 95d55d2..ddc994a 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -2534,6 +2534,7 @@ static int nl80211_dump_interface(struct sk_buff *skb, struct netlink_callback *
>  		struct nl80211_dump_wiphy_state state = {};
>  		int ret;
>  
> +		state.filter_wiphy = -1;
>  		ret = nl80211_dump_wiphy_parse(skb, cb, &state);
>  		if (ret)
>  			return ret;
> 

Re: [PATCH] nl80211: Fix unfiltered GET_INTERFACE dumps

[Denis Kenzior]

Hi Arend,

On 08/25/2016 04:35 PM, Arend van Spriel wrote:
> On 25-08-16 22:44, Denis Kenzior wrote:
>> dump_wiphy_parse only assigns filter_wiphy if one of the supported
>> NL80211 attributes is present.  So for unfiltered dumps, filter_wiphy
>> was always initialized to 0, and only interface 0 was dumped.
>>
>> This was introduced in commit 2d75da13fbb957e955d212555b91101cef36f0ce.
>>
>> Reported-by: Arend Van Spriel <arend.vanspriel@broadcom.com>
>
> Actually sent a patch for this issue a little earlier. I should have
> Cc'ed you explicitly, I guess.
>

Whoops.  I saw your regression report and didn't look to see if you 
fixed it :)

Looking at your patch, I'm worried that

-	int filter_wiphy = -1;
+	int filter_wiphy;

might still break things, since Johannes used:

	if (!cb->args[2]) {
		...
         } else if (cb->args[2] > 0) {
                 filter_wiphy = cb->args[2] - 1;
         }

So for unfiltered dumps, filter_wiphy would not be initialized properly 
for the second & onward call of nl80211_dump_interface. Right?

Regards,
-Denis

Re: [PATCH] nl80211: Fix unfiltered GET_INTERFACE dumps

[Arend van Spriel]

On 25-08-16 23:52, Denis Kenzior wrote:
> Hi Arend,
> 
> On 08/25/2016 04:35 PM, Arend van Spriel wrote:
>> On 25-08-16 22:44, Denis Kenzior wrote:
>>> dump_wiphy_parse only assigns filter_wiphy if one of the supported
>>> NL80211 attributes is present.  So for unfiltered dumps, filter_wiphy
>>> was always initialized to 0, and only interface 0 was dumped.
>>>
>>> This was introduced in commit 2d75da13fbb957e955d212555b91101cef36f0ce.
>>>
>>> Reported-by: Arend Van Spriel <arend.vanspriel@broadcom.com>
>>
>> Actually sent a patch for this issue a little earlier. I should have
>> Cc'ed you explicitly, I guess.
>>
> 
> Whoops.  I saw your regression report and didn't look to see if you
> fixed it :)

I wanted to avoid having to revert so I decided to take a closer look.

> Looking at your patch, I'm worried that
> 
> -    int filter_wiphy = -1;
> +    int filter_wiphy;
> 
> might still break things, since Johannes used:
> 
>     if (!cb->args[2]) {
>         ...

Maybe my look was not close enough. In this branch cb->args[2] is set to
-1. So indeed that would make filter_wiphy uninitialized in subsequent
call(s). Thanks for catching that.

Regards,
Arend

>         } else if (cb->args[2] > 0) {
>                 filter_wiphy = cb->args[2] - 1;
>         }
> 
> So for unfiltered dumps, filter_wiphy would not be initialized properly
> for the second & onward call of nl80211_dump_interface. Right?
> 
> Regards,
> -Denis

Re: [PATCH] nl80211: Fix unfiltered GET_INTERFACE dumps

[Johannes Berg]

On Thu, 2016-08-25 at 15:44 -0500, Denis Kenzior wrote:
> dump_wiphy_parse only assigns filter_wiphy if one of the supported
> NL80211 attributes is present.  So for unfiltered dumps, filter_wiphy
> was always initialized to 0, and only interface 0 was dumped.
> 
> This was introduced in commit
> 2d75da13fbb957e955d212555b91101cef36f0ce.
> 
> Reported-by: Arend Van Spriel <arend.vanspriel@broadcom.com>
> Signed-off-by: Denis Kenzior <denkenz@gmail.com>
> 
Thanks guys, I've squashed this (and moved it into the initializer we
already had.)

johannes