2021-06-25 15:03:04

by Dan Carpenter

[permalink] [raw]
Subject: [PATCH] Bluetooth: sco: prevent information leak in sco_conn_defer_accept()

Smatch complains that some of these struct members are not initialized
leading to a stack information disclosure:

net/bluetooth/sco.c:778 sco_conn_defer_accept() warn:
check that 'cp.retrans_effort' doesn't leak information

This seems like a valid warning. I've added a default case to fix
this issue. It's sort of unusual to have case SCO_AIRMODE_CVSD,
followed by a default case but I think it's nicely readable. :)

Fixes: 2f69a82acf6f ("Bluetooth: Use voice setting in deferred SCO connection request")
Signed-off-by: Dan Carpenter <[email protected]>
---
net/bluetooth/sco.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index d9a4e88dacbb..e2ee00fea64b 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -770,6 +770,7 @@ static void sco_conn_defer_accept(struct hci_conn *conn, u16 setting)
cp.retrans_effort = 0x02;
break;
case SCO_AIRMODE_CVSD:
+ default:
cp.max_latency = cpu_to_le16(0xffff);
cp.retrans_effort = 0xff;
break;
--
2.30.2


2021-06-25 16:20:18

by bluez.test.bot

[permalink] [raw]
Subject: RE: Bluetooth: sco: prevent information leak in sco_conn_defer_accept()

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=507245

---Test result---

Test Summary:
CheckPatch FAIL 0.59 seconds
GitLint FAIL 0.13 seconds
BuildKernel PASS 689.23 seconds
TestRunner: Setup PASS 448.25 seconds
TestRunner: l2cap-tester PASS 3.23 seconds
TestRunner: bnep-tester PASS 2.14 seconds
TestRunner: mgmt-tester FAIL 37.90 seconds
TestRunner: rfcomm-tester PASS 2.62 seconds
TestRunner: sco-tester PASS 2.46 seconds
TestRunner: smp-tester PASS 2.53 seconds
TestRunner: userchan-tester PASS 2.20 seconds

Details
##############################
Test: CheckPatch - FAIL - 0.59 seconds
Run checkpatch.pl script with rule in .checkpatch.conf
Bluetooth: sco: prevent information leak in sco_conn_defer_accept()
WARNING: Unknown commit id '2f69a82acf6f', maybe rebased or not pulled?
#17:
Fixes: 2f69a82acf6f ("Bluetooth: Use voice setting in deferred SCO connection request")

total: 0 errors, 1 warnings, 0 checks, 7 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: sco: prevent information leak 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: GitLint - FAIL - 0.13 seconds
Run gitlint with rule in .gitlint
Bluetooth: sco: prevent information leak in sco_conn_defer_accept()
13: B1 Line exceeds max length (87>80): "Fixes: 2f69a82acf6f ("Bluetooth: Use voice setting in deferred SCO connection request")"


##############################
Test: BuildKernel - PASS - 689.23 seconds
Build Kernel with minimal configuration supports Bluetooth


##############################
Test: TestRunner: Setup - PASS - 448.25 seconds
Setup environment for running Test Runner


##############################
Test: TestRunner: l2cap-tester - PASS - 3.23 seconds
Run test-runner with l2cap-tester
Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: bnep-tester - PASS - 2.14 seconds
Run test-runner with bnep-tester
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: mgmt-tester - FAIL - 37.90 seconds
Run test-runner with mgmt-tester
Total: 446, Passed: 434 (97.3%), Failed: 7, Not Run: 5

Failed Test Cases
Read Ext Controller Info 1 Failed 0.019 seconds
Read Ext Controller Info 2 Failed 0.028 seconds
Read Ext Controller Info 3 Failed 0.021 seconds
Read Ext Controller Info 4 Failed 0.021 seconds
Read Ext Controller Info 5 Failed 0.025 seconds
Add Ext Advertising - Success 21 (Timeout expires) Timed out 3.097 seconds
Multi Ext Advertising - Success 1 Timed out 2.362 seconds

##############################
Test: TestRunner: rfcomm-tester - PASS - 2.62 seconds
Run test-runner with rfcomm-tester
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: sco-tester - PASS - 2.46 seconds
Run test-runner with sco-tester
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: smp-tester - PASS - 2.53 seconds
Run test-runner with smp-tester
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: userchan-tester - PASS - 2.20 seconds
Run test-runner with userchan-tester
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0



---
Regards,
Linux Bluetooth


Attachments:
l2cap-tester.log (43.31 kB)
bnep-tester.log (3.47 kB)
mgmt-tester.log (599.52 kB)
rfcomm-tester.log (11.40 kB)
sco-tester.log (9.68 kB)
smp-tester.log (11.54 kB)
userchan-tester.log (5.33 kB)
Download all attachments

2021-06-25 20:02:42

by Dan Carpenter

[permalink] [raw]
Subject: Re: Bluetooth: sco: prevent information leak in sco_conn_defer_accept()

On Fri, Jun 25, 2021 at 09:17:26AM -0700, [email protected] wrote:
> ##############################
> Test: CheckPatch - FAIL - 0.59 seconds
> Run checkpatch.pl script with rule in .checkpatch.conf
> Bluetooth: sco: prevent information leak in sco_conn_defer_accept()
> WARNING: Unknown commit id '2f69a82acf6f', maybe rebased or not pulled?

I double checked the commit and it's correct. It's from 2013 so it's
not clear how the bot got confused.

regards,
dan carpenter

2021-06-25 20:35:44

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: Bluetooth: sco: prevent information leak in sco_conn_defer_accept()

Hi Dan,

On Fri, Jun 25, 2021 at 1:02 PM Dan Carpenter <[email protected]> wrote:
>
> On Fri, Jun 25, 2021 at 09:17:26AM -0700, [email protected] wrote:
> > ##############################
> > Test: CheckPatch - FAIL - 0.59 seconds
> > Run checkpatch.pl script with rule in .checkpatch.conf
> > Bluetooth: sco: prevent information leak in sco_conn_defer_accept()
> > WARNING: Unknown commit id '2f69a82acf6f', maybe rebased or not pulled?
>
> I double checked the commit and it's correct. It's from 2013 so it's
> not clear how the bot got confused.

Yep, Ive seen this before with my own patches, perhaps there is
something with the CI tree which prevents checkpatch to locate the
commit id or something like that, @An, Tedd please have a look at CI
environment what could be causing this problem.

--
Luiz Augusto von Dentz

2021-07-08 07:13:44

by bluez.test.bot

[permalink] [raw]
Subject: RE: Bluetooth: sco: prevent information leak in sco_conn_defer_accept()

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=507245

---Test result---

Test Summary:
CheckPatch FAIL 0.45 seconds
GitLint FAIL 0.10 seconds
BuildKernel PASS 512.78 seconds
TestRunner: Setup PASS 342.18 seconds
TestRunner: l2cap-tester PASS 2.73 seconds
TestRunner: bnep-tester PASS 1.97 seconds
TestRunner: mgmt-tester PASS 30.46 seconds
TestRunner: rfcomm-tester PASS 2.13 seconds
TestRunner: sco-tester PASS 2.07 seconds
TestRunner: smp-tester FAIL 2.08 seconds
TestRunner: userchan-tester PASS 2.02 seconds

Details
##############################
Test: CheckPatch - FAIL - 0.45 seconds
Run checkpatch.pl script with rule in .checkpatch.conf
Bluetooth: sco: prevent information leak in sco_conn_defer_accept()
WARNING: Unknown commit id '2f69a82acf6f', maybe rebased or not pulled?
#17:
Fixes: 2f69a82acf6f ("Bluetooth: Use voice setting in deferred SCO connection request")

total: 0 errors, 1 warnings, 0 checks, 7 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: sco: prevent information leak 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: GitLint - FAIL - 0.10 seconds
Run gitlint with rule in .gitlint
Bluetooth: sco: prevent information leak in sco_conn_defer_accept()
13: B1 Line exceeds max length (87>80): "Fixes: 2f69a82acf6f ("Bluetooth: Use voice setting in deferred SCO connection request")"


##############################
Test: BuildKernel - PASS - 512.78 seconds
Build Kernel with minimal configuration supports Bluetooth


##############################
Test: TestRunner: Setup - PASS - 342.18 seconds
Setup environment for running Test Runner


##############################
Test: TestRunner: l2cap-tester - PASS - 2.73 seconds
Run test-runner with l2cap-tester
Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: bnep-tester - PASS - 1.97 seconds
Run test-runner with bnep-tester
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: mgmt-tester - PASS - 30.46 seconds
Run test-runner with mgmt-tester
Total: 446, Passed: 443 (99.3%), Failed: 0, Not Run: 3

##############################
Test: TestRunner: rfcomm-tester - PASS - 2.13 seconds
Run test-runner with rfcomm-tester
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: sco-tester - PASS - 2.07 seconds
Run test-runner with sco-tester
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: TestRunner: smp-tester - FAIL - 2.08 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.020 seconds

##############################
Test: TestRunner: userchan-tester - PASS - 2.02 seconds
Run test-runner with userchan-tester
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0



---
Regards,
Linux Bluetooth


Attachments:
l2cap-tester.log (43.31 kB)
bnep-tester.log (3.47 kB)
mgmt-tester.log (600.00 kB)
rfcomm-tester.log (11.40 kB)
sco-tester.log (9.68 kB)
smp-tester.log (11.43 kB)
userchan-tester.log (5.32 kB)
Download all attachments

2021-07-22 14:16:10

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH] Bluetooth: sco: prevent information leak in sco_conn_defer_accept()

Hi Dan,

> Smatch complains that some of these struct members are not initialized
> leading to a stack information disclosure:
>
> net/bluetooth/sco.c:778 sco_conn_defer_accept() warn:
> check that 'cp.retrans_effort' doesn't leak information
>
> This seems like a valid warning. I've added a default case to fix
> this issue. It's sort of unusual to have case SCO_AIRMODE_CVSD,
> followed by a default case but I think it's nicely readable. :)
>
> Fixes: 2f69a82acf6f ("Bluetooth: Use voice setting in deferred SCO connection request")
> Signed-off-by: Dan Carpenter <[email protected]>
> ---
> net/bluetooth/sco.c | 1 +
> 1 file changed, 1 insertion(+)

I actually prefer a separate default statement since otherwise I get confused. Your patch with that minor change has been applied to bluetooth-next tree.

Regards

Marcel