linux-kernel - RE: [PATCH v4] Bluetooth: btbcm: Apply HCI_QUIRK_BROKEN_READ_TRANSMIT

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]

Date: Fri, 28 Jun 2024 05:42:20 +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 v4] Bluetooth: btbcm: Apply
 HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER to CYW4373

Hello,

Please let me know if there are any additional actions needed from my end to facilitate progress on this patch.
Your feedback would be greatly appreciated.

Thank you for your attention to this matter.

Best Regards,
Nobuaki Tsunashima

-----Original Message-----
From: Tsunashima Nobuaki (SMD C3 JP RM WLS AE) 
Sent: Monday, May 27, 2024 10:59 AM
To: 'Luiz Augusto von Dentz' <luiz.dentz@...il.com>
Cc: Marcel Holtmann <marcel@...tmann.org>; linux-bluetooth@...r.kernel.org; linux-kernel@...r.kernel.org
Subject: RE: [PATCH v4] Bluetooth: btbcm: Apply HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER to CYW4373

Hi Luiz,

Thanks for your review.

>>  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);
>> -
>> +       chip_id = skb->data[1];
>> +       baseline = skb->data[3] | (skb->data[4] << 8);
>
>This is not really safe, you shouldn't attempt to access skb->data without first checking skb->len, actually it would be much better that >you would use skb_pull_data which does skb->len check before pulling data.

I think it could be safe because its length is checked inside btbcm_read_verbose_config() as below.
Please let me know if further checking is needed.

>>>
static struct sk_buff *btbcm_read_verbose_config(struct hci_dev *hdev) {
	struct sk_buff *skb;

	skb = __hci_cmd_sync(hdev, 0xfc79, 0, NULL, HCI_INIT_TIMEOUT);
	if (IS_ERR(skb)) {
		bt_dev_err(hdev, "BCM: Read verbose config info failed (%ld)",
			   PTR_ERR(skb));
		return skb;
	}

	if (skb->len != 7) {
		bt_dev_err(hdev, "BCM: Verbose config length mismatch");
		kfree_skb(skb);
		return ERR_PTR(-EIO);
	}

	return skb;
}
<<<

Best Regards,
Nobuaki Tsunashima