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]

Message-ID: <f7a5e281c48b4232a4ba74022dd3f83a@infineon.com>
Date: Mon, 27 May 2024 01:59:12 +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

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