The sdio_read32() calls sd_read(), but does not handle the error if sd_read() fails. This could lead to subsequent operations processing invalid data. A proper implementation can be found in sdio_readN(), which has an error handling for the sd_read().
Add error handling for the sd_read() to free tmpbuf and return error code if sd_read() fails. This ensure that the memcpy() is only performed when the read operation is successful.
Since none of the callers check for the errors, there is no need to return the error code propagated from sd_read(). Returning SDIO_ERR_VAL32 might be a better choice, which is a specialized error code for SDIO.
Another problem of returning propagated error code is that the error code is a s32 type value, which is not fit with the u32 type return value of the sdio_read32().
An practical option would be to go through all the callers and add error handling, which need to pass a pointer to u32 *val and return zero on success or negative on failure. It is not a better choice since will cost unnecessary effort on the error code.
The other opion is to replace sd_read() by sd_read32(), which return an u32 type error code that can be directly used as the return value of sdio_read32(). But, it is also a bad choice to use sd_read32() in a alignment failed branch.
Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver") Cc: stable@vger.kernel.org # v4.12+ Signed-off-by: Wentao Liang vulab@iscas.ac.cn --- v7: Fix error code and add patch explanation v6: Fix improper code to propagate error code v5: Fix error code v4: Add change log and fix error code v3: Add Cc flag v2: Change code to initialize val
drivers/staging/rtl8723bs/hal/sdio_ops.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8723bs/hal/sdio_ops.c b/drivers/staging/rtl8723bs/hal/sdio_ops.c index 21e9f1858745..d79d41727042 100644 --- a/drivers/staging/rtl8723bs/hal/sdio_ops.c +++ b/drivers/staging/rtl8723bs/hal/sdio_ops.c @@ -185,7 +185,12 @@ static u32 sdio_read32(struct intf_hdl *intfhdl, u32 addr) return SDIO_ERR_VAL32;
ftaddr &= ~(u16)0x3; - sd_read(intfhdl, ftaddr, 8, tmpbuf); + err = sd_read(intfhdl, ftaddr, 8, tmpbuf); + if (err) { + kfree(tmpbuf); + return SDIO_ERR_VAL32; + } + memcpy(&le_tmp, tmpbuf + shift, 4); val = le32_to_cpu(le_tmp);