All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
@ 2018-10-05 18:42 ` Gustavo A. R. Silva
  0 siblings, 0 replies; 19+ messages in thread
From: Gustavo A. R. Silva @ 2018-10-05 18:42 UTC (permalink / raw)
  To: Kalle Valo, David S. Miller
  Cc: ath10k, linux-wireless, netdev, linux-kernel, Gustavo A. R. Silva

Currently, the error handling for the call to function
ath10k_get_legacy_rate_idx() doesn't work because
*rate_idx* is of type u8 (8 bits, unsigned), which
makes it impossible for it to hold a value less
than 0.

Fix this by changing the type of variable *rate_idx*
to s8 (8 bits, signed).

Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate table")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index f240525..edd0e74 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
 				struct ath10k_per_peer_tx_stats *peer_stats)
 {
 	struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
-	u8 rate = 0, rate_idx = 0, sgi;
+	u8 rate = 0, sgi;
+	s8 rate_idx = 0;
 	struct rate_info txrate;
 
 	lockdep_assert_held(&ar->data_lock);
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
@ 2018-10-05 18:42 ` Gustavo A. R. Silva
  0 siblings, 0 replies; 19+ messages in thread
From: Gustavo A. R. Silva @ 2018-10-05 18:42 UTC (permalink / raw)
  To: Kalle Valo, David S. Miller
  Cc: netdev, linux-wireless, linux-kernel, ath10k, Gustavo A. R. Silva

Currently, the error handling for the call to function
ath10k_get_legacy_rate_idx() doesn't work because
*rate_idx* is of type u8 (8 bits, unsigned), which
makes it impossible for it to hold a value less
than 0.

Fix this by changing the type of variable *rate_idx*
to s8 (8 bits, signed).

Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate table")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index f240525..edd0e74 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
 				struct ath10k_per_peer_tx_stats *peer_stats)
 {
 	struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
-	u8 rate = 0, rate_idx = 0, sgi;
+	u8 rate = 0, sgi;
+	s8 rate_idx = 0;
 	struct rate_info txrate;
 
 	lockdep_assert_held(&ar->data_lock);
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
@ 2018-10-05 18:42 ` Gustavo A. R. Silva
  0 siblings, 0 replies; 19+ messages in thread
From: Gustavo A. R. Silva @ 2018-10-05 18:42 UTC (permalink / raw)
  To: Kalle Valo, David S. Miller
  Cc: netdev, linux-wireless, linux-kernel, ath10k, Gustavo A. R. Silva

Currently, the error handling for the call to function
ath10k_get_legacy_rate_idx() doesn't work because
*rate_idx* is of type u8 (8 bits, unsigned), which
makes it impossible for it to hold a value less
than 0.

Fix this by changing the type of variable *rate_idx*
to s8 (8 bits, signed).

Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate table")
Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index f240525..edd0e74 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
 				struct ath10k_per_peer_tx_stats *peer_stats)
 {
 	struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
-	u8 rate = 0, rate_idx = 0, sgi;
+	u8 rate = 0, sgi;
+	s8 rate_idx = 0;
 	struct rate_info txrate;
 
 	lockdep_assert_held(&ar->data_lock);
-- 
2.7.4


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
  2018-10-05 18:42 ` Gustavo A. R. Silva
@ 2018-10-05 19:09   ` Ben Greear
  -1 siblings, 0 replies; 19+ messages in thread
From: Ben Greear @ 2018-10-05 19:09 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Kalle Valo, David S. Miller
  Cc: ath10k, linux-wireless, netdev, linux-kernel

On 10/05/2018 11:42 AM, Gustavo A. R. Silva wrote:
> Currently, the error handling for the call to function
> ath10k_get_legacy_rate_idx() doesn't work because
> *rate_idx* is of type u8 (8 bits, unsigned), which
> makes it impossible for it to hold a value less
> than 0.
>
> Fix this by changing the type of variable *rate_idx*
> to s8 (8 bits, signed).

There are more than 127 rates, are you sure this is doing
what you want?

Thanks,
Ben

>
> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate table")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
>  drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index f240525..edd0e74 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
>  				struct ath10k_per_peer_tx_stats *peer_stats)
>  {
>  	struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
> -	u8 rate = 0, rate_idx = 0, sgi;
> +	u8 rate = 0, sgi;
> +	s8 rate_idx = 0;
>  	struct rate_info txrate;
>
>  	lockdep_assert_held(&ar->data_lock);
>


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
@ 2018-10-05 19:09   ` Ben Greear
  0 siblings, 0 replies; 19+ messages in thread
From: Ben Greear @ 2018-10-05 19:09 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Kalle Valo, David S. Miller
  Cc: netdev, linux-wireless, linux-kernel, ath10k

On 10/05/2018 11:42 AM, Gustavo A. R. Silva wrote:
> Currently, the error handling for the call to function
> ath10k_get_legacy_rate_idx() doesn't work because
> *rate_idx* is of type u8 (8 bits, unsigned), which
> makes it impossible for it to hold a value less
> than 0.
>
> Fix this by changing the type of variable *rate_idx*
> to s8 (8 bits, signed).

There are more than 127 rates, are you sure this is doing
what you want?

Thanks,
Ben

>
> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate table")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> ---
>  drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
> index f240525..edd0e74 100644
> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
>  				struct ath10k_per_peer_tx_stats *peer_stats)
>  {
>  	struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
> -	u8 rate = 0, rate_idx = 0, sgi;
> +	u8 rate = 0, sgi;
> +	s8 rate_idx = 0;
>  	struct rate_info txrate;
>
>  	lockdep_assert_held(&ar->data_lock);
>


-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
  2018-10-05 19:09   ` Ben Greear
@ 2018-10-05 19:14     ` Gustavo A. R. Silva
  -1 siblings, 0 replies; 19+ messages in thread
From: Gustavo A. R. Silva @ 2018-10-05 19:14 UTC (permalink / raw)
  To: Ben Greear, Kalle Valo, David S. Miller
  Cc: ath10k, linux-wireless, netdev, linux-kernel



On 10/5/18 9:09 PM, Ben Greear wrote:
> On 10/05/2018 11:42 AM, Gustavo A. R. Silva wrote:
>> Currently, the error handling for the call to function
>> ath10k_get_legacy_rate_idx() doesn't work because
>> *rate_idx* is of type u8 (8 bits, unsigned), which
>> makes it impossible for it to hold a value less
>> than 0.
>>
>> Fix this by changing the type of variable *rate_idx*
>> to s8 (8 bits, signed).
> 
> There are more than 127 rates, are you sure this is doing
> what you want?
> 

Based on the following function, rate_idx can only hold values from 0 to 11

static inline int ath10k_get_legacy_rate_idx(struct ath10k *ar, u8 rate)
{
        static const u8 legacy_rates[] = {1, 2, 5, 11, 6, 9, 12,
                                          18, 24, 36, 48, 54};
        int i;

        for (i = 0; i < ARRAY_SIZE(legacy_rates); i++) {
                if (rate == legacy_rates[i])
                        return i;
        }

        ath10k_warn(ar, "Invalid legacy rate %hhd peer stats", rate);
        return -EINVAL;
}

Thanks
--
Gustavo

> Thanks,
> Ben
> 
>>
>> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
>> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate table")
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>> ---
>>  drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> index f240525..edd0e74 100644
>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> @@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
>>                  struct ath10k_per_peer_tx_stats *peer_stats)
>>  {
>>      struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
>> -    u8 rate = 0, rate_idx = 0, sgi;
>> +    u8 rate = 0, sgi;
>> +    s8 rate_idx = 0;
>>      struct rate_info txrate;
>>
>>      lockdep_assert_held(&ar->data_lock);
>>
> 
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
@ 2018-10-05 19:14     ` Gustavo A. R. Silva
  0 siblings, 0 replies; 19+ messages in thread
From: Gustavo A. R. Silva @ 2018-10-05 19:14 UTC (permalink / raw)
  To: Ben Greear, Kalle Valo, David S. Miller
  Cc: netdev, linux-wireless, linux-kernel, ath10k



On 10/5/18 9:09 PM, Ben Greear wrote:
> On 10/05/2018 11:42 AM, Gustavo A. R. Silva wrote:
>> Currently, the error handling for the call to function
>> ath10k_get_legacy_rate_idx() doesn't work because
>> *rate_idx* is of type u8 (8 bits, unsigned), which
>> makes it impossible for it to hold a value less
>> than 0.
>>
>> Fix this by changing the type of variable *rate_idx*
>> to s8 (8 bits, signed).
> 
> There are more than 127 rates, are you sure this is doing
> what you want?
> 

Based on the following function, rate_idx can only hold values from 0 to 11

static inline int ath10k_get_legacy_rate_idx(struct ath10k *ar, u8 rate)
{
        static const u8 legacy_rates[] = {1, 2, 5, 11, 6, 9, 12,
                                          18, 24, 36, 48, 54};
        int i;

        for (i = 0; i < ARRAY_SIZE(legacy_rates); i++) {
                if (rate == legacy_rates[i])
                        return i;
        }

        ath10k_warn(ar, "Invalid legacy rate %hhd peer stats", rate);
        return -EINVAL;
}

Thanks
--
Gustavo

> Thanks,
> Ben
> 
>>
>> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
>> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate table")
>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>> ---
>>  drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> index f240525..edd0e74 100644
>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>> @@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
>>                  struct ath10k_per_peer_tx_stats *peer_stats)
>>  {
>>      struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
>> -    u8 rate = 0, rate_idx = 0, sgi;
>> +    u8 rate = 0, sgi;
>> +    s8 rate_idx = 0;
>>      struct rate_info txrate;
>>
>>      lockdep_assert_held(&ar->data_lock);
>>
> 
> 

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
@ 2018-10-05 19:15       ` Gustavo A. R. Silva
  0 siblings, 0 replies; 19+ messages in thread
From: Gustavo A. R. Silva @ 2018-10-05 19:15 UTC (permalink / raw)
  To: Ben Greear, Kalle Valo, David S. Miller
  Cc: ath10k, linux-wireless, netdev, linux-kernel



On 10/5/18 9:14 PM, Gustavo A. R. Silva wrote:
> 
> 
> On 10/5/18 9:09 PM, Ben Greear wrote:
>> On 10/05/2018 11:42 AM, Gustavo A. R. Silva wrote:
>>> Currently, the error handling for the call to function
>>> ath10k_get_legacy_rate_idx() doesn't work because
>>> *rate_idx* is of type u8 (8 bits, unsigned), which
>>> makes it impossible for it to hold a value less
>>> than 0.
>>>
>>> Fix this by changing the type of variable *rate_idx*
>>> to s8 (8 bits, signed).
>>
>> There are more than 127 rates, are you sure this is doing
>> what you want?
>>
> 
> Based on the following function, rate_idx can only hold values from 0 to 11
> 

... and of course -EINVAL too

> static inline int ath10k_get_legacy_rate_idx(struct ath10k *ar, u8 rate)
> {
>         static const u8 legacy_rates[] = {1, 2, 5, 11, 6, 9, 12,
>                                           18, 24, 36, 48, 54};
>         int i;
> 
>         for (i = 0; i < ARRAY_SIZE(legacy_rates); i++) {
>                 if (rate == legacy_rates[i])
>                         return i;
>         }
> 
>         ath10k_warn(ar, "Invalid legacy rate %hhd peer stats", rate);
>         return -EINVAL;
> }
> 
> Thanks
> --
> Gustavo
> 
>> Thanks,
>> Ben
>>
>>>
>>> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
>>> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate table")
>>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>>> ---
>>>  drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> index f240525..edd0e74 100644
>>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> @@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
>>>                  struct ath10k_per_peer_tx_stats *peer_stats)
>>>  {
>>>      struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
>>> -    u8 rate = 0, rate_idx = 0, sgi;
>>> +    u8 rate = 0, sgi;
>>> +    s8 rate_idx = 0;
>>>      struct rate_info txrate;
>>>
>>>      lockdep_assert_held(&ar->data_lock);
>>>
>>
>>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
@ 2018-10-05 19:15       ` Gustavo A. R. Silva
  0 siblings, 0 replies; 19+ messages in thread
From: Gustavo A. R. Silva @ 2018-10-05 19:15 UTC (permalink / raw)
  To: Ben Greear, Kalle Valo, David S. Miller
  Cc: ath10k-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	linux-wireless-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA



On 10/5/18 9:14 PM, Gustavo A. R. Silva wrote:
> 
> 
> On 10/5/18 9:09 PM, Ben Greear wrote:
>> On 10/05/2018 11:42 AM, Gustavo A. R. Silva wrote:
>>> Currently, the error handling for the call to function
>>> ath10k_get_legacy_rate_idx() doesn't work because
>>> *rate_idx* is of type u8 (8 bits, unsigned), which
>>> makes it impossible for it to hold a value less
>>> than 0.
>>>
>>> Fix this by changing the type of variable *rate_idx*
>>> to s8 (8 bits, signed).
>>
>> There are more than 127 rates, are you sure this is doing
>> what you want?
>>
> 
> Based on the following function, rate_idx can only hold values from 0 to 11
> 

... and of course -EINVAL too

> static inline int ath10k_get_legacy_rate_idx(struct ath10k *ar, u8 rate)
> {
>         static const u8 legacy_rates[] = {1, 2, 5, 11, 6, 9, 12,
>                                           18, 24, 36, 48, 54};
>         int i;
> 
>         for (i = 0; i < ARRAY_SIZE(legacy_rates); i++) {
>                 if (rate == legacy_rates[i])
>                         return i;
>         }
> 
>         ath10k_warn(ar, "Invalid legacy rate %hhd peer stats", rate);
>         return -EINVAL;
> }
> 
> Thanks
> --
> Gustavo
> 
>> Thanks,
>> Ben
>>
>>>
>>> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
>>> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate table")
>>> Signed-off-by: Gustavo A. R. Silva <gustavo-L1vi/lXTdts+Va1GwOuvDg@public.gmane.org>
>>> ---
>>>  drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> index f240525..edd0e74 100644
>>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> @@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
>>>                  struct ath10k_per_peer_tx_stats *peer_stats)
>>>  {
>>>      struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
>>> -    u8 rate = 0, rate_idx = 0, sgi;
>>> +    u8 rate = 0, sgi;
>>> +    s8 rate_idx = 0;
>>>      struct rate_info txrate;
>>>
>>>      lockdep_assert_held(&ar->data_lock);
>>>
>>
>>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
@ 2018-10-05 19:15       ` Gustavo A. R. Silva
  0 siblings, 0 replies; 19+ messages in thread
From: Gustavo A. R. Silva @ 2018-10-05 19:15 UTC (permalink / raw)
  To: Ben Greear, Kalle Valo, David S. Miller
  Cc: netdev, linux-wireless, linux-kernel, ath10k



On 10/5/18 9:14 PM, Gustavo A. R. Silva wrote:
> 
> 
> On 10/5/18 9:09 PM, Ben Greear wrote:
>> On 10/05/2018 11:42 AM, Gustavo A. R. Silva wrote:
>>> Currently, the error handling for the call to function
>>> ath10k_get_legacy_rate_idx() doesn't work because
>>> *rate_idx* is of type u8 (8 bits, unsigned), which
>>> makes it impossible for it to hold a value less
>>> than 0.
>>>
>>> Fix this by changing the type of variable *rate_idx*
>>> to s8 (8 bits, signed).
>>
>> There are more than 127 rates, are you sure this is doing
>> what you want?
>>
> 
> Based on the following function, rate_idx can only hold values from 0 to 11
> 

... and of course -EINVAL too

> static inline int ath10k_get_legacy_rate_idx(struct ath10k *ar, u8 rate)
> {
>         static const u8 legacy_rates[] = {1, 2, 5, 11, 6, 9, 12,
>                                           18, 24, 36, 48, 54};
>         int i;
> 
>         for (i = 0; i < ARRAY_SIZE(legacy_rates); i++) {
>                 if (rate == legacy_rates[i])
>                         return i;
>         }
> 
>         ath10k_warn(ar, "Invalid legacy rate %hhd peer stats", rate);
>         return -EINVAL;
> }
> 
> Thanks
> --
> Gustavo
> 
>> Thanks,
>> Ben
>>
>>>
>>> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
>>> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate table")
>>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>>> ---
>>>  drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> index f240525..edd0e74 100644
>>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>> @@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k *ar,
>>>                  struct ath10k_per_peer_tx_stats *peer_stats)
>>>  {
>>>      struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
>>> -    u8 rate = 0, rate_idx = 0, sgi;
>>> +    u8 rate = 0, sgi;
>>> +    s8 rate_idx = 0;
>>>      struct rate_info txrate;
>>>
>>>      lockdep_assert_held(&ar->data_lock);
>>>
>>
>>

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
  2018-10-05 19:15       ` Gustavo A. R. Silva
@ 2018-10-08  6:38         ` Anilkumar Kolli
  -1 siblings, 0 replies; 19+ messages in thread
From: Anilkumar Kolli @ 2018-10-08  6:38 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Ben Greear, Kalle Valo, David S. Miller, ath10k, linux-wireless,
	netdev, linux-kernel, linux-wireless-owner

On 2018-10-06 00:45, Gustavo A. R. Silva wrote:
> On 10/5/18 9:14 PM, Gustavo A. R. Silva wrote:
>> 
>> 
>> On 10/5/18 9:09 PM, Ben Greear wrote:
>>> On 10/05/2018 11:42 AM, Gustavo A. R. Silva wrote:
>>>> Currently, the error handling for the call to function
>>>> ath10k_get_legacy_rate_idx() doesn't work because
>>>> *rate_idx* is of type u8 (8 bits, unsigned), which
>>>> makes it impossible for it to hold a value less
>>>> than 0.
>>>> 
>>>> Fix this by changing the type of variable *rate_idx*
>>>> to s8 (8 bits, signed).
>>> 
>>> There are more than 127 rates, are you sure this is doing
>>> what you want?
>>> 
>> 
>> Based on the following function, rate_idx can only hold values from 0 
>> to 11
>> 
> 
> ... and of course -EINVAL too
> 
>> static inline int ath10k_get_legacy_rate_idx(struct ath10k *ar, u8 
>> rate)
>> {
>>         static const u8 legacy_rates[] = {1, 2, 5, 11, 6, 9, 12,
>>                                           18, 24, 36, 48, 54};
>>         int i;
>> 
>>         for (i = 0; i < ARRAY_SIZE(legacy_rates); i++) {
>>                 if (rate == legacy_rates[i])
>>                         return i;
>>         }
>> 
>>         ath10k_warn(ar, "Invalid legacy rate %hhd peer stats", rate);
>>         return -EINVAL;
>> }
>> 
>> Thanks
>> --
>> Gustavo
>> 
>>> Thanks,
>>> Ben
>>> 
>>>> 
>>>> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
>>>> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update 
>>>> the txrate table")
>>>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>>>> ---
>>>>  drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>> 
>>>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
>>>> b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>>> index f240525..edd0e74 100644
>>>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>>>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>>> @@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k 
>>>> *ar,
>>>>                  struct ath10k_per_peer_tx_stats *peer_stats)
>>>>  {
>>>>      struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
>>>> -    u8 rate = 0, rate_idx = 0, sgi;
>>>> +    u8 rate = 0, sgi;
>>>> +    s8 rate_idx = 0;
>>>>      struct rate_info txrate;
>>>> 
>>>>      lockdep_assert_held(&ar->data_lock);
>>>> 
>>> 
>>> 

I have sent a patch to address this,
https://patchwork.kernel.org/patch/10611943/

Thanks
Anil.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
@ 2018-10-08  6:38         ` Anilkumar Kolli
  0 siblings, 0 replies; 19+ messages in thread
From: Anilkumar Kolli @ 2018-10-08  6:38 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: linux-wireless-owner, netdev, linux-wireless, linux-kernel,
	ath10k, Ben Greear, David S. Miller, Kalle Valo

On 2018-10-06 00:45, Gustavo A. R. Silva wrote:
> On 10/5/18 9:14 PM, Gustavo A. R. Silva wrote:
>> 
>> 
>> On 10/5/18 9:09 PM, Ben Greear wrote:
>>> On 10/05/2018 11:42 AM, Gustavo A. R. Silva wrote:
>>>> Currently, the error handling for the call to function
>>>> ath10k_get_legacy_rate_idx() doesn't work because
>>>> *rate_idx* is of type u8 (8 bits, unsigned), which
>>>> makes it impossible for it to hold a value less
>>>> than 0.
>>>> 
>>>> Fix this by changing the type of variable *rate_idx*
>>>> to s8 (8 bits, signed).
>>> 
>>> There are more than 127 rates, are you sure this is doing
>>> what you want?
>>> 
>> 
>> Based on the following function, rate_idx can only hold values from 0 
>> to 11
>> 
> 
> ... and of course -EINVAL too
> 
>> static inline int ath10k_get_legacy_rate_idx(struct ath10k *ar, u8 
>> rate)
>> {
>>         static const u8 legacy_rates[] = {1, 2, 5, 11, 6, 9, 12,
>>                                           18, 24, 36, 48, 54};
>>         int i;
>> 
>>         for (i = 0; i < ARRAY_SIZE(legacy_rates); i++) {
>>                 if (rate == legacy_rates[i])
>>                         return i;
>>         }
>> 
>>         ath10k_warn(ar, "Invalid legacy rate %hhd peer stats", rate);
>>         return -EINVAL;
>> }
>> 
>> Thanks
>> --
>> Gustavo
>> 
>>> Thanks,
>>> Ben
>>> 
>>>> 
>>>> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
>>>> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update 
>>>> the txrate table")
>>>> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
>>>> ---
>>>>  drivers/net/wireless/ath/ath10k/htt_rx.c | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>> 
>>>> diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c 
>>>> b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>>> index f240525..edd0e74 100644
>>>> --- a/drivers/net/wireless/ath/ath10k/htt_rx.c
>>>> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
>>>> @@ -2753,7 +2753,8 @@ ath10k_update_per_peer_tx_stats(struct ath10k 
>>>> *ar,
>>>>                  struct ath10k_per_peer_tx_stats *peer_stats)
>>>>  {
>>>>      struct ath10k_sta *arsta = (struct ath10k_sta *)sta->drv_priv;
>>>> -    u8 rate = 0, rate_idx = 0, sgi;
>>>> +    u8 rate = 0, sgi;
>>>> +    s8 rate_idx = 0;
>>>>      struct rate_info txrate;
>>>> 
>>>>      lockdep_assert_held(&ar->data_lock);
>>>> 
>>> 
>>> 

I have sent a patch to address this,
https://patchwork.kernel.org/patch/10611943/

Thanks
Anil.

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
  2018-10-08  6:38         ` Anilkumar Kolli
@ 2018-10-09 19:19           ` Gustavo A. R. Silva
  -1 siblings, 0 replies; 19+ messages in thread
From: Gustavo A. R. Silva @ 2018-10-09 19:19 UTC (permalink / raw)
  To: Anilkumar Kolli
  Cc: Ben Greear, Kalle Valo, David S. Miller, ath10k, linux-wireless,
	netdev, linux-kernel, linux-wireless-owner


> 
> I have sent a patch to address this,
> https://patchwork.kernel.org/patch/10611943/
> 

That's great.

Thanks for letting me know. :)

--
Gustavo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
@ 2018-10-09 19:19           ` Gustavo A. R. Silva
  0 siblings, 0 replies; 19+ messages in thread
From: Gustavo A. R. Silva @ 2018-10-09 19:19 UTC (permalink / raw)
  To: Anilkumar Kolli
  Cc: linux-wireless-owner, netdev, linux-wireless, linux-kernel,
	ath10k, Ben Greear, David S. Miller, Kalle Valo


> 
> I have sent a patch to address this,
> https://patchwork.kernel.org/patch/10611943/
> 

That's great.

Thanks for letting me know. :)

--
Gustavo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
  2018-10-05 18:42 ` Gustavo A. R. Silva
@ 2018-10-13 17:23   ` Kalle Valo
  -1 siblings, 0 replies; 19+ messages in thread
From: Kalle Valo @ 2018-10-13 17:23 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Gustavo A. R. Silva, netdev, linux-wireless, linux-kernel,
	ath10k, David S. Miller

"Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote:

> Currently, the error handling for the call to function
> ath10k_get_legacy_rate_idx() doesn't work because
> *rate_idx* is of type u8 (8 bits, unsigned), which
> makes it impossible for it to hold a value less
> than 0.
> 
> Fix this by changing the type of variable *rate_idx*
> to s8 (8 bits, signed).
> 
> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate table")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

Patch applied to ath-next branch of ath.git, thanks.

9d9cdbf3f9ed ath10k: htt_rx: fix signedness bug in ath10k_update_per_peer_tx_stats

-- 
https://patchwork.kernel.org/patch/10628675/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
@ 2018-10-13 17:23   ` Kalle Valo
  0 siblings, 0 replies; 19+ messages in thread
From: Kalle Valo @ 2018-10-13 17:23 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: netdev, linux-wireless, linux-kernel, ath10k, David S. Miller

"Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote:

> Currently, the error handling for the call to function
> ath10k_get_legacy_rate_idx() doesn't work because
> *rate_idx* is of type u8 (8 bits, unsigned), which
> makes it impossible for it to hold a value less
> than 0.
> 
> Fix this by changing the type of variable *rate_idx*
> to s8 (8 bits, signed).
> 
> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate table")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

Patch applied to ath-next branch of ath.git, thanks.

9d9cdbf3f9ed ath10k: htt_rx: fix signedness bug in ath10k_update_per_peer_tx_stats

-- 
https://patchwork.kernel.org/patch/10628675/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
  2018-10-05 18:42 ` Gustavo A. R. Silva
                   ` (3 preceding siblings ...)
  (?)
@ 2018-10-13 17:23 ` Kalle Valo
  -1 siblings, 0 replies; 19+ messages in thread
From: Kalle Valo @ 2018-10-13 17:23 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: David S. Miller, ath10k, linux-wireless, netdev, linux-kernel,
	Gustavo A. R. Silva

"Gustavo A. R. Silva" <gustavo@embeddedor.com> wrote:

> Currently, the error handling for the call to function
> ath10k_get_legacy_rate_idx() doesn't work because
> *rate_idx* is of type u8 (8 bits, unsigned), which
> makes it impossible for it to hold a value less
> than 0.
> 
> Fix this by changing the type of variable *rate_idx*
> to s8 (8 bits, signed).
> 
> Addresses-Coverity-ID: 1473914 ("Unsigned compared against 0")
> Fixes: 0189dbd71cbd ("ath10k: get the legacy rate index to update the txrate table")
> Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

Patch applied to ath-next branch of ath.git, thanks.

9d9cdbf3f9ed ath10k: htt_rx: fix signedness bug in ath10k_update_per_peer_tx_stats

-- 
https://patchwork.kernel.org/patch/10628675/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
       [not found] ` <20181013172330.1CC8E60BFE@smtp.codeaurora.org>
@ 2018-10-13 18:26     ` Gustavo A. R. Silva
  0 siblings, 0 replies; 19+ messages in thread
From: Gustavo A. R. Silva @ 2018-10-13 18:26 UTC (permalink / raw)
  To: Kalle Valo; +Cc: David S. Miller, ath10k, linux-wireless, netdev, linux-kernel


On 10/13/18 7:23 PM, Kalle Valo wrote:
> 
> Patch applied to ath-next branch of ath.git, thanks.
> 
> 9d9cdbf3f9ed ath10k: htt_rx: fix signedness bug in ath10k_update_per_peer_tx_stats
> 

Thank you, Kalle.
--
Gustavo

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats
@ 2018-10-13 18:26     ` Gustavo A. R. Silva
  0 siblings, 0 replies; 19+ messages in thread
From: Gustavo A. R. Silva @ 2018-10-13 18:26 UTC (permalink / raw)
  To: Kalle Valo; +Cc: netdev, linux-wireless, David S. Miller, ath10k, linux-kernel


On 10/13/18 7:23 PM, Kalle Valo wrote:
> 
> Patch applied to ath-next branch of ath.git, thanks.
> 
> 9d9cdbf3f9ed ath10k: htt_rx: fix signedness bug in ath10k_update_per_peer_tx_stats
> 

Thank you, Kalle.
--
Gustavo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2018-10-13 18:47 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-05 18:42 [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats Gustavo A. R. Silva
2018-10-05 18:42 ` Gustavo A. R. Silva
2018-10-05 18:42 ` Gustavo A. R. Silva
2018-10-05 19:09 ` Ben Greear
2018-10-05 19:09   ` Ben Greear
2018-10-05 19:14   ` Gustavo A. R. Silva
2018-10-05 19:14     ` Gustavo A. R. Silva
2018-10-05 19:15     ` Gustavo A. R. Silva
2018-10-05 19:15       ` Gustavo A. R. Silva
2018-10-05 19:15       ` Gustavo A. R. Silva
2018-10-08  6:38       ` Anilkumar Kolli
2018-10-08  6:38         ` Anilkumar Kolli
2018-10-09 19:19         ` Gustavo A. R. Silva
2018-10-09 19:19           ` Gustavo A. R. Silva
2018-10-13 17:23 ` Kalle Valo
2018-10-13 17:23   ` Kalle Valo
2018-10-13 17:23 ` Kalle Valo
     [not found] ` <20181013172330.1CC8E60BFE@smtp.codeaurora.org>
2018-10-13 18:26   ` Gustavo A. R. Silva
2018-10-13 18:26     ` Gustavo A. R. Silva

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.