[PATCH] nl80211: Reset split_start on error path when split dump is not used

[PATCH] nl80211: Reset split_start on error path when split dump is not used

[Pontus Fuchs]

When the netlink skb is exhausted split_start is left set. In the
subsequent retry, with a larger buffer, the dump is continued from the
failing point instead of from the beginning.

This was causing my rt28xx based USB dongle to now show up when
running "iw list" with an old iw version without split dump support.

Signed-off-by: Pontus Fuchs <pontus.fuchs@gmail.com>
---
 net/wireless/nl80211.c | 2 ++
 1 file changed, 2 insertions(+)

Supersedes "nl80211: Don't touch split_start when split dump is not used"

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index d0afd82..ba6d438 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1631,6 +1631,8 @@ static int nl80211_send_wiphy(struct cfg80211_registered_device *dev,
 	return genlmsg_end(msg, hdr);
 
  nla_put_failure:
+	if (!state->split)
+		state->split_start = 0;
 	genlmsg_cancel(msg, hdr);
 	return -EMSGSIZE;
 }
-- 
1.8.3.2

Re: [PATCH] nl80211: Reset split_start on error path when split dump is not used

[Johannes Berg]

On Thu, 2014-01-16 at 14:34 +0100, Pontus Fuchs wrote:
> When the netlink skb is exhausted split_start is left set. In the
> subsequent retry, with a larger buffer, the dump is continued from the
> failing point instead of from the beginning.

Hmm. Maybe it should be like this instead?

johannes

diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 138dc3b..2e08af5 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -1680,6 +1680,7 @@ static int nl80211_dump_wiphy(struct sk_buff *skb, struct netlink_callback *cb)
 				    !skb->len &&
 				    cb->min_dump_alloc < 4096) {
 					cb->min_dump_alloc = 4096;
+					state->split_start = 0;
 					rtnl_unlock();
 					return 1;
 				}

Re: [PATCH] nl80211: Reset split_start on error path when split dump is not used

[Pontus Fuchs]

On 2014-01-16 14:41, Johannes Berg wrote:
> On Thu, 2014-01-16 at 14:34 +0100, Pontus Fuchs wrote:
>> When the netlink skb is exhausted split_start is left set. In the
>> subsequent retry, with a larger buffer, the dump is continued from the
>> failing point instead of from the beginning.
>
> Hmm. Maybe it should be like this instead?
>
> johannes
>
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 138dc3b..2e08af5 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -1680,6 +1680,7 @@ static int nl80211_dump_wiphy(struct sk_buff *skb, struct netlink_callback *cb)
>   				    !skb->len &&
>   				    cb->min_dump_alloc < 4096) {
>   					cb->min_dump_alloc = 4096;
> +					state->split_start = 0;
>   					rtnl_unlock();
>   					return 1;
>   				}

Will that not cause problem if split scan is in use and one of the split 
chunks needs to be resent due to EMSGSIZE?

//Pontus

Re: [PATCH] nl80211: Reset split_start on error path when split dump is not used

[Johannes Berg]

On Thu, 2014-01-16 at 14:50 +0100, Pontus Fuchs wrote:
> On 2014-01-16 14:41, Johannes Berg wrote:
> > On Thu, 2014-01-16 at 14:34 +0100, Pontus Fuchs wrote:
> >> When the netlink skb is exhausted split_start is left set. In the
> >> subsequent retry, with a larger buffer, the dump is continued from the
> >> failing point instead of from the beginning.
> >
> > Hmm. Maybe it should be like this instead?
> >
> > johannes
> >
> > diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> > index 138dc3b..2e08af5 100644
> > --- a/net/wireless/nl80211.c
> > +++ b/net/wireless/nl80211.c
> > @@ -1680,6 +1680,7 @@ static int nl80211_dump_wiphy(struct sk_buff *skb, struct netlink_callback *cb)
> >   				    !skb->len &&
> >   				    cb->min_dump_alloc < 4096) {
> >   					cb->min_dump_alloc = 4096;
> > +					state->split_start = 0;
> >   					rtnl_unlock();
> >   					return 1;
> >   				}
> 
> Will that not cause problem if split scan is in use and one of the split 
> chunks needs to be resent due to EMSGSIZE?

That's not supposed to happen, and the "!skb->len" check should catch
it.

johannes

Re: [PATCH] nl80211: Reset split_start on error path when split dump is not used

[Pontus Fuchs]

On 2014-01-16 14:53, Johannes Berg wrote:

>>> @@ -1680,6 +1680,7 @@ static int nl80211_dump_wiphy(struct sk_buff *skb, struct netlink_callback *cb)
>>>    				    !skb->len &&
>>>    				    cb->min_dump_alloc < 4096) {
>>>    					cb->min_dump_alloc = 4096;
>>> +					state->split_start = 0;
>>>    					rtnl_unlock();
>>>    					return 1;
>>>    				}
>>
>> Will that not cause problem if split scan is in use and one of the split
>> chunks needs to be resent due to EMSGSIZE?
>
> That's not supposed to happen, and the "!skb->len" check should catch
> it.
>
> johannes

Ok. Thanks. Lets go for that version then.

//Pontus