Message ID | a36966dd-677a-916c-9a03-c82b4e980652@gmail.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [v2] Bluetooth: btusb: Fix and detect most of the Chinese Bluetooth controllers | expand |
Hi Ismael, > For some reason they tend to squat on the very first CSR/ > Cambridge Silicon Radio VID/PID instead of paying fees. > > This is an extremely common problem; the issue goes as back as 2013 > and these devices are only getting more popular, even rebranded by > reputable vendors and sold by retailers everywhere. > > So, at this point in time there are hundreds of modern dongles reusing > the ID of what originally was an early Bluetooth 1.1 controller. > > Linux is the only place where they don't work due to spotty checks > in our detection code. It only covered a minimum subset. > > So what's the big idea? Take advantage of the fact that all CSR > chips report the same internal version as both the LMP sub-version and > HCI revision number. It always matches, couple that with the manufacturer > code, that rarely lies, and we now have a good idea of who is who. > > Additionally, by compiling a list of user-reported HCI/lsusb dumps, and > searching around for Windows drivers with similar product ranges > we can find an official superset within their .inf files. > > So it turns out that most of what it was thought to be counterfeit > was just a newer chip with different quirks. Internet shows that > this has been a problem on Windows, too. > > So, to sum things up; there are actually three classes of controllers > reusing the same 0A12:0001 VID/PID. This has been broken for a while. > > Known 'fake' bcdDevices: 0x0100, 0x0134, 0x1915, 0x2520, 0x7558, 0x8891 > IC markings on 0x7558: FR3191AHAL 749H15143 (???) > > https://bugzilla.kernel.org/show_bug.cgi?id=60824 > > Fixes: 81cac64ba258ae (Deal with USB devices that are faking CSR vendor) > Reported-by: Michał Wiśniewski <brylozketrzyn@gmail.com> > Tested-by: Ismael Ferreras Morezuelas <swyterzone@gmail.com> > Signed-off-by: Ismael Ferreras Morezuelas <swyterzone@gmail.com> > --- > > Changes in v2: > * Find a better way of detecting which type is which; scrap the wonky >> =Bluetooth 1.2 protocol check and instead do what's described above. > * Move all the quirk logic to btusb_setup_csr(), simplify it a bit. > * Use a switch statement and list all the known broken bcdDevice > instead of trying to penalize the real CSR devices. > * Add two bt_dev_info() prints because this may be important in the > future, given the amount of variables we are playing with here. > * Try to keep my comments within a 80-column limit. > > I think this makes way more sense, getting all the dongles to work without > penalizing either side. And provides a baseline to *maybe* expand the list > in the future, easily. But I'm pretty sure it should cover most of them. > > Let me know what you think. > > drivers/bluetooth/btusb.c | 97 +++++++++++++++++++++++++++++---------- > 1 file changed, 73 insertions(+), 24 deletions(-) > > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c > index 5f022e9cf..b7b8680c3 100644 > --- a/drivers/bluetooth/btusb.c > +++ b/drivers/bluetooth/btusb.c > @@ -1718,6 +1718,8 @@ static int btusb_setup_bcm92035(struct hci_dev *hdev) > > static int btusb_setup_csr(struct hci_dev *hdev) > { > + struct btusb_data *data = hci_get_drvdata(hdev); > + struct usb_device *udev = data->udev; > struct hci_rp_read_local_version *rp; > struct sk_buff *skb; > > @@ -1739,18 +1741,77 @@ static int btusb_setup_csr(struct hci_dev *hdev) > > rp = (struct hci_rp_read_local_version *)skb->data; > > - /* Detect controllers which aren't real CSR ones. */ > - if (le16_to_cpu(rp->manufacturer) != 10 || > - le16_to_cpu(rp->lmp_subver) == 0x0c5c) { > - /* Clear the reset quirk since this is not an actual > - * early Bluetooth 1.1 device from CSR. > + /* Of interest to fine-tune the logic in the future */ > + bt_dev_info(hdev, "CSR: New controller detected; bcdDevice=%#x, HCI manufacturer=%u, HCI rev=%#x, LMP subver=%#x", > + le16_to_cpu(udev->descriptor.bcdDevice), > + le16_to_cpu(rp->manufacturer), > + le16_to_cpu(rp->hci_rev), > + le16_to_cpu(rp->lmp_subver)); scrap this one. It is too noisy. > + /* Detect a wide host of Chinese controllers that rely on heavy VID/PID > + * squatting of this poor old Bluetooth 1.1 device. As if that wasn't > + * enough there are actually three classes of controllers reusing > + * the same 0A12:0001 VID/PID: > + * > + * * Old CSR Bluetooth 1.1 devices (BlueCore?): > + * HCI_QUIRK_SIMULTANEOUS_DISCOVERY > + * HCI_QUIRK_RESET_ON_CLOSE Use - instead of * for items > + * > + * * New CSR Bluetooth devices based on CSR8510 (BlueSoleil?): > + * HCI_QUIRK_BROKEN_STORED_LINK_KEY > + * Remove BlueCore and BlueSoleil from here. All CSR chips are BlueCore chips. The BlueSoleil was a host stack as far as I remember. > + * * Unbranded CSR clone: > + * Their HCI chip uses a different manufacturer number; > + * sourced from various vendors. Most common ones are: > + * - Broadcom Corporation (15) > + * - Mitel Semiconductor (16) > + * > + * No quirks, varies depending on the real manufacturer. > + * > + * We detect actual CSR devices by checking that the HDI manufacturer code > + * is Cambridge Silicon Radio (10) and ensuring that LMP sub-version and > + * the HID values will always match. The full list of newer bcdDevices > + * is documented in the official driver .inf files. > + * > + * Because diagnosing all this has been very tricky in the past, with the > + * original bug being reported in 2013, and left unsolved until 2020, let's > + * report the chip type to potentially have a better coverage and reports. > + */ > + if (le16_to_cpu(rp->manufacturer) == 10 && > + le16_to_cpu(rp->hci_rev) == le16_to_cpu(rp->lmp_subver)) { > + /* Only apply the reset quirk on actual, early Bluetooth 1.1 devices > + * from CSR. Old firmware would otherwise execute USB reset > */ > - clear_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks); > + if (le16_to_cpu(udev->descriptor.bcdDevice) < 0x117) > + set_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks); > > - /* These fake CSR controllers have all a broken > - * stored link key handling and so just disable it. > - */ > - set_bit(HCI_QUIRK_BROKEN_STORED_LINK_KEY, &hdev->quirks); > + if (udev->descriptor.idVendor == 0x0a12 && > + udev->descriptor.idProduct == 0x0001) { > + switch (udev->descriptor.bcdDevice) { > + case 0x0100: > + case 0x0134: > + case 0x1915: > + case 0x1958: > + case 0x2520: > + case 0x3164: > + case 0x4839: > + case 0x5276: > + case 0x7558: > + case 0x8891: > + /* These newer CSR controllers have all a broken > + * stored link key handling, so just disable it. > + */ > + set_bit(HCI_QUIRK_BROKEN_STORED_LINK_KEY, &hdev->quirks); > + bt_dev_info(hdev, "CSR: Modern CSR controller type detected"); Scrap the bt_dev_info here. > + break; > + default: > + /* Only apply this quirk to actual, old CSR devices */ > + set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks); > + bt_dev_info(hdev, "CSR: Old CSR controller type detected"); So you are saying the new ones can’t do LE scan and BR/EDR inquiry? That makes no sense since the “old" CSR chips are all BR/EDR only. Also no need for the bt_dev_info here. > + } > + } > + } else { > + bt_dev_info(hdev, "CSR: Unbranded CSR clone detected; adding workaround"); Use bt_dev_warn here, but scrap the workaround mention since we don’t add one. > } > > kfree_skb(skb); > @@ -3993,20 +4054,8 @@ static int btusb_probe(struct usb_interface *intf, > set_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks); > } > > - if (id->driver_info & BTUSB_CSR) { > - struct usb_device *udev = data->udev; > - u16 bcdDevice = le16_to_cpu(udev->descriptor.bcdDevice); > - > - /* Old firmware would otherwise execute USB reset */ > - if (bcdDevice < 0x117) > - set_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks); > - > - /* Fake CSR devices with broken commands */ > - if (bcdDevice <= 0x100 || bcdDevice == 0x134) > - hdev->setup = btusb_setup_csr; > - > - set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks); > - } > + if (id->driver_info & BTUSB_CSR) > + hdev->setup = btusb_setup_csr; If you do this, then please introduce BTUSB_INTEL_CSR and assign it to the one Intel controller that was using a CSR chip instead of its own silicon. Regards Marcel
Hi Ismael, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on bluetooth-next/master] [also build test WARNING on bluetooth/master v5.8-rc3 next-20200629] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use as documented in https://git-scm.com/docs/git-format-patch] url: https://github.com/0day-ci/linux/commits/Ismael-Ferreras-Morezuelas/Bluetooth-btusb-Fix-and-detect-most-of-the-Chinese-Bluetooth-controllers/20200624-083108 base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master config: parisc-randconfig-s032-20200629 (attached as .config) compiler: hppa-linux-gcc (GCC) 9.3.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # apt-get install sparse # sparse version: v0.6.2-3-gfa153962-dirty # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C= CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=parisc If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot <lkp@intel.com> sparse warnings: (new ones prefixed by >>) >> drivers/bluetooth/btusb.c:1788:37: sparse: sparse: restricted __le16 degrades to integer drivers/bluetooth/btusb.c:1789:37: sparse: sparse: restricted __le16 degrades to integer drivers/bluetooth/btusb.c:1790:49: sparse: sparse: restricted __le16 degrades to integer drivers/bluetooth/btusb.c:1790:49: sparse: sparse: restricted __le16 degrades to integer drivers/bluetooth/btusb.c:1790:49: sparse: sparse: restricted __le16 degrades to integer drivers/bluetooth/btusb.c:1790:49: sparse: sparse: restricted __le16 degrades to integer drivers/bluetooth/btusb.c:1790:49: sparse: sparse: restricted __le16 degrades to integer drivers/bluetooth/btusb.c:1790:49: sparse: sparse: restricted __le16 degrades to integer drivers/bluetooth/btusb.c:1790:49: sparse: sparse: restricted __le16 degrades to integer drivers/bluetooth/btusb.c:1790:49: sparse: sparse: restricted __le16 degrades to integer drivers/bluetooth/btusb.c:1790:49: sparse: sparse: restricted __le16 degrades to integer drivers/bluetooth/btusb.c:1790:49: sparse: sparse: restricted __le16 degrades to integer drivers/bluetooth/btusb.c:2306:25: sparse: sparse: cast to restricted __le16 drivers/bluetooth/btusb.c:2306:25: sparse: sparse: cast to restricted __le16 drivers/bluetooth/btusb.c:2306:25: sparse: sparse: cast to restricted __le16 drivers/bluetooth/btusb.c:2306:25: sparse: sparse: cast to restricted __le16 drivers/bluetooth/btusb.c:2315:25: sparse: sparse: cast to restricted __le16 drivers/bluetooth/btusb.c:2315:25: sparse: sparse: cast to restricted __le16 drivers/bluetooth/btusb.c:2315:25: sparse: sparse: cast to restricted __le16 drivers/bluetooth/btusb.c:2315:25: sparse: sparse: cast to restricted __le16 drivers/bluetooth/btusb.c:2316:25: sparse: sparse: cast to restricted __le16 drivers/bluetooth/btusb.c:2316:25: sparse: sparse: cast to restricted __le16 drivers/bluetooth/btusb.c:2316:25: sparse: sparse: cast to restricted __le16 drivers/bluetooth/btusb.c:2316:25: sparse: sparse: cast to restricted __le16 drivers/bluetooth/btusb.c:2317:25: sparse: sparse: cast to restricted __le16 drivers/bluetooth/btusb.c:2317:25: sparse: sparse: cast to restricted __le16 drivers/bluetooth/btusb.c:2317:25: sparse: sparse: cast to restricted __le16 drivers/bluetooth/btusb.c:2317:25: sparse: sparse: cast to restricted __le16 vim +1788 drivers/bluetooth/btusb.c 1718 1719 static int btusb_setup_csr(struct hci_dev *hdev) 1720 { 1721 struct btusb_data *data = hci_get_drvdata(hdev); 1722 struct usb_device *udev = data->udev; 1723 struct hci_rp_read_local_version *rp; 1724 struct sk_buff *skb; 1725 1726 BT_DBG("%s", hdev->name); 1727 1728 skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL, 1729 HCI_INIT_TIMEOUT); 1730 if (IS_ERR(skb)) { 1731 int err = PTR_ERR(skb); 1732 bt_dev_err(hdev, "CSR: Local version failed (%d)", err); 1733 return err; 1734 } 1735 1736 if (skb->len != sizeof(struct hci_rp_read_local_version)) { 1737 bt_dev_err(hdev, "CSR: Local version length mismatch"); 1738 kfree_skb(skb); 1739 return -EIO; 1740 } 1741 1742 rp = (struct hci_rp_read_local_version *)skb->data; 1743 1744 /* Of interest to fine-tune the logic in the future */ 1745 bt_dev_info(hdev, "CSR: New controller detected; bcdDevice=%#x, HCI manufacturer=%u, HCI rev=%#x, LMP subver=%#x", 1746 le16_to_cpu(udev->descriptor.bcdDevice), 1747 le16_to_cpu(rp->manufacturer), 1748 le16_to_cpu(rp->hci_rev), 1749 le16_to_cpu(rp->lmp_subver)); 1750 1751 /* Detect a wide host of Chinese controllers that rely on heavy VID/PID 1752 * squatting of this poor old Bluetooth 1.1 device. As if that wasn't 1753 * enough there are actually three classes of controllers reusing 1754 * the same 0A12:0001 VID/PID: 1755 * 1756 * * Old CSR Bluetooth 1.1 devices (BlueCore?): 1757 * HCI_QUIRK_SIMULTANEOUS_DISCOVERY 1758 * HCI_QUIRK_RESET_ON_CLOSE 1759 * 1760 * * New CSR Bluetooth devices based on CSR8510 (BlueSoleil?): 1761 * HCI_QUIRK_BROKEN_STORED_LINK_KEY 1762 * 1763 * * Unbranded CSR clone: 1764 * Their HCI chip uses a different manufacturer number; 1765 * sourced from various vendors. Most common ones are: 1766 * - Broadcom Corporation (15) 1767 * - Mitel Semiconductor (16) 1768 * 1769 * No quirks, varies depending on the real manufacturer. 1770 * 1771 * We detect actual CSR devices by checking that the HDI manufacturer code 1772 * is Cambridge Silicon Radio (10) and ensuring that LMP sub-version and 1773 * the HID values will always match. The full list of newer bcdDevices 1774 * is documented in the official driver .inf files. 1775 * 1776 * Because diagnosing all this has been very tricky in the past, with the 1777 * original bug being reported in 2013, and left unsolved until 2020, let's 1778 * report the chip type to potentially have a better coverage and reports. 1779 */ 1780 if (le16_to_cpu(rp->manufacturer) == 10 && 1781 le16_to_cpu(rp->hci_rev) == le16_to_cpu(rp->lmp_subver)) { 1782 /* Only apply the reset quirk on actual, early Bluetooth 1.1 devices 1783 * from CSR. Old firmware would otherwise execute USB reset 1784 */ 1785 if (le16_to_cpu(udev->descriptor.bcdDevice) < 0x117) 1786 set_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks); 1787 > 1788 if (udev->descriptor.idVendor == 0x0a12 && 1789 udev->descriptor.idProduct == 0x0001) { 1790 switch (udev->descriptor.bcdDevice) { 1791 case 0x0100: 1792 case 0x0134: 1793 case 0x1915: 1794 case 0x1958: 1795 case 0x2520: 1796 case 0x3164: 1797 case 0x4839: 1798 case 0x5276: 1799 case 0x7558: 1800 case 0x8891: 1801 /* These newer CSR controllers have all a broken 1802 * stored link key handling, so just disable it. 1803 */ 1804 set_bit(HCI_QUIRK_BROKEN_STORED_LINK_KEY, &hdev->quirks); 1805 bt_dev_info(hdev, "CSR: Modern CSR controller type detected"); 1806 break; 1807 default: 1808 /* Only apply this quirk to actual, old CSR devices */ 1809 set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks); 1810 bt_dev_info(hdev, "CSR: Old CSR controller type detected"); 1811 } 1812 } 1813 } else { 1814 bt_dev_info(hdev, "CSR: Unbranded CSR clone detected; adding workaround"); 1815 } 1816 1817 kfree_skb(skb); 1818 1819 return 0; 1820 } 1821 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index 5f022e9cf..b7b8680c3 100644 --- a/drivers/bluetooth/btusb.c +++ b/drivers/bluetooth/btusb.c @@ -1718,6 +1718,8 @@ static int btusb_setup_bcm92035(struct hci_dev *hdev) static int btusb_setup_csr(struct hci_dev *hdev) { + struct btusb_data *data = hci_get_drvdata(hdev); + struct usb_device *udev = data->udev; struct hci_rp_read_local_version *rp; struct sk_buff *skb; @@ -1739,18 +1741,77 @@ static int btusb_setup_csr(struct hci_dev *hdev) rp = (struct hci_rp_read_local_version *)skb->data; - /* Detect controllers which aren't real CSR ones. */ - if (le16_to_cpu(rp->manufacturer) != 10 || - le16_to_cpu(rp->lmp_subver) == 0x0c5c) { - /* Clear the reset quirk since this is not an actual - * early Bluetooth 1.1 device from CSR. + /* Of interest to fine-tune the logic in the future */ + bt_dev_info(hdev, "CSR: New controller detected; bcdDevice=%#x, HCI manufacturer=%u, HCI rev=%#x, LMP subver=%#x", + le16_to_cpu(udev->descriptor.bcdDevice), + le16_to_cpu(rp->manufacturer), + le16_to_cpu(rp->hci_rev), + le16_to_cpu(rp->lmp_subver)); + + /* Detect a wide host of Chinese controllers that rely on heavy VID/PID + * squatting of this poor old Bluetooth 1.1 device. As if that wasn't + * enough there are actually three classes of controllers reusing + * the same 0A12:0001 VID/PID: + * + * * Old CSR Bluetooth 1.1 devices (BlueCore?): + * HCI_QUIRK_SIMULTANEOUS_DISCOVERY + * HCI_QUIRK_RESET_ON_CLOSE + * + * * New CSR Bluetooth devices based on CSR8510 (BlueSoleil?): + * HCI_QUIRK_BROKEN_STORED_LINK_KEY + * + * * Unbranded CSR clone: + * Their HCI chip uses a different manufacturer number; + * sourced from various vendors. Most common ones are: + * - Broadcom Corporation (15) + * - Mitel Semiconductor (16) + * + * No quirks, varies depending on the real manufacturer. + * + * We detect actual CSR devices by checking that the HDI manufacturer code + * is Cambridge Silicon Radio (10) and ensuring that LMP sub-version and + * the HID values will always match. The full list of newer bcdDevices + * is documented in the official driver .inf files. + * + * Because diagnosing all this has been very tricky in the past, with the + * original bug being reported in 2013, and left unsolved until 2020, let's + * report the chip type to potentially have a better coverage and reports. + */ + if (le16_to_cpu(rp->manufacturer) == 10 && + le16_to_cpu(rp->hci_rev) == le16_to_cpu(rp->lmp_subver)) { + /* Only apply the reset quirk on actual, early Bluetooth 1.1 devices + * from CSR. Old firmware would otherwise execute USB reset */ - clear_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks); + if (le16_to_cpu(udev->descriptor.bcdDevice) < 0x117) + set_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks); - /* These fake CSR controllers have all a broken - * stored link key handling and so just disable it. - */ - set_bit(HCI_QUIRK_BROKEN_STORED_LINK_KEY, &hdev->quirks); + if (udev->descriptor.idVendor == 0x0a12 && + udev->descriptor.idProduct == 0x0001) { + switch (udev->descriptor.bcdDevice) { + case 0x0100: + case 0x0134: + case 0x1915: + case 0x1958: + case 0x2520: + case 0x3164: + case 0x4839: + case 0x5276: + case 0x7558: + case 0x8891: + /* These newer CSR controllers have all a broken + * stored link key handling, so just disable it. + */ + set_bit(HCI_QUIRK_BROKEN_STORED_LINK_KEY, &hdev->quirks); + bt_dev_info(hdev, "CSR: Modern CSR controller type detected"); + break; + default: + /* Only apply this quirk to actual, old CSR devices */ + set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks); + bt_dev_info(hdev, "CSR: Old CSR controller type detected"); + } + } + } else { + bt_dev_info(hdev, "CSR: Unbranded CSR clone detected; adding workaround"); } kfree_skb(skb); @@ -3993,20 +4054,8 @@ static int btusb_probe(struct usb_interface *intf, set_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks); } - if (id->driver_info & BTUSB_CSR) { - struct usb_device *udev = data->udev; - u16 bcdDevice = le16_to_cpu(udev->descriptor.bcdDevice); - - /* Old firmware would otherwise execute USB reset */ - if (bcdDevice < 0x117) - set_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks); - - /* Fake CSR devices with broken commands */ - if (bcdDevice <= 0x100 || bcdDevice == 0x134) - hdev->setup = btusb_setup_csr; - - set_bit(HCI_QUIRK_SIMULTANEOUS_DISCOVERY, &hdev->quirks); - } + if (id->driver_info & BTUSB_CSR) + hdev->setup = btusb_setup_csr; if (id->driver_info & BTUSB_SNIFFER) { struct usb_device *udev = data->udev;