linux-kernel - Re: [PATCH 5/5] staging: r8188eu: turbo scan is always off for r8188eu

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <6d33d346-b797-aeb3-8b1b-64332e05fb87@gmail.com>
Date:   Mon, 3 Jan 2022 00:44:12 +0300
From:   Pavel Skripkin <paskripkin@...il.com>
To:     Martin Kaiser <martin@...ser.cx>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     Larry Finger <Larry.Finger@...inger.net>,
        Phillip Potter <phil@...lpotter.co.uk>,
        Michael Straube <straube.linux@...il.com>,
        linux-staging@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 5/5] staging: r8188eu: turbo scan is always off for
 r8188eu

On 1/2/22 20:59, Martin Kaiser wrote:
> Turbo scan is always disabled. Remove the TurboScanOff variable and
> related checks.
> 
> Signed-off-by: Martin Kaiser <martin@...ser.cx>
> ---
>   drivers/staging/r8188eu/hal/rtl8188e_rf6052.c | 21 +++++++------------
>   1 file changed, 7 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/staging/r8188eu/hal/rtl8188e_rf6052.c b/drivers/staging/r8188eu/hal/rtl8188e_rf6052.c
> index 0b982cc7c86f..6e0231099986 100644
> --- a/drivers/staging/r8188eu/hal/rtl8188e_rf6052.c
> +++ b/drivers/staging/r8188eu/hal/rtl8188e_rf6052.c
> @@ -87,28 +87,21 @@ rtl8188e_PHY_RF6052SetCckTxPower(
>   	struct hal_data_8188e *pHalData = &Adapter->haldata;
>   	struct mlme_ext_priv *pmlmeext = &Adapter->mlmeextpriv;
>   	u32 TxAGC[2] = {0, 0}, tmpval = 0, pwrtrac_value;
> -	bool TurboScanOff = false;
>   	u8 idx1, idx2;
>   	u8 *ptr;
>   	u8 direction;
> -	/* FOR CE ,must disable turbo scan */
> -	TurboScanOff = true;
>   
>   	if (pmlmeext->sitesurvey_res.state == SCAN_PROCESS) {
>   		TxAGC[RF_PATH_A] = 0x3f3f3f3f;
>   		TxAGC[RF_PATH_B] = 0x3f3f3f3f;
>   
> -		TurboScanOff = true;/* disable turbo scan */
> -
> -		if (TurboScanOff) {
> -			for (idx1 = RF_PATH_A; idx1 <= RF_PATH_B; idx1++) {
> -				TxAGC[idx1] =
> -					pPowerlevel[idx1] | (pPowerlevel[idx1] << 8) |
> -					(pPowerlevel[idx1] << 16) | (pPowerlevel[idx1] << 24);
> -				/*  2010/10/18 MH For external PA module. We need to limit power index to be less than 0x20. */
> -				if (TxAGC[idx1] > 0x20 && pHalData->ExternalPA)
> -					TxAGC[idx1] = 0x20;
> -			}
> +		for (idx1 = RF_PATH_A; idx1 <= RF_PATH_B; idx1++) {
> +			TxAGC[idx1] =
> +				pPowerlevel[idx1] | (pPowerlevel[idx1] << 8) |
> +				(pPowerlevel[idx1] << 16) | (pPowerlevel[idx1] << 24);
> +			/*  2010/10/18 MH For external PA module. We need to limit power index to be less than 0x20. */
> +			if (TxAGC[idx1] > 0x20 && pHalData->ExternalPA)
> +				TxAGC[idx1] = 0x20;
>   		}
>   	} else {
>   		for (idx1 = RF_PATH_A; idx1 <= RF_PATH_B; idx1++) {

Hi Martin,

thanks for your clean up work!


It's a bit unrelated to the patch, but I found it while reviewing this. 
It's in the same function rtl8188e_PHY_RF6052SetCckTxPower():

89: 	u32 TxAGC[2] = {0, 0};
...
92	u8 *ptr;
...
129	for (idx1 = RF_PATH_A; idx1 <= RF_PATH_B; idx1++) {
130		ptr = (u8 *)(&TxAGC[idx1]);
131		for (idx2 = 0; idx2 < 4; idx2++) {
132			if (*ptr > RF6052_MAX_TX_PWR)
133				*ptr = RF6052_MAX_TX_PWR;
134			ptr++;
135		}
136	}


What is going on here? Code just checks and writes to random place on 
stack outside TxAGC array? I might be missing something, but it looks 
wrong...

Just wanted to share, since I believe your knowledge about this driver 
is much deeper than mine :)

thanks


With regards,
Pavel Skripkin