Hi,
On 09.10.2019 12:35, Hans de Goede wrote:
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:
- 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.
Thank you for perfect investigation! I can only agree with you, because this code is buggy. It looks like no one faced this bug previously and the code can be safely removed.
Best Regards, Denis
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