| Message ID | 1472157884-3174-1-git-send-email-denkenz@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Johannes Berg |
| Headers | show |
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; >
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
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
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
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;
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(+)