On 5/6/2024 9:03 PM, samasth.norway.ananda@xxxxxxxxxx wrote:
Hi,I came across an issue reported by an internal static analysis tool when tested on the code for brcm80211 driver.The commit which introduced is - 5b435de0d786869c95d1962121af0d7df2542009 ("net: wireless: add brcm80211 drivers") In the file - drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c within the function wlc_lcnphy_tx_iqlo_cal() we assign value to band_idx as below band_idx = (CHSPEC_IS5G(pi->radio_chanspec) ? 1 : 0); From this band_idx could be either 1 or 0. But when we look at the array iqcal_gainparams_numgains_lcnphy[] at drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c We can notice that it is initialized with only one value in it. static const u16 iqcal_gainparams_numgains_lcnphy[1] = { ARRAY_SIZE(tbl_iqcal_gainparams_lcnphy_2G), }; So, when we try to access iqcal_gainparams_numgains_lcnphy[band_idx]within the for loop in the same function we could be reading a wrong value of iqcal_gainparams_numgains_lcnphy[1]Do you have any suggestion on how we can resolve this? Is it possible to keep band_idx to just 0?
Hi Samasth,Did some digging through the code and the only device for which the LCN phy code is used is the BCM4313 which is a 2G-only device. Hence the band_idx in the code you pointed out will never be 1. My suggestion would be to get rid on band_idx and always use 0 instead and add:
if (WARN_ON(CHSPEC_IS5G(pi->radio_chanspec))) return; before the kmalloc_array() call at the beginning of the function. Regards, Arend
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
