Messages in this thread | | | Date | Wed, 13 Apr 2022 21:42:28 +0200 | Subject | Re: staging: r8188eu: struct rt_firmware_header issues | From | Michael Straube <> |
| |
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.
regards, Michael
| |