diff mbox series

[1/2] wifi: rtw89: don't request partial firmware if SECURITY_LOADPIN_ENFORCE

Message ID 20221202060521.501512-2-pkshih@realtek.com
State New
Headers show
Series [1/2] wifi: rtw89: don't request partial firmware if SECURITY_LOADPIN_ENFORCE | expand

Commit Message

Ping-Ke Shih Dec. 2, 2022, 6:05 a.m. UTC
From: Zong-Zhe Yang <kevin_yang@realtek.com>

Kernel logs on platform enabling SECURITY_LOADPIN_ENFORCE
------
```
LoadPin: firmware old-api-denied obj=<unknown> pid=810 cmdline="modprobe -q -- rtw89_8852ce"
rtw89_8852ce 0000:01:00.0: loading /lib/firmware/rtw89/rtw8852c_fw.bin failed with error -1
rtw89_8852ce 0000:01:00.0: Direct firmware load for rtw89/rtw8852c_fw.bin failed with error -1
rtw89_8852ce 0000:01:00.0: failed to early request firmware: -1
```

Trace
------
```
request_partial_firmware_into_buf()
> _request_firmware()
>> fw_get_filesystem_firmware()
>>> kernel_read_file_from_path_initns()
>>>> kernel_read_file()
>>>>> security_kernel_read_file()
// It will iterate enabled LSMs' hooks for kernel_read_file.
// With loadpin, it hooks loadpin_read_file.
```

If SECURITY_LOADPIN_ENFORCE is enabled, doing kernel_read_file() on partial
files will be denied and return -EPERM (-1). Then, the outer API based on it,
e.g. request_partial_firmware_into_buf(), will get the error.

In the case, we cannot get the firmware stuffs right, even though there might
be no error other than a permission issue on reading a partial file. So we have
to request full firmware if SECURITY_LOADPIN_ENFORCE is enabled. It makes us
still have a chance to do early firmware work on this kind of platforms.

Signed-off-by: Zong-Zhe Yang <kevin_yang@realtek.com>
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtw89/fw.c | 30 +++++++++++++++++--------
 drivers/net/wireless/realtek/rtw89/fw.h | 15 +++++++++++++
 2 files changed, 36 insertions(+), 9 deletions(-)

Comments

Kalle Valo Dec. 8, 2022, 2:47 p.m. UTC | #1
Ping-Ke Shih <pkshih@realtek.com> wrote:

> From: Zong-Zhe Yang <kevin_yang@realtek.com>
> 
> Kernel logs on platform enabling SECURITY_LOADPIN_ENFORCE
> ------
> ```
> LoadPin: firmware old-api-denied obj=<unknown> pid=810 cmdline="modprobe -q -- rtw89_8852ce"
> rtw89_8852ce 0000:01:00.0: loading /lib/firmware/rtw89/rtw8852c_fw.bin failed with error -1
> rtw89_8852ce 0000:01:00.0: Direct firmware load for rtw89/rtw8852c_fw.bin failed with error -1
> rtw89_8852ce 0000:01:00.0: failed to early request firmware: -1
> ```
> 
> Trace
> ------
> ```
> request_partial_firmware_into_buf()
> > _request_firmware()
> >> fw_get_filesystem_firmware()
> >>> kernel_read_file_from_path_initns()
> >>>> kernel_read_file()
> >>>>> security_kernel_read_file()
> // It will iterate enabled LSMs' hooks for kernel_read_file.
> // With loadpin, it hooks loadpin_read_file.
> ```
> 
> If SECURITY_LOADPIN_ENFORCE is enabled, doing kernel_read_file() on partial
> files will be denied and return -EPERM (-1). Then, the outer API based on it,
> e.g. request_partial_firmware_into_buf(), will get the error.
> 
> In the case, we cannot get the firmware stuffs right, even though there might
> be no error other than a permission issue on reading a partial file. So we have
> to request full firmware if SECURITY_LOADPIN_ENFORCE is enabled. It makes us
> still have a chance to do early firmware work on this kind of platforms.
> 
> Signed-off-by: Zong-Zhe Yang <kevin_yang@realtek.com>
> Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>

2 patches applied to wireless-next.git, thanks.

3ddfe3bdd3cf wifi: rtw89: don't request partial firmware if SECURITY_LOADPIN_ENFORCE
13eb07e0be1b wifi: rtw89: request full firmware only once if it's early requested
diff mbox series

Patch

diff --git a/drivers/net/wireless/realtek/rtw89/fw.c b/drivers/net/wireless/realtek/rtw89/fw.c
index 7e682709232d7..f5daea0d4f93c 100644
--- a/drivers/net/wireless/realtek/rtw89/fw.c
+++ b/drivers/net/wireless/realtek/rtw89/fw.c
@@ -277,25 +277,37 @@  void rtw89_early_fw_feature_recognize(struct device *device,
 				      const struct rtw89_chip_info *chip,
 				      u32 *early_feat_map)
 {
-	union {
-		struct rtw89_mfw_hdr mfw_hdr;
-		u8 fw_hdr[RTW89_FW_HDR_SIZE];
-	} buf = {};
+	union rtw89_compat_fw_hdr buf = {};
 	const struct firmware *firmware;
+	bool full_req = false;
 	u32 ver_code;
 	int ret;
 	int i;
 
-	ret = request_partial_firmware_into_buf(&firmware, chip->fw_name,
-						device, &buf, sizeof(buf), 0);
+	/* If SECURITY_LOADPIN_ENFORCE is enabled, reading partial files will
+	 * be denied (-EPERM). Then, we don't get right firmware things as
+	 * expected. So, in this case, we have to request full firmware here.
+	 */
+	if (IS_ENABLED(CONFIG_SECURITY_LOADPIN_ENFORCE))
+		full_req = true;
+
+	if (full_req)
+		ret = request_firmware(&firmware, chip->fw_name, device);
+	else
+		ret = request_partial_firmware_into_buf(&firmware, chip->fw_name,
+							device, &buf, sizeof(buf),
+							0);
+
 	if (ret) {
 		dev_err(device, "failed to early request firmware: %d\n", ret);
 		return;
 	}
 
-	ver_code = buf.mfw_hdr.sig != RTW89_MFW_SIG ?
-		   RTW89_FW_HDR_VER_CODE(&buf.fw_hdr) :
-		   RTW89_MFW_HDR_VER_CODE(&buf.mfw_hdr);
+	if (full_req)
+		ver_code = rtw89_compat_fw_hdr_ver_code(firmware->data);
+	else
+		ver_code = rtw89_compat_fw_hdr_ver_code(&buf);
+
 	if (!ver_code)
 		goto out;
 
diff --git a/drivers/net/wireless/realtek/rtw89/fw.h b/drivers/net/wireless/realtek/rtw89/fw.h
index 46d57414f24e2..5c4c7de1b4f5d 100644
--- a/drivers/net/wireless/realtek/rtw89/fw.h
+++ b/drivers/net/wireless/realtek/rtw89/fw.h
@@ -3291,6 +3291,21 @@  struct fwcmd_hdr {
 	__le32 hdr1;
 };
 
+union rtw89_compat_fw_hdr {
+	struct rtw89_mfw_hdr mfw_hdr;
+	u8 fw_hdr[RTW89_FW_HDR_SIZE];
+};
+
+static inline u32 rtw89_compat_fw_hdr_ver_code(const void *fw_buf)
+{
+	const union rtw89_compat_fw_hdr *compat = (typeof(compat))fw_buf;
+
+	if (compat->mfw_hdr.sig == RTW89_MFW_SIG)
+		return RTW89_MFW_HDR_VER_CODE(&compat->mfw_hdr);
+	else
+		return RTW89_FW_HDR_VER_CODE(&compat->fw_hdr);
+}
+
 #define RTW89_H2C_RF_PAGE_SIZE 500
 #define RTW89_H2C_RF_PAGE_NUM 3
 struct rtw89_fw_h2c_rf_reg_info {