| 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
| ||
|
Message-ID: <CABBYNZ+PtSR-b1gpxdeDCV7qdDFP2ZxqBDS1ic4i=H1LUGBdWA@mail.gmail.com>
Date: Thu, 31 Oct 2024 16:47:16 -0400
From: Luiz Augusto von Dentz <luiz.dentz@...il.com>
To: Hilda Wu <hildawu@...ltek.com>
Cc: marcel@...tmann.org, linux-bluetooth@...r.kernel.org,
linux-kernel@...r.kernel.org, max.chou@...ltek.com, alex_lu@...lsil.com.cn,
kidman@...ltek.com
Subject: Re: [PATCH] bluetooth: add quirk using packet size 60
Hi Hilda,
On Wed, Oct 30, 2024 at 6:08 AM Hilda Wu <hildawu@...ltek.com> wrote:
>
> The RTL8852BE-VT supports USB alternate setting 6.
> However, its descriptor does not report this capability to the host.
> Therefore, a quirk is needed to bypass the RTL8852BE-VT's descriptor
> and allow it to use USB ALT 6 directly.
It is getting very hackish trying to fixup the descriptor in software,
isn't it possible to update the USB descriptors via firmware update?
Not to mention you didn't include any logs of how this was tested, I
suppose this is for HFP/SCO wideband (using msbc), is that just plain
broken right now?
> Signed-off-by: Hilda Wu <hildawu@...ltek.com>
> ---
> drivers/bluetooth/btrtl.c | 3 ++
> drivers/bluetooth/btrtl.h | 1 +
> drivers/bluetooth/btusb.c | 89 ++++++++++++++++++++++++++++++---------
> 3 files changed, 73 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
> index 0bcb44cf7b31..b75f0b36a09b 100644
> --- a/drivers/bluetooth/btrtl.c
> +++ b/drivers/bluetooth/btrtl.c
> @@ -1312,6 +1312,9 @@ void btrtl_set_quirks(struct hci_dev *hdev, struct btrtl_device_info *btrtl_dev)
> btrtl_dev->project_id == CHIP_ID_8852C)
> set_bit(HCI_QUIRK_USE_MSFT_EXT_ADDRESS_FILTER, &hdev->quirks);
>
> + if (btrtl_dev->project_id == CHIP_ID_8852BT)
> + btrealtek_set_flag(hdev, REALTEK_ALT6_FORCE);
> +
> hci_set_aosp_capable(hdev);
> break;
> default:
> diff --git a/drivers/bluetooth/btrtl.h b/drivers/bluetooth/btrtl.h
> index a2d9d34f9fb0..ffec2fca88ec 100644
> --- a/drivers/bluetooth/btrtl.h
> +++ b/drivers/bluetooth/btrtl.h
> @@ -105,6 +105,7 @@ struct rtl_vendor_cmd {
>
> enum {
> REALTEK_ALT6_CONTINUOUS_TX_CHIP,
> + REALTEK_ALT6_FORCE,
>
> __REALTEK_NUM_FLAGS,
> };
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 514d593923ad..eca0b173232e 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -804,6 +804,7 @@ struct qca_dump_info {
> #define BTUSB_USE_ALT3_FOR_WBS 15
> #define BTUSB_ALT6_CONTINUOUS_TX 16
> #define BTUSB_HW_SSR_ACTIVE 17
> +#define BTUSB_ALT_CHANGED 18
>
> struct btusb_data {
> struct hci_dev *hdev;
> @@ -2130,16 +2131,61 @@ static void btusb_notify(struct hci_dev *hdev, unsigned int evt)
> }
> }
>
> +static struct usb_host_interface *btusb_find_altsetting(struct btusb_data *data,
> + int alt)
> +{
> + struct usb_interface *intf = data->isoc;
> + int i;
> +
> + BT_DBG("Looking for Alt no :%d", alt);
> +
> + if (!intf)
> + return NULL;
> +
> + for (i = 0; i < intf->num_altsetting; i++) {
> + if (intf->altsetting[i].desc.bAlternateSetting == alt)
> + return &intf->altsetting[i];
> + }
> +
> + return NULL;
> +}
> +
> static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
> {
> struct btusb_data *data = hci_get_drvdata(hdev);
> struct usb_interface *intf = data->isoc;
> struct usb_endpoint_descriptor *ep_desc;
> + struct usb_host_interface *alt;
> int i, err;
>
> if (!data->isoc)
> return -ENODEV;
>
> + /* For some Realtek chips, they actually have the altsetting 6, but its
> + * altsetting descriptor is not exposed. We can activate altsetting 6 by
> + * replacing the altsetting 5.
> + */
> + if (altsetting == 6 && !btusb_find_altsetting(data, 6) &&
> + btrealtek_test_flag(hdev, REALTEK_ALT6_FORCE)) {
> + alt = NULL;
> + for (i = 0; i < intf->num_altsetting; i++) {
> + if (intf->altsetting[i].desc.bAlternateSetting == 5) {
> + alt = &intf->altsetting[i];
> + break;
> + }
> + }
I believe you can replace the code above with btusb_find_altsetting so
please use that instead of duplicating its logic.
> + if (alt) {
> + for (i = 0; i < alt->desc.bNumEndpoints; i++) {
> + ep_desc = &alt->endpoint[i].desc;
> + if (usb_endpoint_is_isoc_out(ep_desc) ||
> + usb_endpoint_is_isoc_in(ep_desc))
> + ep_desc->wMaxPacketSize = 63;
> + }
> + alt->desc.bAlternateSetting = 6;
> + set_bit(BTUSB_ALT_CHANGED, &data->flags);
> + }
> + }
> +
> err = usb_set_interface(data->udev, data->isoc_ifnum, altsetting);
> if (err < 0) {
> bt_dev_err(hdev, "setting interface failed (%d)", -err);
> @@ -2151,6 +2197,27 @@ static inline int __set_isoc_interface(struct hci_dev *hdev, int altsetting)
> data->isoc_tx_ep = NULL;
> data->isoc_rx_ep = NULL;
>
> + /* Recover alt 5 desc if alt 0 is set. */
> + if (!altsetting && test_bit(BTUSB_ALT_CHANGED, &data->flags)) {
> + alt = NULL;
> + for (i = 0; i < intf->num_altsetting; i++) {
> + if (intf->altsetting[i].desc.bAlternateSetting == 6) {
> + alt = &intf->altsetting[i];
> + break;
> + }
> + }
Ditto.
> + if (alt) {
> + for (i = 0; i < alt->desc.bNumEndpoints; i++) {
> + ep_desc = &alt->endpoint[i].desc;
> + if (usb_endpoint_is_isoc_out(ep_desc) ||
> + usb_endpoint_is_isoc_in(ep_desc))
> + ep_desc->wMaxPacketSize = 49;
> + }
> + alt->desc.bAlternateSetting = 5;
> + clear_bit(BTUSB_ALT_CHANGED, &data->flags);
> + }
> + }
> +
> for (i = 0; i < intf->cur_altsetting->desc.bNumEndpoints; i++) {
> ep_desc = &intf->cur_altsetting->endpoint[i].desc;
>
> @@ -2213,25 +2280,6 @@ static int btusb_switch_alt_setting(struct hci_dev *hdev, int new_alts)
> return 0;
> }
>
> -static struct usb_host_interface *btusb_find_altsetting(struct btusb_data *data,
> - int alt)
> -{
> - struct usb_interface *intf = data->isoc;
> - int i;
> -
> - BT_DBG("Looking for Alt no :%d", alt);
> -
> - if (!intf)
> - return NULL;
> -
> - for (i = 0; i < intf->num_altsetting; i++) {
> - if (intf->altsetting[i].desc.bAlternateSetting == alt)
> - return &intf->altsetting[i];
> - }
> -
> - return NULL;
> -}
> -
> static void btusb_work(struct work_struct *work)
> {
> struct btusb_data *data = container_of(work, struct btusb_data, work);
> @@ -2269,7 +2317,8 @@ static void btusb_work(struct work_struct *work)
> * MTU >= 3 (packets) * 25 (size) - 3 (headers) = 72
> * see also Core spec 5, vol 4, B 2.1.1 & Table 2.1.
> */
> - if (btusb_find_altsetting(data, 6))
> + if (btusb_find_altsetting(data, 6) ||
> + btrealtek_test_flag(hdev, REALTEK_ALT6_FORCE))
> new_alts = 6;
> else if (btusb_find_altsetting(data, 3) &&
> hdev->sco_mtu >= 72 &&
> --
> 2.34.1
>
--
Luiz Augusto von Dentz
Powered by blists - more mailing lists