Re: [PATCH] Bluetooth: btrtl: Avoid loading the config file on security chips

[Paul Menzel <pmenzel@molgen.mpg.de>]

Dear Max,


Thank you for your patch.

Am 05.11.25 um 07:37 schrieb Max Chou:
> For chips with security enabled, it's only possible to load firmware
> with a valid signature pattern.

How can security be enabled?

What is currently logged? An error?

Please go into the changes. What is the vendor command 0xAD over 0x0D?

> - Example log for a security chip.
> 
> Bluetooth: hci0: RTL: examining hci_ver=0c hci_rev=000a
>    lmp_ver=0c lmp_subver=8922
> Bluetooth: hci0: RTL: rom_version status=0 version=1
> Bluetooth: hci0: RTL: loading rtl_bt/rtl8922au_fw.bin
> Bluetooth: hci0: RTL: cfg_sz 0, total sz 71301
> Bluetooth: hci0: RTL: fw version 0x41c0c905
> 
> - Example log for a normal chip.
> 
> Bluetooth: hci0: RTL: examining hci_ver=0c hci_rev=000a
>    lmp_ver=0c lmp_subver=8922
> Bluetooth: hci0: RTL: rom_version status=0 version=1
> Bluetooth: hci0: RTL: loading rtl_bt/rtl8922au_fw.bin
> Bluetooth: hci0: RTL: loading rtl_bt/rtl8922au_config.bin
> Bluetooth: hci0: RTL: cfg_sz 6, total sz 71307
> Bluetooth: hci0: RTL: fw version 0x41c0c905
> 
> Tested-by: Hilda Wu <hildawu@realtek.com>
> Signed-off-by: Nial Ni <niall_ni@realsil.com.cn>
> Signed-off-by: Max Chou <max.chou@realtek.com>
> ---
>   drivers/bluetooth/btrtl.c | 24 +++++++++++++-----------
>   1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
> index 8290932b8f7b..f6fccc6fdf22 100644
> --- a/drivers/bluetooth/btrtl.c
> +++ b/drivers/bluetooth/btrtl.c
> @@ -50,7 +50,7 @@
>   
>   #define	RTL_CHIP_SUBVER (&(struct rtl_vendor_cmd) {{0x10, 0x38, 0x04, 0x28, 0x80}})
>   #define	RTL_CHIP_REV    (&(struct rtl_vendor_cmd) {{0x10, 0x3A, 0x04, 0x28, 0x80}})
> -#define	RTL_SEC_PROJ    (&(struct rtl_vendor_cmd) {{0x10, 0xA4, 0x0D, 0x00, 0xb0}})
> +#define	RTL_SEC_PROJ    (&(struct rtl_vendor_cmd) {{0x10, 0xA4, 0xAD, 0x00, 0xb0}})
>   
>   #define RTL_PATCH_SNIPPETS		0x01
>   #define RTL_PATCH_DUMMY_HEADER		0x02
> @@ -544,7 +544,6 @@ static int rtlbt_parse_firmware_v2(struct hci_dev *hdev,
>   {
>   	struct rtl_epatch_header_v2 *hdr;
>   	int rc;
> -	u8 reg_val[2];
>   	u8 key_id;
>   	u32 num_sections;
>   	struct rtl_section *section;
> @@ -559,14 +558,7 @@ static int rtlbt_parse_firmware_v2(struct hci_dev *hdev,
>   		.len  = btrtl_dev->fw_len - 7, /* Cut the tail */
>   	};
>   
> -	rc = btrtl_vendor_read_reg16(hdev, RTL_SEC_PROJ, reg_val);
> -	if (rc < 0)
> -		return -EIO;
> -	key_id = reg_val[0];
> -
> -	rtl_dev_dbg(hdev, "%s: key id %u", __func__, key_id);
> -
> -	btrtl_dev->key_id = key_id;
> +	key_id = btrtl_dev->key_id;
>   
>   	hdr = rtl_iov_pull_data(&iov, sizeof(*hdr));
>   	if (!hdr)
> @@ -1081,6 +1073,8 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
>   	u16 hci_rev, lmp_subver;
>   	u8 hci_ver, lmp_ver, chip_type = 0;
>   	int ret;
> +	int rc;
> +	u8 key_id;
>   	u8 reg_val[2];
>   
>   	btrtl_dev = kzalloc(sizeof(*btrtl_dev), GFP_KERNEL);
> @@ -1191,6 +1185,14 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
>   		goto err_free;
>   	}
>   
> +	rc = btrtl_vendor_read_reg16(hdev, RTL_SEC_PROJ, reg_val);
> +	if (rc < 0)
> +		goto err_free;
> +
> +	key_id = reg_val[0];
> +	btrtl_dev->key_id = key_id;
> +	rtl_dev_dbg(hdev, "%s: key id %u", __func__, key_id);
> +
>   	btrtl_dev->fw_len = -EIO;
>   	if (lmp_subver == RTL_ROM_LMP_8852A && hci_rev == 0x000c) {
>   		snprintf(fw_name, sizeof(fw_name), "%s_v2.bin",
> @@ -1213,7 +1215,7 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
>   		goto err_free;
>   	}
>   
> -	if (btrtl_dev->ic_info->cfg_name) {
> +	if (btrtl_dev->ic_info->cfg_name && !btrtl_dev->key_id) {

So on non-security enabled chips, key_id is 0? It’d be great if that 
could be made clear in the commit message.

>   		if (postfix) {
>   			snprintf(cfg_name, sizeof(cfg_name), "%s-%s.bin",
>   				 btrtl_dev->ic_info->cfg_name, postfix);


Kind regards,

Paul