Re: [PATCH] cfg80211: Validate legacy rateset.

[Johannes Berg <johannes@sipsolutions.net>]

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