Messages in this thread | | | Date | Wed, 13 Apr 2022 16:12:07 -0500 | Subject | Re: staging: r8188eu: struct rt_firmware_header issues | From | Larry Finger <> |
| |
On 4/13/22 14:42, Michael Straube wrote: > On 4/13/22 18:27, Michael Straube wrote: >> Hi all, >> >> I think the rt_firmware_hdr structure in rtw_fw.c has some issues. >> >> >> struct rt_firmware_hdr { >> /* 8-byte alinment required */ >> /* LONG WORD 0 ---- */ >> __le16 Signature; /* 92C0: test chip; 92C, >> * 88C0: test chip; 88C1: MP A-cut; >> * 92C1: MP A-cut */ >> u8 Category; /* AP/NIC and USB/PCI */ >> u8 Function; /* Reserved for different FW function >> * indcation, for further use when >> * driver needs to download different >> * FW for different conditions */ >> __le16 Version; /* FW Version */ >> u8 Subversion; /* FW Subversion, default 0x00 */ >> u16 Rsvd1; >> >> /* LONG WORD 1 ---- */ >> u8 Month; /* Release time Month field */ >> u8 Date; /* Release time Date field */ >> u8 Hour; /* Release time Hour field */ >> u8 Minute; /* Release time Minute field */ >> __le16 RamCodeSize; /* The size of RAM code */ >> u8 Foundry; >> u8 Rsvd2; >> >> /* LONG WORD 2 ---- */ >> __le32 SvnIdx; /* The SVN entry index */ >> u32 Rsvd3; >> >> /* LONG WORD 3 ---- */ >> u32 Rsvd4; >> u32 Rsvd5; >> }; >> >> >> Then we have in rtl8188e_firmware_download(): >> >> >> fwhdr = (struct rt_firmware_hdr *)dvobj->firmware.data; >> >> <snip> >> >> if (IS_FW_HEADER_EXIST(fwhdr)) { >> /* Shift 32 bytes for FW header */ >> fw_data = fw_data + 32; >> fw_size = fw_size - 32; >> } >> >> We add/sub 32 bytes but the size of struct rt_firmware_hdr is actually >> 33 bytes. I noticed this when I wanted to replace: >> >> fw_data = fw_data + 32; >> fw_size = fw_size - 32; >> >> with: >> fw_data = fw_data + sizeof(struct rt_firmware_hdr); >> fw_size = fw_size - sizeof(struct rt_firmware_hdr);; >> >> To me it looks add/sub 32 is correct here but the struct is >> wrong. I don't know if the firmware for this driver is so much different >> from firmware for the drivers in drivers/net/wireless/realtek/rtlwifi. >> They use a struct of size 32. >> >> Also, souldn't the u16 and u32 variables in the struct be __le16 and >> __le32 ? >> >> I wonder if we can just use the rtlwifi_firmware_header structure from >> drivers/net/wireless/realtek/rtlwifi/wifi.h ? >> >> Comments from people with better knowledge appreciated. :) >> >> regards, >> Michael >> >> > > Ok, I figured it out by looking at the hexdumps of firmware files. The > field Rsvd1 should be u8 instead of u16. I'll prepare a patch for this.
I agree. I had prepared an analysis, but did not get it mailed before I got this one.
Larry
| |