lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <eb261ca4a0fd4e91900b1359c9923b1d@infineon.com>
Date: Fri, 5 Jul 2024 02:07:22 +0000
From: <Nobuaki.Tsunashima@...ineon.com>
To: <luiz.dentz@...il.com>
CC: <marcel@...tmann.org>, <linux-bluetooth@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH v5] Bluetooth: btbcm: Apply
 HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER to CYW4373

Hi Luiz,

Thanks for your comment.

>> +#define BCM_ROMFW_BASELINE_NUM 0xFFFF static const struct 
>> +bcm_chip_version_table disable_broken_read_transmit_power_by_chip_ver[] = {
>> +       { 0x87, BCM_ROMFW_BASELINE_NUM }                /* CYW4373/4373E */
>> +};
>
> Can we have a little less verbose names? e.g. broken_read_tx_power and btbcm_broken_read_tx_power sounds a lot better imo.

We already have a table named "disable_broken_read_transmit_power" as below.

> static const struct dmi_system_id disable_broken_read_transmit_power[] = {
>	{
>		 .matches = {
>			DMI_MATCH(DMI_BOARD_VENDOR, "Apple Inc."),
>			DMI_MATCH(DMI_PRODUCT_NAME, "MacBookPro16,1"),
>		},
>	},

So, as my new table is same role of the original table but referring chip version, I think the name is straight forward.

Or, we may shorten name of the original table as well like below. Do you like it?

Original table
static const struct dmi_system_id broken_read_tx_power_dmi[] 

New table
static const struc bcm_chip_version_table broken_read_tx_power_chip_ver[]

Best Regards,
Nobuaki Tsunashima

--
---Original Message-----
From: Luiz Augusto von Dentz <luiz.dentz@...il.com> 
Sent: Friday, July 5, 2024 10:34 AM
To: Tsunashima Nobuaki (SMD C3 JP RM WLS AE) <Nobuaki.Tsunashima@...ineon.com>
Cc: Marcel Holtmann <marcel@...tmann.org>; linux-bluetooth@...r.kernel.org; linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5] Bluetooth: btbcm: Apply HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER to CYW4373

Caution: This e-mail originated outside Infineon Technologies. Please be cautious when sharing information or opening attachments especially from unknown senders. Refer to our intranet guide<https://intranet-content.infineon.com/explore/aboutinfineon/rules/informationsecurity/ug/SocialEngineering/Pages/SocialEngineeringElements_en.aspx> to help you identify Phishing email.



Hi Nobuaki,

On Thu, Jul 4, 2024 at 9:16 PM Nobuaki Tsunashima <nobuaki.tsunashima@...ineon.com> wrote:
>
> From: Nobuaki Tsunashima <Nobuaki.Tsunashima@...ineon.com>
>
> CYW4373 ROM FW has an issue that it claims LE_Read_Transmit_Power 
> command as supported in a response of Read_Local_Supported_Command 
> command but rejects the LE_Read_Transmit_Power command with "Unknown HCI Command"
> status. Because Bluetooth driver of kernel 5.11 added sending the 
> LE_Read_Transmit_Power command in initialize phase, hci up fails due 
> to the issue.
>
> Especially in USB i/f case, it would be difficult to download patch FW 
> that includes its fix unless hci is up.
>
> The driver already contains infrastructure to apply the quirk for the 
> issue, but currently it only supports DMI based matching. Add support 
> to match by chip id and baseline FW version to detect CYW4373 ROM FW 
> build in generic system.
>
> Fixes: 7c395ea521e6 ("Bluetooth: Query LE tx power on startup")
> Signed-off-by: Nobuaki Tsunashima <Nobuaki.Tsunashima@...ineon.com>
> ---
> V4 -> V5: Use skb_pull_data() to access skb->data as safer manner.
> V3 -> V4: Fix a few coding style warnings and refine comments for clarify.
> V2 -> V3: Fix a few coding style warnings and change the subject as more specific.
> V1 -> V2: Fix several coding style warnings.
>
>  drivers/bluetooth/btbcm.c | 41 
> +++++++++++++++++++++++++++++++++++++--
>  drivers/bluetooth/btusb.c |  4 ++++
>  2 files changed, 43 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c 
> index 0a5445ac5e1b..dd7262a8dc8e 100644
> --- a/drivers/bluetooth/btbcm.c
> +++ b/drivers/bluetooth/btbcm.c
> @@ -437,16 +437,53 @@ static const struct dmi_system_id disable_broken_read_transmit_power[] = {
>         { }
>  };
>
> +struct bcm_chip_version_table {
> +       u8 chip_id;                     /* Chip ID */
> +       u16 baseline;           /* Baseline version of patch FW */
> +};
> +#define BCM_ROMFW_BASELINE_NUM 0xFFFF static const struct 
> +bcm_chip_version_table disable_broken_read_transmit_power_by_chip_ver[] = {
> +       { 0x87, BCM_ROMFW_BASELINE_NUM }                /* CYW4373/4373E */
> +};

Can we have a little less verbose names? e.g. broken_read_tx_power and btbcm_broken_read_tx_power sounds a lot better imo.

> +static bool btbcm_is_disable_broken_read_tx_power_by_chip_ver(u8 
> +chip_id, u16 baseline) {
> +       int i;
> +       size_t table_size = ARRAY_SIZE(disable_broken_read_transmit_power_by_chip_ver);
> +       const struct bcm_chip_version_table *entry =
> +                                               
> +&disable_broken_read_transmit_power_by_chip_ver[0];
> +
> +       for (i = 0 ; i < table_size ; i++, entry++)     {
> +               if ((chip_id == entry->chip_id) && (baseline == entry->baseline))
> +                       return true;
> +       }
> +
> +       return false;
> +}
> +
>  static int btbcm_read_info(struct hci_dev *hdev)  {
>         struct sk_buff *skb;
> +       u8 *chip_id;
> +       u16 *baseline;
>
>         /* Read Verbose Config Version Info */
>         skb = btbcm_read_verbose_config(hdev);
>         if (IS_ERR(skb))
>                 return PTR_ERR(skb);
> -
> -       bt_dev_info(hdev, "BCM: chip id %u", skb->data[1]);
> +       skb_pull_data(skb, 1);
> +       chip_id = skb_pull_data(skb, sizeof(*chip_id));
> +       skb_pull_data(skb, 1);
> +       baseline = skb_pull_data(skb, sizeof(*baseline));
> +
> +       if (chip_id) {
> +               bt_dev_info(hdev, "BCM: chip id %u", *chip_id);
> +
> +               if (baseline) {
> +                       /* Check Chip ID and disable broken Read LE Min/Max Tx Power */
> +                       if (btbcm_is_disable_broken_read_tx_power_by_chip_ver(*chip_id, *baseline))
> +                               set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, &hdev->quirks);
> +               }
> +       }
>         kfree_skb(skb);
>
>         return 0;
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c 
> index d31edad7a056..52561c8d8828 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -142,6 +142,10 @@ static const struct usb_device_id btusb_table[] = {
>         { USB_VENDOR_AND_INTERFACE_INFO(0x04ca, 0xff, 0x01, 0x01),
>           .driver_info = BTUSB_BCM_PATCHRAM },
>
> +       /* Cypress devices with vendor specific id */
> +       { USB_VENDOR_AND_INTERFACE_INFO(0x04b4, 0xff, 0x01, 0x01),
> +         .driver_info = BTUSB_BCM_PATCHRAM },
> +
>         /* Broadcom devices with vendor specific id */
>         { USB_VENDOR_AND_INTERFACE_INFO(0x0a5c, 0xff, 0x01, 0x01),
>           .driver_info = BTUSB_BCM_PATCHRAM },
> --
> 2.25.1
>


--
Luiz Augusto von Dentz

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ