[PATCH] iw: Fix bitrate output when no rate info found

[PATCH] iw: Fix bitrate output when no rate info found

[Masashi Honma]

Previously, bitrate showed uninitialized buffer when no rate info found.
This patch fixes the issue.

Signed-off-by: Masashi Honma <masashi.honma@gmail.com>
---
 station.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/station.c b/station.c
index f3e3da8..9d3eb4d 100644
--- a/station.c
+++ b/station.c
@@ -151,6 +151,10 @@ void parse_bitrate(struct nlattr *bitrate_attr, char *buf, int buflen)
 	if (rate > 0)
 		pos += snprintf(pos, buflen - (pos - buf),
 				"%d.%d MBit/s", rate / 10, rate % 10);
+	else {
+		snprintf(buf, buflen, "No rate info found!");
+		return;
+	}
 
 	if (rinfo[NL80211_RATE_INFO_MCS])
 		pos += snprintf(pos, buflen - (pos - buf),
-- 
2.7.4

Re: [PATCH] iw: Fix bitrate output when no rate info found

[Masashi Honma]

[-- Attachment #1: Type: text/plain, Size: 189 bytes --]

On 2017/02/14 14:21, Masashi Honma wrote:
> Previously, bitrate showed uninitialized buffer when no rate info found.
> This patch fixes the issue.

This is the screen shot.

Masashi Honma.

[-- Attachment #2: tx_bitrate.jpeg --]
[-- Type: image/jpeg, Size: 62242 bytes --]

Re: [PATCH] iw: Fix bitrate output when no rate info found

[Johannes Berg]

On Tue, 2017-02-14 at 14:21 +0900, Masashi Honma wrote:
> Previously, bitrate showed uninitialized buffer when no rate info
> found.

When would this happen?

I'm not really sure this is right - perhaps we don't have
RATE_INFO_BITRATE(32), but still have the MCS data?

How about we just add "(unknown)" or so and not return here?

johannes

Re: [PATCH] iw: Fix bitrate output when no rate info found

[Masashi Honma]

On 2017-02-14 17:35, Johannes Berg wrote:
> On Tue, 2017-02-14 at 14:21 +0900, Masashi Honma wrote:
>> Previously, bitrate showed uninitialized buffer when no rate info
>> found.
>
> When would this happen?

I could see in mesh STA connection with 11n and legacy mixed.
STA A has disable_ht=1.
STA B has disable_ht=0.

 > I'm not really sure this is right - perhaps we don't have
 > RATE_INFO_BITRATE(32), but still have the MCS data?

I recognized there was a issue on such a case. I will send a patch to 
wpa_supplicant. Anyway, showing string message is better than showing 
raw binary data.

> How about we just add "(unknown)" or so and not return here?

Yes. First time, I supposed to use "unknown". But in the function 
parse_bitrate(), nla_parse_nested() returns message "failed to parse 
nested rate attributes!". This explains why the bitrate is unknown. So I 
used explaining message. We could see the message like this.

tx bitrate: No rate info found!
rx bitrate: 48.0 MBit/s

Masashi Honma.

Re: [PATCH] iw: Fix bitrate output when no rate info found

[Johannes Berg]

On Tue, 2017-02-14 at 17:55 +0900, Masashi Honma wrote:
> On 2017-02-14 17:35, Johannes Berg wrote:
> > On Tue, 2017-02-14 at 14:21 +0900, Masashi Honma wrote:
> > > Previously, bitrate showed uninitialized buffer when no rate info
> > > found.
> > 
> > When would this happen?
> 
> I could see in mesh STA connection with 11n and legacy mixed.
> STA A has disable_ht=1.
> STA B has disable_ht=0.

Interesting, ok.

>  > I'm not really sure this is right - perhaps we don't have
>  > RATE_INFO_BITRATE(32), but still have the MCS data?
> 
> I recognized there was a issue on such a case. I will send a patch
> to wpa_supplicant. Anyway, showing string message is better than
> showing raw binary data.
> 
> > How about we just add "(unknown)" or so and not return here?
> 
> Yes. First time, I supposed to use "unknown". 

Ok.

> But in the function 
> parse_bitrate(), nla_parse_nested() returns message "failed to parse 
> nested rate attributes!". This explains why the bitrate is unknown. 

Yes, this is bad! But if you saw that, why did you ever get to the
below code that checked for the BITRATE(32) attributes?

> So I used explaining message. We could see the message like this.
> 
> tx bitrate: No rate info found!
> rx bitrate: 48.0 MBit/s

Yeah I was just thinking we could also see

tx bitrate: (unknown)

and then if there was MCS anyway you'd see

tx bitrate: (unknown) MCS 7

or something like that?

johannes

Re: [PATCH] iw: Fix bitrate output when no rate info found

[Masashi Honma]

On 2017-02-14 18:14, Johannes Berg wrote:
> Yes, this is bad! But if you saw that, why did you ever get to the
> below code that checked for the BITRATE(32) attributes?
>
>> So I used explaining message. We could see the message like this.
>>
>> tx bitrate: No rate info found!
>> rx bitrate: 48.0 MBit/s
>
> Yeah I was just thinking we could also see
>
> tx bitrate: (unknown)
>
> and then if there was MCS anyway you'd see
>
> tx bitrate: (unknown) MCS 7
>
> or something like that?

Yes, showing information as far as possible looks good.

I will modify this patch.

Masashi Honma.

[PATCH v2] iw: Fix bitrate output when no rate info found

[Masashi Honma]

Previously, bitrate showed uninitialized buffer when no rate info found.
This patch fixes the issue.

Signed-off-by: Masashi Honma <masashi.honma@gmail.com>
---
 station.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/station.c b/station.c
index f3e3da8..4885dc0 100644
--- a/station.c
+++ b/station.c
@@ -151,6 +151,8 @@ void parse_bitrate(struct nlattr *bitrate_attr, char *buf, int buflen)
 	if (rate > 0)
 		pos += snprintf(pos, buflen - (pos - buf),
 				"%d.%d MBit/s", rate / 10, rate % 10);
+	else
+		pos += snprintf(pos, buflen - (pos - buf), "(unknown)");
 
 	if (rinfo[NL80211_RATE_INFO_MCS])
 		pos += snprintf(pos, buflen - (pos - buf),
-- 
2.7.4

Re: [PATCH v2] iw: Fix bitrate output when no rate info found

[Johannes Berg]

On Tue, 2017-02-14 at 18:38 +0900, Masashi Honma wrote:
> Previously, bitrate showed uninitialized buffer when no rate info
> found.
> This patch fixes the issue.
> 

Applied, thanks.

johannes