2022-04-14 13:45:58

by Michael Straube

Subject: [PATCH 7/7] staging: r8188eu: check firmware header existence before access

We should access the fields of fwhdr only if the check for firmware
header existence is true. Move the affected code into the if block
that checks firmware header existence.

Signed-off-by: Michael Straube <[email protected]>
---
Do we need the IS_FW_HEADER_EXIST(fwhdr) check at all?
The header _does_ exist in rtl8188eufw.bin and it's very
unlikely that it ever changes _and_ the header will be
removed.

drivers/staging/r8188eu/core/rtw_fw.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/r8188eu/core/rtw_fw.c b/drivers/staging/r8188eu/core/rtw_fw.c
index 64963507a346..c58bce1a1856 100644
--- a/drivers/staging/r8188eu/core/rtw_fw.c
+++ b/drivers/staging/r8188eu/core/rtw_fw.c
@@ -249,14 +249,13 @@ int rtl8188e_firmware_download(struct adapter *padapter)
fw_data = dvobj->firmware.data;
fw_size = dvobj->firmware.size;

- /* To Check Fw header. Added by tynli. 2009.12.04. */
fwhdr = (struct rt_firmware_hdr *)dvobj->firmware.data;

- pr_info("%sFirmware Version %d, SubVersion %d, Signature 0x%x\n",
- DRIVER_PREFIX, le16_to_cpu(fwhdr->version), fwhdr->subversion,
- le16_to_cpu(fwhdr->signature));
-
if (IS_FW_HEADER_EXIST(fwhdr)) {
+ pr_info("%sFirmware Version %d, SubVersion %d, Signature 0x%x\n",
+ DRIVER_PREFIX, le16_to_cpu(fwhdr->version), fwhdr->subversion,
+ le16_to_cpu(fwhdr->signature));
+
fw_data = fw_data + sizeof(struct rt_firmware_hdr);
fw_size = fw_size - sizeof(struct rt_firmware_hdr);
}
--
2.35.1

2022-04-16 02:30:11

by Larry Finger

Subject: Re: [PATCH 7/7] staging: r8188eu: check firmware header existence before access

On 4/14/22 03:38, Michael Straube wrote:
> We should access the fields of fwhdr only if the check for firmware
> header existence is true. Move the affected code into the if block
> that checks firmware header existence.
>
> Signed-off-by: Michael Straube <[email protected]>
> ---
> Do we need the IS_FW_HEADER_EXIST(fwhdr) check at all?
> The header _does_ exist in rtl8188eufw.bin and it's very
> unlikely that it ever changes _and_ the header will be
> removed.
>
> drivers/staging/r8188eu/core/rtw_fw.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/staging/r8188eu/core/rtw_fw.c b/drivers/staging/r8188eu/core/rtw_fw.c
> index 64963507a346..c58bce1a1856 100644
> --- a/drivers/staging/r8188eu/core/rtw_fw.c
> +++ b/drivers/staging/r8188eu/core/rtw_fw.c
> @@ -249,14 +249,13 @@ int rtl8188e_firmware_download(struct adapter *padapter)
> fw_data = dvobj->firmware.data;
> fw_size = dvobj->firmware.size;
>
> - /* To Check Fw header. Added by tynli. 2009.12.04. */
> fwhdr = (struct rt_firmware_hdr *)dvobj->firmware.data;
>
> - pr_info("%sFirmware Version %d, SubVersion %d, Signature 0x%x\n",
> - DRIVER_PREFIX, le16_to_cpu(fwhdr->version), fwhdr->subversion,
> - le16_to_cpu(fwhdr->signature));
> -
> if (IS_FW_HEADER_EXIST(fwhdr)) {
> + pr_info("%sFirmware Version %d, SubVersion %d, Signature 0x%x\n",
> + DRIVER_PREFIX, le16_to_cpu(fwhdr->version), fwhdr->subversion,
> + le16_to_cpu(fwhdr->signature));
> +
> fw_data = fw_data + sizeof(struct rt_firmware_hdr);
> fw_size = fw_size - sizeof(struct rt_firmware_hdr);
> }

You can probably remove the IS_FW_HEADER_EXIST macro, but please restore the
guard against logging the version more than once.

Larry