Re: [PATCH] staging: rtl8723bs: hal: Fix memcpy calls

[Hans de Goede <hdegoede@redhat.com>]

Hi Denis,

On 30-09-2019 13:01, Denis Efremov wrote:
> memcpy() in phy_ConfigBBWithParaFile() and PHY_ConfigRFWithParaFile() is
> called with "src == NULL && len == 0". This is an undefined behavior.
> Moreover this if pre-condition "pBufLen && (*pBufLen == 0) && !pBuf"
> is constantly false because it is a nested if in the else brach, i.e.,
> "if (cond) { ... } else { if (cond) {...} }". This patch alters the
> if condition to check "pBufLen && pBuf" pointers are not NULL.
> 
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Hans de Goede <hdegoede@redhat.com>
> Cc: Bastien Nocera <hadess@hadess.net>
> Cc: Larry Finger <Larry.Finger@lwfinger.net>
> Cc: Jes Sorensen <jes.sorensen@gmail.com>
> Cc: stable@vger.kernel.org
> Signed-off-by: Denis Efremov <efremov@linux.com>
> ---
> Not tested. I don't have the hardware. The fix is based on my guess.

Thsnk you for your patch.

So I've been doing some digging and this code normally never executes.

For this to execute the user would need to change the rtw_load_phy_file module
param from its default of 0x44 (LOAD_BB_PG_PARA_FILE | LOAD_RF_TXPWR_LMT_PARA_FILE)
to something which includes 0x02 (LOAD_BB_PARA_FILE) as mask.

And even with that param set for this code to actually do something /
for pBuf to ever not be NULL the following conditions would have to
be true:

1) Set the rtw_load_phy_file module param from its default of
    0x44 (LOAD_BB_PG_PARA_FILE | LOAD_RF_TXPWR_LMT_PARA_FILE) to something
    which includes 0x02 as mask; and
2) Set rtw_phy_file_path module parameter to say "/lib/firmware/"; and
3) Store a /lib/firmware/rtl8723b/PHY_REG.txt file in the expected format.

So I've come to the conclusion that all the phy_Config*WithParaFile functions
(and a bunch of stuff they use) can be removed.

I will prepare and submit a patch for this.

Regards,

Hans



> 
>   drivers/staging/rtl8723bs/hal/hal_com_phycfg.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c b/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
> index 6539bee9b5ba..0902dc3c1825 100644
> --- a/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
> +++ b/drivers/staging/rtl8723bs/hal/hal_com_phycfg.c
> @@ -2320,7 +2320,7 @@ int phy_ConfigBBWithParaFile(
>   			}
>   		}
>   	} else {
> -		if (pBufLen && (*pBufLen == 0) && !pBuf) {
> +		if (pBufLen && pBuf) {
>   			memcpy(pHalData->para_file_buf, pBuf, *pBufLen);
>   			rtStatus = _SUCCESS;
>   		} else
> @@ -2752,7 +2752,7 @@ int PHY_ConfigRFWithParaFile(
>   			}
>   		}
>   	} else {
> -		if (pBufLen && (*pBufLen == 0) && !pBuf) {
> +		if (pBufLen && pBuf) {
>   			memcpy(pHalData->para_file_buf, pBuf, *pBufLen);
>   			rtStatus = _SUCCESS;
>   		} else
> 

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel