[PATCH] Bluetooth: btintel: prevent buffer overflow in btintel_read_version_tlv()

[PATCH] Bluetooth: btintel: prevent buffer overflow in btintel_read_version_tlv()

[Dan Carpenter]

Smatch says that "tlv->len" comes from skb->data and so it's untrusted.
It can be 0-255 which is more than the size of "version->otp_bd_addr"
which is 6 bytes so the memcpy() could lead to memory corruption.

drivers/bluetooth/btintel.c:583 btintel_read_version_tlv() error: '__memcpy()' '&version->otp_bd_addr' too small (6 vs 255)

Fix this by clamping the length to sizeof(version->otp_bd_addr).

Fixes: 57375beef71a ("Bluetooth: btintel: Add infrastructure to read controller information")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/bluetooth/btintel.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index e44b6993cf91..654288e974b0 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -515,6 +515,7 @@ int btintel_read_version_tlv(struct hci_dev *hdev, struct intel_version_tlv *ver
 	 */
 	while (skb->len) {
 		struct intel_tlv *tlv;
+		int len;
 
 		tlv = (struct intel_tlv *)skb->data;
 		switch (tlv->type) {
@@ -580,7 +581,8 @@ int btintel_read_version_tlv(struct hci_dev *hdev, struct intel_version_tlv *ver
 			version->sbe_type = tlv->val[0];
 			break;
 		case INTEL_TLV_OTP_BDADDR:
-			memcpy(&version->otp_bd_addr, tlv->val, tlv->len);
+			len = min_t(int, tlv->len, sizeof(version->otp_bd_addr));
+			memcpy(&version->otp_bd_addr, tlv->val, len);
 			break;
 		default:
 			/* Ignore rest of information */
-- 
2.30.2

RE: Bluetooth: btintel: prevent buffer overflow in btintel_read_version_tlv()

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 3113 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=464363

---Test result---

##############################
Test: CheckPatch - FAIL
Bluetooth: btintel: prevent buffer overflow in btintel_read_version_tlv()
WARNING: Unknown commit id '57375beef71a', maybe rebased or not pulled?
#15: 
Fixes: 57375beef71a ("Bluetooth: btintel: Add infrastructure to read controller information")

total: 0 errors, 1 warnings, 16 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

"[PATCH] Bluetooth: btintel: prevent buffer overflow in" has style problems, please review.

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: CheckGitLint - FAIL
Bluetooth: btintel: prevent buffer overflow in btintel_read_version_tlv()
1: T1 Title exceeds max length (73>72): "Bluetooth: btintel: prevent buffer overflow in btintel_read_version_tlv()"
7: B1 Line exceeds max length (123>80): "drivers/bluetooth/btintel.c:583 btintel_read_version_tlv() error: '__memcpy()' '&version->otp_bd_addr' too small (6 vs 255)"
11: B1 Line exceeds max length (93>80): "Fixes: 57375beef71a ("Bluetooth: btintel: Add infrastructure to read controller information")"


##############################
Test: CheckBuildK - PASS


##############################
Test: CheckTestRunner: Setup - PASS


##############################
Test: CheckTestRunner: l2cap-tester - PASS
Total: 40, Passed: 34 (85.0%), Failed: 0, Not Run: 6

##############################
Test: CheckTestRunner: bnep-tester - PASS
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: mgmt-tester - FAIL
Total: 416, Passed: 396 (95.2%), Failed: 6, Not Run: 14

Failed Test Cases
Set connectable off (LE) - Success 2                 Failed       0.028 seconds
Set connectable off (LE) - Success 3                 Failed       0.028 seconds
Set connectable off (LE) - Success 4                 Failed       0.028 seconds
Add Advertising - Success 13 (ADV_SCAN_IND)          Failed       0.020 seconds
Add Advertising - Success 14 (ADV_NONCONN_IND)       Failed       0.024 seconds
Add Advertising - Success 17 (Connectable -> off)    Failed       0.032 seconds

##############################
Test: CheckTestRunner: rfcomm-tester - PASS
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: sco-tester - PASS
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: smp-tester - PASS
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: userchan-tester - PASS
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0



---
Regards,
Linux Bluetooth


[-- Attachment #2: l2cap-tester.log --]
[-- Type: application/octet-stream, Size: 43345 bytes --]

[-- Attachment #3: bnep-tester.log --]
[-- Type: application/octet-stream, Size: 3535 bytes --]

[-- Attachment #4: mgmt-tester.log --]
[-- Type: application/octet-stream, Size: 547604 bytes --]

[-- Attachment #5: rfcomm-tester.log --]
[-- Type: application/octet-stream, Size: 11656 bytes --]

[-- Attachment #6: sco-tester.log --]
[-- Type: application/octet-stream, Size: 9891 bytes --]

[-- Attachment #7: smp-tester.log --]
[-- Type: application/octet-stream, Size: 11802 bytes --]

[-- Attachment #8: userchan-tester.log --]
[-- Type: application/octet-stream, Size: 5432 bytes --]

Re: Bluetooth: btintel: prevent buffer overflow in btintel_read_version_tlv()

[Dan Carpenter]

On Fri, Apr 09, 2021 at 06:28:24AM -0700, bluez.test.bot@gmail.com wrote:
> This is automated email and please do not reply to this email!
  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

LOL, just did! #ANARCHIST

> 
> Dear submitter,
> 
> Thank you for submitting the patches to the linux bluetooth mailing list.
> This is a CI test results with your patch series:
> PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=464363 
> 
> ---Test result---
> 
> ##############################
> Test: CheckPatch - FAIL
> Bluetooth: btintel: prevent buffer overflow in btintel_read_version_tlv()
> WARNING: Unknown commit id '57375beef71a', maybe rebased or not pulled?
                              ^^^^^^^^^^^^
This commit is from last Sept so probably the problem is on your end.

> #15: 
> Fixes: 57375beef71a ("Bluetooth: btintel: Add infrastructure to read controller information")
> 
> total: 0 errors, 1 warnings, 16 lines checked

regards,
dan carpenter