Legacy rates are not validated while configuring
tx rateset using iw. So below cmd is accepted by nl80211.
sudo iw wlan2 set bitrates legacy-2.4 3
Validate legacy rates and return
error while configuring improper rates.
Signed-off-by: Bala Shanmugam <[email protected]>
---
net/wireless/nl80211.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index e49da27..cc7bef7 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -5540,6 +5540,8 @@ static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
sband,
nla_data(tb[NL80211_TXRATE_LEGACY]),
nla_len(tb[NL80211_TXRATE_LEGACY]));
+ if (mask.control[band].legacy == 0)
+ return -EINVAL;
}
if (tb[NL80211_TXRATE_MCS]) {
if (!ht_rateset_to_mask(
--
1.7.4.1
On 04/16/2012 11:01 PM, Johannes Berg wrote:
> On 4/12/2012 11:22 PM, Bala Shanmugam wrote:
>> Legacy rates are not validated while configuring
>> tx rateset using iw. So below cmd is accepted by nl80211.
>> sudo iw wlan2 set bitrates legacy-2.4 3
>>
>> Validate legacy rates and return
>> error while configuring improper rates.
>>
>> Signed-off-by: Bala Shanmugam<[email protected]>
>> ---
>> net/wireless/nl80211.c | 2 ++
>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
>> index e49da27..cc7bef7 100644
>> --- a/net/wireless/nl80211.c
>> +++ b/net/wireless/nl80211.c
>> @@ -5540,6 +5540,8 @@ static int nl80211_set_tx_bitrate_mask(struct
>> sk_buff *skb,
>> sband,
>> nla_data(tb[NL80211_TXRATE_LEGACY]),
>> nla_len(tb[NL80211_TXRATE_LEGACY]));
>> + if (mask.control[band].legacy == 0)
>> + return -EINVAL;
>
> I don't think this is right since it would also reject an
> intentionally empty set.
>
> johannes
This check comes into picture only when legacy rates are configured.
For intentionally empty set "if (tb[NL80211_TXRATE_LEGACY])" condition
will fail and control will not come here. I tested the same.
With this patch
"sudo iw wlan0 set bitrates mcs-5 6" executes without any error
message.
"sudo iw wlan0 set bitrates legacy-2.4 3" fails with error (command
failed: Invalid argument (-22))
Without this patch
"sudo iw wlan0 set bitrates legacy-2.4 3" executes without any
error message
Regards,
Bala.
On 04/17/2012 07:29 PM, Johannes Berg wrote:
> On 4/17/2012 1:07 AM, Bala Shanmugam wrote:
>> On 04/16/2012 11:01 PM, Johannes Berg wrote:
>>> On 4/12/2012 11:22 PM, Bala Shanmugam wrote:
>>>> Legacy rates are not validated while configuring
>>>> tx rateset using iw. So below cmd is accepted by nl80211.
>>>> sudo iw wlan2 set bitrates legacy-2.4 3
>>>>
>>>> Validate legacy rates and return
>>>> error while configuring improper rates.
>>>>
>>>> Signed-off-by: Bala Shanmugam<[email protected]>
>>>> ---
>>>> net/wireless/nl80211.c | 2 ++
>>>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
>>>> index e49da27..cc7bef7 100644
>>>> --- a/net/wireless/nl80211.c
>>>> +++ b/net/wireless/nl80211.c
>>>> @@ -5540,6 +5540,8 @@ static int nl80211_set_tx_bitrate_mask(struct
>>>> sk_buff *skb,
>>>> sband,
>>>> nla_data(tb[NL80211_TXRATE_LEGACY]),
>>>> nla_len(tb[NL80211_TXRATE_LEGACY]));
>>>> + if (mask.control[band].legacy == 0)
>>>> + return -EINVAL;
>>>
>>> I don't think this is right since it would also reject an
>>> intentionally empty set.
>
>> This check comes into picture only when legacy rates are configured.
>> For intentionally empty set "if (tb[NL80211_TXRATE_LEGACY])" condition
>> will fail and control will not come here. I tested the same.
>> With this patch
>> "sudo iw wlan0 set bitrates mcs-5 6" executes without any error message.
>
> However, that means "don't change legacy settings", not "empty legacy
> rateset".
>
> johannes
Good catch.
I didn't think of below configuration which is to specify empty legacy
rateset for 2.4 band.
sudo iw wlan0 set bitrates legacy-2.4
Correct me if I am wrong.
Anyway we need to reject invalid legacy rates and below additional check
will allow empty rateset
and block invalid rates
if (mask.control[band].legacy == 0 && nla_len(tb[NL80211_TXRATE_LEGACY]))
return -EINVAL;
Will send new patch.
Regards,
Bala.
On 4/17/2012 1:07 AM, Bala Shanmugam wrote:
> On 04/16/2012 11:01 PM, Johannes Berg wrote:
>> On 4/12/2012 11:22 PM, Bala Shanmugam wrote:
>>> Legacy rates are not validated while configuring
>>> tx rateset using iw. So below cmd is accepted by nl80211.
>>> sudo iw wlan2 set bitrates legacy-2.4 3
>>>
>>> Validate legacy rates and return
>>> error while configuring improper rates.
>>>
>>> Signed-off-by: Bala Shanmugam<[email protected]>
>>> ---
>>> net/wireless/nl80211.c | 2 ++
>>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
>>> index e49da27..cc7bef7 100644
>>> --- a/net/wireless/nl80211.c
>>> +++ b/net/wireless/nl80211.c
>>> @@ -5540,6 +5540,8 @@ static int nl80211_set_tx_bitrate_mask(struct
>>> sk_buff *skb,
>>> sband,
>>> nla_data(tb[NL80211_TXRATE_LEGACY]),
>>> nla_len(tb[NL80211_TXRATE_LEGACY]));
>>> + if (mask.control[band].legacy == 0)
>>> + return -EINVAL;
>>
>> I don't think this is right since it would also reject an
>> intentionally empty set.
> This check comes into picture only when legacy rates are configured.
> For intentionally empty set "if (tb[NL80211_TXRATE_LEGACY])" condition
> will fail and control will not come here. I tested the same.
> With this patch
> "sudo iw wlan0 set bitrates mcs-5 6" executes without any error message.
However, that means "don't change legacy settings", not "empty legacy
rateset".
johannes
On 4/17/2012 11:03 PM, Bala Shanmugam wrote:
> On 04/17/2012 07:29 PM, Johannes Berg wrote:
>> On 4/17/2012 1:07 AM, Bala Shanmugam wrote:
>>> On 04/16/2012 11:01 PM, Johannes Berg wrote:
>>>> On 4/12/2012 11:22 PM, Bala Shanmugam wrote:
>>>>> Legacy rates are not validated while configuring
>>>>> tx rateset using iw. So below cmd is accepted by nl80211.
>>>>> sudo iw wlan2 set bitrates legacy-2.4 3
>>>>>
>>>>> Validate legacy rates and return
>>>>> error while configuring improper rates.
>>>>>
>>>>> Signed-off-by: Bala Shanmugam<[email protected]>
>>>>> ---
>>>>> net/wireless/nl80211.c | 2 ++
>>>>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>>>>
>>>>> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
>>>>> index e49da27..cc7bef7 100644
>>>>> --- a/net/wireless/nl80211.c
>>>>> +++ b/net/wireless/nl80211.c
>>>>> @@ -5540,6 +5540,8 @@ static int nl80211_set_tx_bitrate_mask(struct
>>>>> sk_buff *skb,
>>>>> sband,
>>>>> nla_data(tb[NL80211_TXRATE_LEGACY]),
>>>>> nla_len(tb[NL80211_TXRATE_LEGACY]));
>>>>> + if (mask.control[band].legacy == 0)
>>>>> + return -EINVAL;
>>>>
>>>> I don't think this is right since it would also reject an
>>>> intentionally empty set.
>>
>>> This check comes into picture only when legacy rates are configured.
>>> For intentionally empty set "if (tb[NL80211_TXRATE_LEGACY])" condition
>>> will fail and control will not come here. I tested the same.
>>> With this patch
>>> "sudo iw wlan0 set bitrates mcs-5 6" executes without any error message.
>>
>> However, that means "don't change legacy settings", not "empty legacy
>> rateset".
>>
>> johannes
> Good catch.
> I didn't think of below configuration which is to specify empty legacy
> rateset for 2.4 band.
> sudo iw wlan0 set bitrates legacy-2.4
> Correct me if I am wrong.
>
> Anyway we need to reject invalid legacy rates and below additional check
> will allow empty rateset
> and block invalid rates
> if (mask.control[band].legacy == 0 && nla_len(tb[NL80211_TXRATE_LEGACY]))
> return -EINVAL;
But then you can still say
sudo iw wlan0 set bitrates legacy-2.4 1 2 3 4
and have it be accepted which is wrong as well.
You really should verify every single rate.
johannes
On 4/12/2012 11:22 PM, Bala Shanmugam wrote:
> Legacy rates are not validated while configuring
> tx rateset using iw. So below cmd is accepted by nl80211.
> sudo iw wlan2 set bitrates legacy-2.4 3
>
> Validate legacy rates and return
> error while configuring improper rates.
>
> Signed-off-by: Bala Shanmugam<[email protected]>
> ---
> net/wireless/nl80211.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index e49da27..cc7bef7 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -5540,6 +5540,8 @@ static int nl80211_set_tx_bitrate_mask(struct sk_buff *skb,
> sband,
> nla_data(tb[NL80211_TXRATE_LEGACY]),
> nla_len(tb[NL80211_TXRATE_LEGACY]));
> + if (mask.control[band].legacy == 0)
> + return -EINVAL;
I don't think this is right since it would also reject an intentionally
empty set.
johannes
On 04/18/2012 07:55 PM, Johannes Berg wrote:
> On 4/17/2012 11:03 PM, Bala Shanmugam wrote:
>> On 04/17/2012 07:29 PM, Johannes Berg wrote:
>>> On 4/17/2012 1:07 AM, Bala Shanmugam wrote:
>>>> On 04/16/2012 11:01 PM, Johannes Berg wrote:
>>>>> On 4/12/2012 11:22 PM, Bala Shanmugam wrote:
>>>>>> Legacy rates are not validated while configuring
>>>>>> tx rateset using iw. So below cmd is accepted by nl80211.
>>>>>> sudo iw wlan2 set bitrates legacy-2.4 3
>>>>>>
>>>>>> Validate legacy rates and return
>>>>>> error while configuring improper rates.
>>>>>>
>>>>>> Signed-off-by: Bala Shanmugam<[email protected]>
>>>>>> ---
>>>>>> net/wireless/nl80211.c | 2 ++
>>>>>> 1 files changed, 2 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
>>>>>> index e49da27..cc7bef7 100644
>>>>>> --- a/net/wireless/nl80211.c
>>>>>> +++ b/net/wireless/nl80211.c
>>>>>> @@ -5540,6 +5540,8 @@ static int nl80211_set_tx_bitrate_mask(struct
>>>>>> sk_buff *skb,
>>>>>> sband,
>>>>>> nla_data(tb[NL80211_TXRATE_LEGACY]),
>>>>>> nla_len(tb[NL80211_TXRATE_LEGACY]));
>>>>>> + if (mask.control[band].legacy == 0)
>>>>>> + return -EINVAL;
>>>>>
>>>>> I don't think this is right since it would also reject an
>>>>> intentionally empty set.
>>>
>>>> This check comes into picture only when legacy rates are configured.
>>>> For intentionally empty set "if (tb[NL80211_TXRATE_LEGACY])" condition
>>>> will fail and control will not come here. I tested the same.
>>>> With this patch
>>>> "sudo iw wlan0 set bitrates mcs-5 6" executes without any error
>>>> message.
>>>
>>> However, that means "don't change legacy settings", not "empty legacy
>>> rateset".
>>>
>>> johannes
>> Good catch.
>> I didn't think of below configuration which is to specify empty legacy
>> rateset for 2.4 band.
>> sudo iw wlan0 set bitrates legacy-2.4
>> Correct me if I am wrong.
>>
>> Anyway we need to reject invalid legacy rates and below additional check
>> will allow empty rateset
>> and block invalid rates
>> if (mask.control[band].legacy == 0 &&
>> nla_len(tb[NL80211_TXRATE_LEGACY]))
>> return -EINVAL;
>
> But then you can still say
>
> sudo iw wlan0 set bitrates legacy-2.4 1 2 3 4
>
> and have it be accepted which is wrong as well.
>
> You really should verify every single rate.
>
> johannes
rateset_to_mask function will return zero if it couldn't find any of the
rates in the rateset
or for empty rateset.
mask.control[band].legacy is return value of rateset_to_mask, so above
command you mentioned will
not be accepted. I tested the same.
Regards,
Bala.