When a firmware is loaded in the compressed format or via user-mode
helper, it's mapped in read-only, and the rtw89 driver crashes at
rtw89_fw_download() when it tries to modify some data.
This patch is an attemp to avoid the crash by re-allocating the data
via vmalloc() for the data modification.
Buglink: https://bugzilla.opensuse.org/show_bug.cgi?id=1188303
Signed-off-by: Takashi Iwai <[email protected]>
---
drivers/net/wireless/realtek/rtw89/core.h | 3 ++-
drivers/net/wireless/realtek/rtw89/fw.c | 15 ++++++++++-----
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h
index c2885e4dd882..048855e05697 100644
--- a/drivers/net/wireless/realtek/rtw89/core.h
+++ b/drivers/net/wireless/realtek/rtw89/core.h
@@ -2309,7 +2309,8 @@ struct rtw89_fw_suit {
RTW89_FW_VER_CODE((s)->major_ver, (s)->minor_ver, (s)->sub_ver, (s)->sub_idex)
struct rtw89_fw_info {
- const struct firmware *firmware;
+ const void *firmware;
+ size_t firmware_size;
struct rtw89_dev *rtwdev;
struct completion completion;
u8 h2c_seq;
diff --git a/drivers/net/wireless/realtek/rtw89/fw.c b/drivers/net/wireless/realtek/rtw89/fw.c
index 212aaf577d3c..b59fecaeea25 100644
--- a/drivers/net/wireless/realtek/rtw89/fw.c
+++ b/drivers/net/wireless/realtek/rtw89/fw.c
@@ -124,8 +124,8 @@ int rtw89_mfw_recognize(struct rtw89_dev *rtwdev, enum rtw89_fw_type type,
struct rtw89_fw_suit *fw_suit)
{
struct rtw89_fw_info *fw_info = &rtwdev->fw;
- const u8 *mfw = fw_info->firmware->data;
- u32 mfw_len = fw_info->firmware->size;
+ const u8 *mfw = fw_info->firmware;
+ u32 mfw_len = fw_info->firmware_size;
const struct rtw89_mfw_hdr *mfw_hdr = (const struct rtw89_mfw_hdr *)mfw;
const struct rtw89_mfw_info *mfw_info;
int i;
@@ -489,7 +489,10 @@ static void rtw89_load_firmware_cb(const struct firmware *firmware, void *contex
return;
}
- fw->firmware = firmware;
+ fw->firmware = vmalloc(firmware->size);
+ if (fw->firmware)
+ memcpy((void *)fw->firmware, firmware->data, firmware->size);
+ release_firmware(firmware);
complete_all(&fw->completion);
}
@@ -518,8 +521,10 @@ void rtw89_unload_firmware(struct rtw89_dev *rtwdev)
rtw89_wait_firmware_completion(rtwdev);
- if (fw->firmware)
- release_firmware(fw->firmware);
+ if (fw->firmware) {
+ vfree(fw->firmware);
+ fw->firmware = NULL;
+ }
}
#define H2C_CAM_LEN 60
--
2.26.2
On Fri, 05 Nov 2021 08:17:25 +0100,
Takashi Iwai wrote:
>
> When a firmware is loaded in the compressed format or via user-mode
> helper, it's mapped in read-only, and the rtw89 driver crashes at
> rtw89_fw_download() when it tries to modify some data.
>
> This patch is an attemp to avoid the crash by re-allocating the data
> via vmalloc() for the data modification.
Alternatively, we may drop the code that modifies the loaded firmware
data? At least SET_FW_HDR_PART_SIZE() in rtw89_fw_hdr_parser() looks
writing it, and I have no idea why this overwrite is needed.
thanks,
Takashi
>
> Buglink: https://bugzilla.opensuse.org/show_bug.cgi?id=1188303
> Signed-off-by: Takashi Iwai <[email protected]>
>
> ---
> drivers/net/wireless/realtek/rtw89/core.h | 3 ++-
> drivers/net/wireless/realtek/rtw89/fw.c | 15 ++++++++++-----
> 2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw89/core.h b/drivers/net/wireless/realtek/rtw89/core.h
> index c2885e4dd882..048855e05697 100644
> --- a/drivers/net/wireless/realtek/rtw89/core.h
> +++ b/drivers/net/wireless/realtek/rtw89/core.h
> @@ -2309,7 +2309,8 @@ struct rtw89_fw_suit {
> RTW89_FW_VER_CODE((s)->major_ver, (s)->minor_ver, (s)->sub_ver, (s)->sub_idex)
>
> struct rtw89_fw_info {
> - const struct firmware *firmware;
> + const void *firmware;
> + size_t firmware_size;
> struct rtw89_dev *rtwdev;
> struct completion completion;
> u8 h2c_seq;
> diff --git a/drivers/net/wireless/realtek/rtw89/fw.c b/drivers/net/wireless/realtek/rtw89/fw.c
> index 212aaf577d3c..b59fecaeea25 100644
> --- a/drivers/net/wireless/realtek/rtw89/fw.c
> +++ b/drivers/net/wireless/realtek/rtw89/fw.c
> @@ -124,8 +124,8 @@ int rtw89_mfw_recognize(struct rtw89_dev *rtwdev, enum rtw89_fw_type type,
> struct rtw89_fw_suit *fw_suit)
> {
> struct rtw89_fw_info *fw_info = &rtwdev->fw;
> - const u8 *mfw = fw_info->firmware->data;
> - u32 mfw_len = fw_info->firmware->size;
> + const u8 *mfw = fw_info->firmware;
> + u32 mfw_len = fw_info->firmware_size;
> const struct rtw89_mfw_hdr *mfw_hdr = (const struct rtw89_mfw_hdr *)mfw;
> const struct rtw89_mfw_info *mfw_info;
> int i;
> @@ -489,7 +489,10 @@ static void rtw89_load_firmware_cb(const struct firmware *firmware, void *contex
> return;
> }
>
> - fw->firmware = firmware;
> + fw->firmware = vmalloc(firmware->size);
> + if (fw->firmware)
> + memcpy((void *)fw->firmware, firmware->data, firmware->size);
> + release_firmware(firmware);
> complete_all(&fw->completion);
> }
>
> @@ -518,8 +521,10 @@ void rtw89_unload_firmware(struct rtw89_dev *rtwdev)
>
> rtw89_wait_firmware_completion(rtwdev);
>
> - if (fw->firmware)
> - release_firmware(fw->firmware);
> + if (fw->firmware) {
> + vfree(fw->firmware);
> + fw->firmware = NULL;
> + }
> }
>
> #define H2C_CAM_LEN 60
> --
> 2.26.2
>
Takashi Iwai <[email protected]> writes:
> On Fri, 05 Nov 2021 08:17:25 +0100,
> Takashi Iwai wrote:
>>
>> When a firmware is loaded in the compressed format or via user-mode
>> helper, it's mapped in read-only, and the rtw89 driver crashes at
>> rtw89_fw_download() when it tries to modify some data.
>>
>> This patch is an attemp to avoid the crash by re-allocating the data
>> via vmalloc() for the data modification.
>
> Alternatively, we may drop the code that modifies the loaded firmware
> data? At least SET_FW_HDR_PART_SIZE() in rtw89_fw_hdr_parser() looks
> writing it, and I have no idea why this overwrite is needed.
Strange, isn't the firmware data marked as const just to avoid this kind
of problem? Does rtw89 have wrong casts somewhere which removes the
const?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches