[PATCH 1/2] Bluetooth: hci_core: Return -ENOMEM when the sent_cmd clone fails

[PATCH 1/2] Bluetooth: hci_core: Return -ENOMEM when the sent_cmd clone fails

[Hans de Goede]

From: Ibrahim Abdelkader <iabdelka@qti.qualcomm.com>

hci_send_cmd_sync() returns -EINVAL when skb_clone() fails for sent_cmd,
which describes an invalid argument rather than an allocation failure.

Return -ENOMEM instead. The only caller, hci_cmd_work(), tests the result
for zero, so there is no functional change.

Signed-off-by: Ibrahim Abdelkader <iabdelka@qti.qualcomm.com>
Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
---
 net/bluetooth/hci_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 5ba9fe8261ec..8aeb3024428b 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -4073,7 +4073,7 @@ static int hci_send_cmd_sync(struct hci_dev *hdev, struct sk_buff *skb)
 	if (!hdev->sent_cmd) {
 		skb_queue_head(&hdev->cmd_q, skb);
 		queue_work(hdev->workqueue, &hdev->cmd_work);
-		return -EINVAL;
+		return -ENOMEM;
 	}
 
 	if (hci_skb_opcode(skb) != HCI_OP_NOP) {
-- 
2.55.0

[PATCH 2/2] Bluetooth: hci_core: Handle skb_clone() failure in hci_send_cmd_sync()

[Hans de Goede]

From: Ibrahim Abdelkader <iabdelka@qti.qualcomm.com>

hci_send_cmd_sync() does not handle the second skb_clone() failure as it
does the first one. At that point the command has already been sent and
HCI_CMD_PENDING is set, so an allocation failure leaves the flag set with
req_skb NULL. The response then matches nothing and the caller blocks for
HCI_INIT_TIMEOUT before getting -ETIMEDOUT, reporting a memory shortage as
a controller timeout.

Clear the flag, cancel the pending request with -ENOMEM so the caller fails
immediately instead of waiting out the timeout, and return -ENOMEM so
hci_cmd_work() does not arm cmd_timer for a request that no longer exists.

Clearing HCI_CMD_PENDING here is safe: all code touching that flag runs
from hdev->workqueue, which is an ordered workqueue, so those paths are
serialised and cannot race. The one exception is the cleanup added by
commit ed516e3f1aa6 ("Bluetooth: hci_sync: Clear HCI_CMD_PENDING when
dropping the last request"), which runs on hci_dev_open_sync() failure and
from hci_dev_close_sync(), and the workqueue is stopped before that runs.

Fixes: 2615fd9a7c25 ("Bluetooth: hci_sync: Fix overwriting request callback")
Signed-off-by: Ibrahim Abdelkader <iabdelka@qti.qualcomm.com>
Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
---
 net/bluetooth/hci_core.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 8aeb3024428b..9585b42e7970 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -4092,6 +4092,11 @@ static int hci_send_cmd_sync(struct hci_dev *hdev, struct sk_buff *skb)
 	    !hci_dev_test_and_set_flag(hdev, HCI_CMD_PENDING)) {
 		kfree_skb(hdev->req_skb);
 		hdev->req_skb = skb_clone(hdev->sent_cmd, GFP_KERNEL);
+		if (!hdev->req_skb) {
+			hci_dev_clear_flag(hdev, HCI_CMD_PENDING);
+			hci_cmd_sync_cancel_sync(hdev, ENOMEM);
+			err = -ENOMEM;
+		}
 	}
 
 	return err;
-- 
2.55.0

RE: [1/2] Bluetooth: hci_core: Return -ENOMEM when the sent_cmd clone fails

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    PASS      1.49 seconds
VerifyFixes                   PASS      0.59 seconds
VerifySignedoff               PASS      1.01 seconds
GitLint                       PASS      0.70 seconds
SubjectPrefix                 PASS      0.26 seconds
BuildKernel                   PASS      26.35 seconds
CheckAllWarning               PASS      28.52 seconds
CheckSparse                   PASS      27.81 seconds
BuildKernel32                 PASS      25.42 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      458.20 seconds
TestRunner_l2cap-tester       PASS      63.59 seconds
TestRunner_iso-tester         PASS      84.05 seconds
TestRunner_bnep-tester        PASS      19.50 seconds
TestRunner_mgmt-tester        FAIL      222.35 seconds
TestRunner_rfcomm-tester      PASS      25.44 seconds
TestRunner_sco-tester         PASS      31.39 seconds
TestRunner_ioctl-tester       PASS      26.27 seconds
TestRunner_mesh-tester        FAIL      26.01 seconds
TestRunner_smp-tester         PASS      23.90 seconds
TestRunner_userchan-tester    PASS      19.82 seconds
TestRunner_6lowpan-tester     PASS      23.52 seconds
IncrementalBuild              PASS      26.42 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: 501, Passed: 496 (99.0%), Failed: 1, Not Run: 4

Failed Test Cases
Read Exp Feature - Success                           Failed       0.252 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.431 seconds
Mesh - Send cancel - 2                               Timed out    1.986 seconds


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

---
Regards,
Linux Bluetooth