Messages in this thread | | | Date | Mon, 08 Oct 2018 12:08:06 +0530 | From | Anilkumar Kolli <> | Subject | Re: [PATCH] ath10k: htt_rx: Fix signedness bug in ath10k_update_per_peer_tx_stats |
| |
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.
| |