Received-SPF: pass (google.com: domain of linux-wireless+bounces-7366-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <667bb700-1cef-4969-85e7-cf2e9e5be7c2@oracle.com>
Date: Wed, 8 May 2024 16:27:36 -0700
User-Agent: Mozilla Thunderbird
Subject: Re: [External] : Re: Buffer overrun error found in brcm80211 driver
 code
To: Arend van Spriel <arend.vanspriel@broadcom.com>,
        Kalle Valo <kvalo@kernel.org>
Cc: linux-wireless@vger.kernel.org,
        "John W. Linville"
 <linville@tuxdriver.com>
References: <8292faa6-a2f6-4b43-8d60-9eeba85aa6b1@oracle.com>
 <c8d8ac40-d4d5-417b-a67a-8b1fb016d2ca@broadcom.com>
Content-Language: en-US
From: samasth.norway.ananda@oracle.com
In-Reply-To: <c8d8ac40-d4d5-417b-a67a-8b1fb016d2ca@broadcom.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?dHMxV3BVbGdWdE5QTjdIZEhKclFBN0FpbGVyTzEveW1wRjh6MDIxQ1AzZzJ4?=
 =?utf-8?B?UE5FVHNDUW9GQXVHVWU4d0grc3NPbTVma29ibFFhOFlUaDYwZjhCZ0EvcWhI?=
 =?utf-8?B?a25ZRDZrWWNhZ2pjWlZKSFk2VVFPWnN4RWcwNmJBN0JUa0hPVURYNXZBOEFo?=
 =?utf-8?B?QWhDUGNRZlNxZ250akpyN2lFdUEyblJQYVB1U3BkNW4wYXlTS01ZcXp0OE81?=
 =?utf-8?B?eVl4dVluUUVWaklXdGhBWWRKVjhjSExLMFRwMlVIMHV3TjUrak1OSnVjU0ln?=
 =?utf-8?B?RkhHVWc0TmpMV3RTQmRqeSs3VzlCR01RRlZ2dEdMek5kbTJwWGFlcElIOFBl?=
 =?utf-8?B?RVVGaU1FMFBWUDlBRndxdDF5ZXRKZ1NobFdtQkEzT2xWVmRUUkdSUk9UZWJM?=
 =?utf-8?B?b2NEU3pJekNVNlBmcGhHWFZlNlFNWEhLSGcxUWhwb2lUY1UrdXJBQWlaRVBl?=
 =?utf-8?B?aTJBQS9acWxLeGhkNjllcWsyRVdGK3V4VFFqbVpQRjZiQk82UVFpbmtIc2xy?=
 =?utf-8?B?S3RqZDZlN3oxRHpJTm02eDNhWlN2SHZsVG9abytEbDlDaHdDYlFuSUQxelhn?=
 =?utf-8?B?bnRXZWo2ejAyOWJ0SVhTeUxWWnpEcmJhU0lFalZXT1hzMzM1cVdaZ3l3N3Rk?=
 =?utf-8?B?bnMxbVVIdWIwRlFwTm9QUzdDQlhVTU0wMVA4bUdzeldmeEg2aVUxdkE5QjZD?=
 =?utf-8?B?UDIrUEV1TGkyamw3SzBoV2g3LzJjdWFuUzUwM201bE9kSUlQS3RaN0tuVkxK?=
 =?utf-8?B?VHV5WDVvRVRsWkp0bENhMlUyOTAvdU96WnFtaVBpYUZvY3RuRHhoWEE1T1Mw?=
 =?utf-8?B?ampRYlZkSnhRaWo2eG1lWlpKWXE1ZTIzSzdCTXlaTG1NODVnRzBCalVjV2dn?=
 =?utf-8?B?ckxYYTdHUTBjaGNEaTZCNE5JNU1VM0F3MzRiK3JPRUs3YWNmWnUxTi9TeEpE?=
 =?utf-8?B?Ky9UbnJxR09WVG9uZVJDSUV5RTRGWkRYMVQ2RXYxZFBJSk9rWW5CVG1GL3px?=
 =?utf-8?B?OWRRVGhSNTRJQkFYR3AyNHJreXJYeEplNzlaUzQrQ3daWFl0M0lKM2syNC9t?=
 =?utf-8?B?bjhsN3o1bnRWRDZ0RzQxWGpzZjBwWnpNeVRBTjMvY2haazFsSzJpVUxPY3Qz?=
 =?utf-8?B?UUllTWtmall1Z3FvTVV0dk9uZnJCeWdmNUlrYzM5TE9WNHcyT2FKRzJyR2E0?=
 =?utf-8?B?UGtKUUlpaUM2REg5Q0VSREFmOUtEZ0Z3Y2FwOHNDc1MzellpLzBLNStvYVJH?=
 =?utf-8?B?bUlzM0E3UDE1MTBPay9YbFR6U2dneEh5TE1PK0ZidGUwb1lPM1luT0w4bSs1?=
 =?utf-8?B?L1NjazlsaTFqRWRFc1hvMHN2R3ZHdk1IdysySktPYUJ1SUV4R2JSWFZ3anpH?=
 =?utf-8?B?WENxa1hjd3hXay8vMGVHQy8rd25QeElVWGFXdGVoS0c5YVZMWTlWZHNwNTNS?=
 =?utf-8?B?V04vSWR4MmNncWpTQ05MRERpbU9QL3NBdUV0WmxYWG1lWno0VE1pUWRMVFVw?=
 =?utf-8?B?djkvVlY4U2xDSXJzZ05reVVZOVdqRU5FSHRHWTNmRkFVNjBhNXByditJVVlW?=
 =?utf-8?B?aEFVVTRVU1FpUjNrQzdCUHpDNFZmZkxib0d3VFExWmc1Z0FjeVljZ3pZNjNz?=
 =?utf-8?B?WXl5T2FmaUNJTkNIM3B0TGt6dGVXSWYvbnBFaFdRa2RlSDlOUlIzZWxlY0Fk?=
 =?utf-8?B?bzNmVTB1UTluWEp1MDI4QnpqQTBoeWlNWkQ5STRERWNUdzRuL1dIN1JLM1c1?=
 =?utf-8?B?ZHdYcTdvWXVNcXdzdWFkaWFNclNGQWRuQjl6SE0rS3JOZVVEK0haUzdKT0pN?=
 =?utf-8?B?ek9oWlM5OGRNcVNyT0UrK2dnYzhzdjQvSkZyMmw5aWh6WndxQmc1WEp4d2E5?=
 =?utf-8?B?c0V1TmdueUNSRWppb2F0UnZwdVJoR3BRMGtIUTl6ZFNVNzZFWjhhbzdaaUEr?=
 =?utf-8?B?TWhNeEVld3p1am5mWVR4UjZyYmxyOWJ2NmI4WGtLWjdKQk43b2hOY3lpTlBp?=
 =?utf-8?B?aTl2M0l0aGF5MHMyYTNHM0hZSk1wNnZDMFB5eko3UlBaL3VNRjhxTW9yZzk2?=
 =?utf-8?B?L2QzY01aNEh5YldWM1lqMWRsT2pLQmRrMzJWZjltb3VPLzJPRkx5SENPVDE3?=
 =?utf-8?B?bStZM09CRG5pZWR5M1hYVlVVci9BWkNkSzNTZStYQWpZU3hnRE45aFY4OHVS?=
 =?utf-8?Q?VbBXfCSSU0gBIi9qP9tNfvo=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	+OV5b+8ZkaDS5HxVmRpntNqauKtSOGl828f1/ujLzwwvG/ny0mVjSHrXZasIR8J6zhrof/8eKP0bH7UWOtZP+3ss01RpasPVYnIUqQEWm0y5MpCHnui7HtwQ2e1+uCclT9IhCD5690VK6Hz4dfP0z/gxp9XIXKo/bWoijKa+Qfuh+JzvyGwZ41XmgehgAS/e7pXXpqjy92iLzKNPC+dqyZAVLuqI9e00ViijpbEAPuUkPqeQai3E7KyC9Zs35NbujtXtxJTVRuxun5KACX5AS/Bo4UGqTXa8WoCjITDBxRYLLF7QMjBuNbqzB3ZKZZdrgr4LiSQXF1NfYQVoReeoGKKiMuS3SOuEzHpwl0C4ALu4KlQBFhzjKEbnkTcB7DpcdgRtUBhQ1HPMZfgPxFvwCAM63CvfmtAGXropFzELTfg75KsSfDfGcSEaCKMLsKyceNYJsdP8y4CaYTTRlRZFtyznsZzjKGVeZ9+rcganC+IvshgYGubSFLvAO3jzK4EJbvUT+f3lr+PVFcSYgpu/zOHxoo6QaddI5XAf2BAIfvLdWyB29yaskO0JqcCPA2r24JGR/c56Qfgl/1LoKsm409sb+pNPGz7sY4C5vhina6s=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f86f08b8-77a3-4040-9899-08dc6fb67398
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB3817.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 May 2024 23:27:40.0985
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: CZL1Zz9t8ITxd4ZaD6jJNnk+/hkdrFJAQDRsgCq6Qh0C8l9QD+TP3O4GDSIEsxy4oB0eL7uuD/gVm17Zi+HnHJdIDvhhH4GXH1jnSBkQfd+Q0mFO1XHFVNp4wlxIy0Bu
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW5PR10MB5689
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.650,FMLib:17.11.176.26
 definitions=2024-05-08_09,2024-05-08_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 bulkscore=0 mlxlogscore=999
 adultscore=0 suspectscore=0 malwarescore=0 phishscore=0 spamscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2405010000 definitions=main-2405080175
X-Proofpoint-ORIG-GUID: T6siaI2hqBb148VODvEi7YPGHUAOCcC0
X-Proofpoint-GUID: T6siaI2hqBb148VODvEi7YPGHUAOCcC0


On 5/7/24 12:09 PM, Arend van Spriel wrote:
> - moved John to Cc: as he passed the wireless maintainer role to Kalle 
> long ago.
> 
> On 5/6/2024 9:03 PM, samasth.norway.ananda@oracle.com 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.

Thanks for looking into it Arend.
I will work on a patch and make the changes as you suggested. Will send 
it out for review soon.

Samasth.
> 
> Regards,
> Arend