2016-08-25 20:46:27

by Denis Kenzior

[permalink] [raw]
Subject: [PATCH] nl80211: Fix unfiltered GET_INTERFACE dumps

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 <[email protected]>
Signed-off-by: Denis Kenzior <[email protected]>
---
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


2016-08-26 08:13:10

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] nl80211: Fix unfiltered GET_INTERFACE dumps

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 <[email protected]>
> Signed-off-by: Denis Kenzior <[email protected]>
>
Thanks guys, I've squashed this (and moved it into the initializer we
already had.)

johannes

2016-08-25 22:07:02

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCH] nl80211: Fix unfiltered GET_INTERFACE dumps

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 <[email protected]>
>>
>> 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

2016-08-25 22:30:00

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCH] nl80211: Fix unfiltered GET_INTERFACE dumps

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 <[email protected]>

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 <[email protected]>
> ---
> 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;
>

2016-08-25 21:53:38

by Denis Kenzior

[permalink] [raw]
Subject: Re: [PATCH] nl80211: Fix unfiltered GET_INTERFACE dumps

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 <[email protected]>
>
> 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