RE: Bluetooth: MGMT: Add length check in confirm_name

RE: Bluetooth: MGMT: Add length check in confirm_name

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1903 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=1124368

---Test result---

Test Summary:
CheckPatch                    PASS      2.59 seconds
VerifyFixes                   PASS      0.18 seconds
VerifySignedoff               PASS      0.13 seconds
GitLint                       PASS      0.36 seconds
SubjectPrefix                 PASS      0.12 seconds
BuildKernel                   PASS      25.29 seconds
CheckAllWarning               PASS      28.05 seconds
CheckSparse                   PASS      28.63 seconds
BuildKernel32                 PASS      25.20 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      459.03 seconds
TestRunner_mgmt-tester        FAIL      208.06 seconds
TestRunner_mesh-tester        FAIL      25.12 seconds
IncrementalBuild              PASS      24.14 seconds

Details
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 494, Passed: 489 (99.0%), Failed: 1, Not Run: 4

Failed Test Cases
Read Exp Feature - Success                           Failed       0.234 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Timed out    2.128 seconds
Mesh - Send cancel - 2                               Timed out    1.988 seconds


https://github.com/bluez/bluetooth-next/pull/416

---
Regards,
Linux Bluetooth

[PATCH] Bluetooth: MGMT: Add length check in confirm_name

[caitao]

From: caitao <caitao@kylinos.cn>

The confirm_name function did not validate the length of the incoming
data before accessing the mgmt_cp_confirm_name structure, which could
lead to out-of-bounds reads if a short packet was sent.

Add a length check at the beginning of the function to ensure the data
is at least sizeof(struct mgmt_cp_confirm_name). If the check fails,
return MGMT_STATUS_INVALID_PARAMS without accessing the invalid data.

Signed-off-by: caitao <caitao@kylinos.cn>
---
 net/bluetooth/mgmt.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index a0a491e5311e..aab8e64283aa 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -6271,6 +6271,10 @@ static int confirm_name(struct sock *sk, struct hci_dev *hdev, void *data,
 
 	bt_dev_dbg(hdev, "sock %p", sk);
 
+	if (len < sizeof(*cp))
+		return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_CONFIRM_NAME,
+					 MGMT_STATUS_INVALID_PARAMS, NULL, 0);
+
 	hci_dev_lock(hdev);
 
 	if (!hci_discovery_active(hdev)) {
-- 
2.43.0

RE: Bluetooth: MGMT: Add length check in confirm_name

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1903 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=1127042

---Test result---

Test Summary:
CheckPatch                    PASS      0.76 seconds
VerifyFixes                   PASS      0.10 seconds
VerifySignedoff               PASS      0.09 seconds
GitLint                       PASS      0.24 seconds
SubjectPrefix                 PASS      0.09 seconds
BuildKernel                   PASS      22.68 seconds
CheckAllWarning               PASS      25.41 seconds
CheckSparse                   PASS      24.71 seconds
BuildKernel32                 PASS      22.49 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      416.57 seconds
TestRunner_mgmt-tester        FAIL      204.83 seconds
TestRunner_mesh-tester        FAIL      23.76 seconds
IncrementalBuild              PASS      23.30 seconds

Details
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 494, Passed: 489 (99.0%), Failed: 1, Not Run: 4

Failed Test Cases
Read Exp Feature - Success                           Failed       0.234 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Timed out    1.974 seconds
Mesh - Send cancel - 2                               Timed out    1.988 seconds


https://github.com/bluez/bluetooth-next/pull/434

---
Regards,
Linux Bluetooth

Re: [PATCH] Bluetooth: MGMT: Add length check in confirm_name

[Paul Menzel]

Dear caitao,


Thank you for your patch.

Am 14.07.26 um 04:52 schrieb caitao:
> From: caitao <caitao@kylinos.cn>

Should this be spelled Cai Tao, or do you just have one name? It’d be 
great if you updated the spelling.

> The confirm_name function did not validate the length of the incoming
> data before accessing the mgmt_cp_confirm_name structure, which could
> lead to out-of-bounds reads if a short packet was sent.
> 
> Add a length check at the beginning of the function to ensure the data
> is at least sizeof(struct mgmt_cp_confirm_name). If the check fails,
> return MGMT_STATUS_INVALID_PARAMS without accessing the invalid data.

What will be logged in this case? Should an additional error be logged 
for a better user and developer experience?

> Signed-off-by: caitao <caitao@kylinos.cn>
> ---
>   net/bluetooth/mgmt.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> index a0a491e5311e..aab8e64283aa 100644
> --- a/net/bluetooth/mgmt.c
> +++ b/net/bluetooth/mgmt.c
> @@ -6271,6 +6271,10 @@ static int confirm_name(struct sock *sk, struct hci_dev *hdev, void *data,
>   
>   	bt_dev_dbg(hdev, "sock %p", sk);
>   
> +	if (len < sizeof(*cp))
> +		return mgmt_cmd_complete(sk, hdev->id, MGMT_OP_CONFIRM_NAME,
> +					 MGMT_STATUS_INVALID_PARAMS, NULL, 0);
> +

gemini/gemini-3.1-pro-preview comments [1]:

> Is this length check actually reachable?
> 
> The framework dispatcher hci_mgmt_cmd() in net/bluetooth/hci_sock.c already
> enforces strict length validation before the handler is invoked. Since
> confirm_name() is registered with a fixed length and without the
> HCI_MGMT_VAR_LEN flag, the framework ensures len == sizeof(*cp) before
> confirm_name() is ever called.
> 
> Additionally, if this error path were somehow reachable, would returning a
> 0-byte payload for a Command Complete event violate the API? It looks like
> returning a NULL payload with a length of 0 instead of the expected
> struct mgmt_rp_confirm_name could cause userspace parsing errors. Should this
> instead use mgmt_cmd_status() for early parameter validation failures?


>   	hci_dev_lock(hdev);
>   
>   	if (!hci_discovery_active(hdev)) {


Kind regards,

Paul


[1]: 
https://sashiko.dev/#/patchset/tencent_406D2B6F64B41152DB435F3A88E40205340A%40qq.com