diff mbox series

Bluetooth: sco: prevent information leak in sco_conn_defer_accept()

Message ID YNXveZZwzS3crmHH@mwanda (mailing list archive)
State New, archived
Headers show
Series Bluetooth: sco: prevent information leak in sco_conn_defer_accept() | expand

Commit Message

Dan Carpenter June 25, 2021, 3 p.m. UTC
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 <dan.carpenter@oracle.com>
---
 net/bluetooth/sco.c | 1 +
 1 file changed, 1 insertion(+)

Comments

bluez.test.bot@gmail.com June 25, 2021, 4:17 p.m. UTC | #1
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
Dan Carpenter June 25, 2021, 7:59 p.m. UTC | #2
On Fri, Jun 25, 2021 at 09:17:26AM -0700, bluez.test.bot@gmail.com 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
Luiz Augusto von Dentz June 25, 2021, 8:33 p.m. UTC | #3
Hi Dan,

On Fri, Jun 25, 2021 at 1:02 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> On Fri, Jun 25, 2021 at 09:17:26AM -0700, bluez.test.bot@gmail.com 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.
bluez.test.bot@gmail.com July 8, 2021, 7:11 a.m. UTC | #4
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
Marcel Holtmann July 22, 2021, 2:14 p.m. UTC | #5
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 <dan.carpenter@oracle.com>
> ---
> 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
diff mbox series

Patch

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;