* [PATCH] Bluetooth: mgmt: Pessimize compile-time bounds-check
@ 2021-08-18 4:39 Kees Cook
2021-08-18 5:16 ` bluez.test.bot
2021-08-19 14:52 ` [PATCH] " Marcel Holtmann
0 siblings, 2 replies; 4+ messages in thread
From: Kees Cook @ 2021-08-18 4:39 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Kees Cook, Johan Hedberg, Luiz Augusto von Dentz, David S. Miller,
Jakub Kicinski, linux-bluetooth, netdev, linux-kernel,
linux-hardening
After gaining __alloc_size hints, GCC thinks it can reach a memcpy()
with eir_len == 0 (since it can't see into the rewrite of status).
Instead, check eir_len == 0, avoiding this future warning:
In function 'eir_append_data',
inlined from 'read_local_oob_ext_data_complete' at net/bluetooth/mgmt.c:7210:12:
./include/linux/fortify-string.h:54:29: warning: '__builtin_memcpy' offset 5 is out of the bounds [0, 3] [-Warray-bounds]
...
net/bluetooth/hci_request.h:133:2: note: in expansion of macro 'memcpy'
133 | memcpy(&eir[eir_len], data, data_len);
| ^~~~~~
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Cc: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: linux-bluetooth@vger.kernel.org
Cc: netdev@vger.kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
net/bluetooth/mgmt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 1e21e014efd2..cea01e275f1e 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -7204,7 +7204,7 @@ static void read_local_oob_ext_data_complete(struct hci_dev *hdev, u8 status,
if (!mgmt_rp)
goto done;
- if (status)
+ if (eir_len == 0)
goto send_rsp;
eir_len = eir_append_data(mgmt_rp->eir, 0, EIR_CLASS_OF_DEV,
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread* RE: Bluetooth: mgmt: Pessimize compile-time bounds-check 2021-08-18 4:39 [PATCH] Bluetooth: mgmt: Pessimize compile-time bounds-check Kees Cook @ 2021-08-18 5:16 ` bluez.test.bot 2021-08-18 6:20 ` Kees Cook 2021-08-19 14:52 ` [PATCH] " Marcel Holtmann 1 sibling, 1 reply; 4+ messages in thread From: bluez.test.bot @ 2021-08-18 5:16 UTC (permalink / raw) To: linux-bluetooth, keescook [-- Attachment #1: Type: text/plain, Size: 3722 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=533133 ---Test result--- Test Summary: CheckPatch FAIL 0.46 seconds GitLint FAIL 0.11 seconds BuildKernel PASS 536.10 seconds TestRunner: Setup PASS 347.74 seconds TestRunner: l2cap-tester PASS 2.56 seconds TestRunner: bnep-tester PASS 1.90 seconds TestRunner: mgmt-tester PASS 30.68 seconds TestRunner: rfcomm-tester PASS 2.08 seconds TestRunner: sco-tester PASS 2.02 seconds TestRunner: smp-tester FAIL 2.06 seconds TestRunner: userchan-tester PASS 1.98 seconds Details ############################## Test: CheckPatch - FAIL - 0.46 seconds Run checkpatch.pl script with rule in .checkpatch.conf Bluetooth: mgmt: Pessimize compile-time bounds-check WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line) #11: inlined from 'read_local_oob_ext_data_complete' at net/bluetooth/mgmt.c:7210:12: total: 0 errors, 1 warnings, 0 checks, 8 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: mgmt: Pessimize compile-time bounds-check" 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: GitLint - FAIL - 0.11 seconds Run gitlint with rule in .gitlint Bluetooth: mgmt: Pessimize compile-time bounds-check 8: B1 Line exceeds max length (84>80): " inlined from 'read_local_oob_ext_data_complete' at net/bluetooth/mgmt.c:7210:12:" 9: B1 Line exceeds max length (121>80): "./include/linux/fortify-string.h:54:29: warning: '__builtin_memcpy' offset 5 is out of the bounds [0, 3] [-Warray-bounds]" ############################## Test: BuildKernel - PASS - 536.10 seconds Build Kernel with minimal configuration supports Bluetooth ############################## Test: TestRunner: Setup - PASS - 347.74 seconds Setup environment for running Test Runner ############################## Test: TestRunner: l2cap-tester - PASS - 2.56 seconds Run test-runner with l2cap-tester Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0 ############################## Test: TestRunner: bnep-tester - PASS - 1.90 seconds Run test-runner with bnep-tester Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0 ############################## Test: TestRunner: mgmt-tester - PASS - 30.68 seconds Run test-runner with mgmt-tester Total: 448, Passed: 445 (99.3%), Failed: 0, Not Run: 3 ############################## Test: TestRunner: rfcomm-tester - PASS - 2.08 seconds Run test-runner with rfcomm-tester Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0 ############################## Test: TestRunner: sco-tester - PASS - 2.02 seconds Run test-runner with sco-tester Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 ############################## Test: TestRunner: smp-tester - FAIL - 2.06 seconds Run test-runner with smp-tester Total: 8, Passed: 7 (87.5%), Failed: 1, Not Run: 0 Failed Test Cases SMP Client - SC Request 2 Failed 0.021 seconds ############################## Test: TestRunner: userchan-tester - PASS - 1.98 seconds Run test-runner with userchan-tester Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0 --- Regards, Linux Bluetooth [-- Attachment #2: l2cap-tester.log --] [-- Type: application/octet-stream, Size: 44386 bytes --] [-- Attachment #3: bnep-tester.log --] [-- Type: application/octet-stream, Size: 3593 bytes --] [-- Attachment #4: mgmt-tester.log --] [-- Type: application/octet-stream, Size: 616863 bytes --] [-- Attachment #5: rfcomm-tester.log --] [-- Type: application/octet-stream, Size: 11713 bytes --] [-- Attachment #6: sco-tester.log --] [-- Type: application/octet-stream, Size: 9948 bytes --] [-- Attachment #7: smp-tester.log --] [-- Type: application/octet-stream, Size: 11741 bytes --] [-- Attachment #8: userchan-tester.log --] [-- Type: application/octet-stream, Size: 5489 bytes --] ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Bluetooth: mgmt: Pessimize compile-time bounds-check 2021-08-18 5:16 ` bluez.test.bot @ 2021-08-18 6:20 ` Kees Cook 0 siblings, 0 replies; 4+ messages in thread From: Kees Cook @ 2021-08-18 6:20 UTC (permalink / raw) To: linux-bluetooth On Tue, Aug 17, 2021 at 10:16:53PM -0700, bluez.test.bot@gmail.com wrote: > This is automated email and please do not reply to this email! ... I think I will though. :) > 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=533133 > > ---Test result--- > > Test Summary: > CheckPatch FAIL 0.46 seconds > GitLint FAIL 0.11 seconds > BuildKernel PASS 536.10 seconds > TestRunner: Setup PASS 347.74 seconds > TestRunner: l2cap-tester PASS 2.56 seconds > TestRunner: bnep-tester PASS 1.90 seconds > TestRunner: mgmt-tester PASS 30.68 seconds > TestRunner: rfcomm-tester PASS 2.08 seconds > TestRunner: sco-tester PASS 2.02 seconds > TestRunner: smp-tester FAIL 2.06 seconds > TestRunner: userchan-tester PASS 1.98 seconds > > Details > ############################## > Test: CheckPatch - FAIL - 0.46 seconds > Run checkpatch.pl script with rule in .checkpatch.conf > Bluetooth: mgmt: Pessimize compile-time bounds-check > WARNING: Possible unwrapped commit description (prefer a maximum 75 chars per line) > #11: > inlined from 'read_local_oob_ext_data_complete' at net/bluetooth/mgmt.c:7210:12: This is a literal gcc warning output, so wrapping shouldn't happen. > > total: 0 errors, 1 warnings, 0 checks, 8 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: mgmt: Pessimize compile-time bounds-check" 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: GitLint - FAIL - 0.11 seconds > Run gitlint with rule in .gitlint > Bluetooth: mgmt: Pessimize compile-time bounds-check > 8: B1 Line exceeds max length (84>80): " inlined from 'read_local_oob_ext_data_complete' at net/bluetooth/mgmt.c:7210:12:" > 9: B1 Line exceeds max length (121>80): "./include/linux/fortify-string.h:54:29: warning: '__builtin_memcpy' offset 5 is out of the bounds [0, 3] [-Warray-bounds]" Same. > > > ############################## > Test: BuildKernel - PASS - 536.10 seconds > Build Kernel with minimal configuration supports Bluetooth > > > ############################## > Test: TestRunner: Setup - PASS - 347.74 seconds > Setup environment for running Test Runner > > > ############################## > Test: TestRunner: l2cap-tester - PASS - 2.56 seconds > Run test-runner with l2cap-tester > Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0 > > ############################## > Test: TestRunner: bnep-tester - PASS - 1.90 seconds > Run test-runner with bnep-tester > Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0 > > ############################## > Test: TestRunner: mgmt-tester - PASS - 30.68 seconds > Run test-runner with mgmt-tester > Total: 448, Passed: 445 (99.3%), Failed: 0, Not Run: 3 > > ############################## > Test: TestRunner: rfcomm-tester - PASS - 2.08 seconds > Run test-runner with rfcomm-tester > Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0 > > ############################## > Test: TestRunner: sco-tester - PASS - 2.02 seconds > Run test-runner with sco-tester > Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 > > ############################## > Test: TestRunner: smp-tester - FAIL - 2.06 seconds > Run test-runner with smp-tester > Total: 8, Passed: 7 (87.5%), Failed: 1, Not Run: 0 > > Failed Test Cases > SMP Client - SC Request 2 Failed 0.021 seconds ? Any details on this? > > ############################## > Test: TestRunner: userchan-tester - PASS - 1.98 seconds > Run test-runner with userchan-tester > Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0 > > > > --- > Regards, > Linux Bluetooth > -- Kees Cook ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Bluetooth: mgmt: Pessimize compile-time bounds-check 2021-08-18 4:39 [PATCH] Bluetooth: mgmt: Pessimize compile-time bounds-check Kees Cook 2021-08-18 5:16 ` bluez.test.bot @ 2021-08-19 14:52 ` Marcel Holtmann 1 sibling, 0 replies; 4+ messages in thread From: Marcel Holtmann @ 2021-08-19 14:52 UTC (permalink / raw) To: Kees Cook Cc: Johan Hedberg, Luiz Augusto von Dentz, David S. Miller, Jakub Kicinski, open list:BLUETOOTH SUBSYSTEM, open list:NETWORKING [GENERAL], open list, linux-hardening Hi Kees, > After gaining __alloc_size hints, GCC thinks it can reach a memcpy() > with eir_len == 0 (since it can't see into the rewrite of status). > Instead, check eir_len == 0, avoiding this future warning: > > In function 'eir_append_data', > inlined from 'read_local_oob_ext_data_complete' at net/bluetooth/mgmt.c:7210:12: > ./include/linux/fortify-string.h:54:29: warning: '__builtin_memcpy' offset 5 is out of the bounds [0, 3] [-Warray-bounds] > ... > net/bluetooth/hci_request.h:133:2: note: in expansion of macro 'memcpy' > 133 | memcpy(&eir[eir_len], data, data_len); > | ^~~~~~ > > Cc: Marcel Holtmann <marcel@holtmann.org> > Cc: Johan Hedberg <johan.hedberg@gmail.com> > Cc: Luiz Augusto von Dentz <luiz.dentz@gmail.com> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Jakub Kicinski <kuba@kernel.org> > Cc: linux-bluetooth@vger.kernel.org > Cc: netdev@vger.kernel.org > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > net/bluetooth/mgmt.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) patch has been applied to bluetooth-next tree. Regards Marcel ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-08-19 14:52 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-08-18 4:39 [PATCH] Bluetooth: mgmt: Pessimize compile-time bounds-check Kees Cook 2021-08-18 5:16 ` bluez.test.bot 2021-08-18 6:20 ` Kees Cook 2021-08-19 14:52 ` [PATCH] " Marcel Holtmann
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox