* [PATCH] cfg80211: Validate legacy rateset.
@ 2012-04-13 6:22 Bala Shanmugam
2012-04-16 17:31 ` Johannes Berg
0 siblings, 1 reply; 7+ messages in thread
From: Bala Shanmugam @ 2012-04-13 6:22 UTC (permalink / raw)
To: linville; +Cc: linux-wireless
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 <bkamatch@qca.qualcomm.com>
---
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] cfg80211: Validate legacy rateset.
2012-04-13 6:22 [PATCH] cfg80211: Validate legacy rateset Bala Shanmugam
@ 2012-04-16 17:31 ` Johannes Berg
2012-04-17 8:07 ` Bala Shanmugam
0 siblings, 1 reply; 7+ messages in thread
From: Johannes Berg @ 2012-04-16 17:31 UTC (permalink / raw)
To: Bala Shanmugam; +Cc: linville, linux-wireless
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<bkamatch@qca.qualcomm.com>
> ---
> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cfg80211: Validate legacy rateset.
2012-04-16 17:31 ` Johannes Berg
@ 2012-04-17 8:07 ` Bala Shanmugam
2012-04-17 13:59 ` Johannes Berg
0 siblings, 1 reply; 7+ messages in thread
From: Bala Shanmugam @ 2012-04-17 8:07 UTC (permalink / raw)
To: Johannes Berg; +Cc: linville, linux-wireless
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<bkamatch@qca.qualcomm.com>
>> ---
>> 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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cfg80211: Validate legacy rateset.
2012-04-17 8:07 ` Bala Shanmugam
@ 2012-04-17 13:59 ` Johannes Berg
2012-04-18 6:03 ` Bala Shanmugam
0 siblings, 1 reply; 7+ messages in thread
From: Johannes Berg @ 2012-04-17 13:59 UTC (permalink / raw)
To: Bala Shanmugam; +Cc: linville, linux-wireless
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<bkamatch@qca.qualcomm.com>
>>> ---
>>> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cfg80211: Validate legacy rateset.
2012-04-17 13:59 ` Johannes Berg
@ 2012-04-18 6:03 ` Bala Shanmugam
2012-04-18 14:25 ` Johannes Berg
0 siblings, 1 reply; 7+ messages in thread
From: Bala Shanmugam @ 2012-04-18 6:03 UTC (permalink / raw)
To: Johannes Berg; +Cc: linville, linux-wireless
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<bkamatch@qca.qualcomm.com>
>>>> ---
>>>> 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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cfg80211: Validate legacy rateset.
2012-04-18 6:03 ` Bala Shanmugam
@ 2012-04-18 14:25 ` Johannes Berg
2012-04-19 14:43 ` Bala Shanmugam
0 siblings, 1 reply; 7+ messages in thread
From: Johannes Berg @ 2012-04-18 14:25 UTC (permalink / raw)
To: Bala Shanmugam; +Cc: linville, linux-wireless
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<bkamatch@qca.qualcomm.com>
>>>>> ---
>>>>> 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cfg80211: Validate legacy rateset.
2012-04-18 14:25 ` Johannes Berg
@ 2012-04-19 14:43 ` Bala Shanmugam
0 siblings, 0 replies; 7+ messages in thread
From: Bala Shanmugam @ 2012-04-19 14:43 UTC (permalink / raw)
To: Johannes Berg; +Cc: linville, linux-wireless
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<bkamatch@qca.qualcomm.com>
>>>>>> ---
>>>>>> 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.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-04-19 14:43 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-04-13 6:22 [PATCH] cfg80211: Validate legacy rateset Bala Shanmugam
2012-04-16 17:31 ` Johannes Berg
2012-04-17 8:07 ` Bala Shanmugam
2012-04-17 13:59 ` Johannes Berg
2012-04-18 6:03 ` Bala Shanmugam
2012-04-18 14:25 ` Johannes Berg
2012-04-19 14:43 ` Bala Shanmugam
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).