[PATCH v2 1/2] Bluetooth: hci_core: Add reset_type parameter to hdev->reset() callback

[PATCH v2 1/2] Bluetooth: hci_core: Add reset_type parameter to hdev->reset() callback

[Chandrashekar Devegowda]

Add a u8 reset_type parameter to the hdev->reset() callback to allow
userspace to select the reset level via sysfs. Each driver interprets
the level according to its own capabilities.

The reset_type values are:
  0 - default reset
  1 - deeper reset

Writing any value other than 1 defaults to level 0.

Internal callers (command timeout, suspend/resume, coredump)
default to level 0.

All drivers implementing the reset callback are updated to accept
the new parameter:
  - btusb: btusb_intel_reset, btusb_qca_reset, btusb_rtl_reset
  - hci_qca: qca_reset
  - btmtksdio: btmtksdio_reset
  - btmtk: btmtk_reset_sync
  - btnxpuart: nxp_reset
  - btintel_pcie: btintel_pcie_reset

Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
---
v2:
  - Use generic reset level terminology instead of Intel-specific
    FLR/PLDR in sysfs ABI documentation and commit message (Luiz)
  - Fix docutils formatting warning by adding blank lines around
    the indented list in sysfs-class-bluetooth (kernel test robot)
v1:
  - Initial version
---
 Documentation/ABI/stable/sysfs-class-bluetooth |  9 ++++++++-
 drivers/bluetooth/btintel_pcie.c               | 17 +++++++++++------
 drivers/bluetooth/btmtk.c                      |  6 +++---
 drivers/bluetooth/btmtk.h                      |  4 ++--
 drivers/bluetooth/btmtksdio.c                  |  2 +-
 drivers/bluetooth/btnxpuart.c                  |  2 +-
 drivers/bluetooth/btusb.c                      |  6 +++---
 drivers/bluetooth/hci_qca.c                    |  2 +-
 include/net/bluetooth/hci_core.h               |  2 +-
 net/bluetooth/hci_core.c                       |  2 +-
 net/bluetooth/hci_sysfs.c                      | 10 ++++++++--
 11 files changed, 40 insertions(+), 22 deletions(-)

diff --git a/Documentation/ABI/stable/sysfs-class-bluetooth b/Documentation/ABI/stable/sysfs-class-bluetooth
index 36be02471174..fb445e20f972 100644
--- a/Documentation/ABI/stable/sysfs-class-bluetooth
+++ b/Documentation/ABI/stable/sysfs-class-bluetooth
@@ -3,7 +3,14 @@ Date:		14-Jan-2025
 KernelVersion:	6.13
 Contact:	linux-bluetooth@vger.kernel.org
 Description: 	This write-only attribute allows users to trigger the vendor reset
-		method on the Bluetooth device when arbitrary data is written.
+		method on the Bluetooth device. The value written selects the
+		reset level. Each driver interprets the level according to its
+		own capabilities:
+
+		  - 0: default reset
+		  - 1: deeper reset
+
+		Writing any value other than 1 defaults to level 0.
 		The reset may or may not be done through the device transport
 		(e.g., UART/USB), and can also be done through an out-of-band
 		approach such as GPIO.
diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index 9e39327dc1fe..d3a03cf96421 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -2486,7 +2486,7 @@ static void btintel_pcie_inc_recovery_count(struct pci_dev *pdev,
 }
 
 static int btintel_pcie_setup_hdev(struct btintel_pcie_data *data);
-static void btintel_pcie_reset(struct hci_dev *hdev);
+static void btintel_pcie_reset(struct hci_dev *hdev, u8 reset_type);
 
 static int btintel_pcie_acpi_reset_method(struct btintel_pcie_data *data)
 {
@@ -2680,7 +2680,7 @@ static void btintel_pcie_reset_work(struct work_struct *wk)
 	pci_unlock_rescan_remove();
 }
 
-static void btintel_pcie_reset(struct hci_dev *hdev)
+static void btintel_pcie_reset(struct hci_dev *hdev, u8 reset_type)
 {
 	struct btintel_pcie_data *data;
 
@@ -2692,6 +2692,12 @@ static void btintel_pcie_reset(struct hci_dev *hdev)
 	if (test_and_set_bit(BTINTEL_PCIE_RECOVERY_IN_PROGRESS, &data->flags))
 		return;
 
+	data->reset_type = (reset_type == 1) ?
+			    BTINTEL_PCIE_IOSF_PRR_PLDR : BTINTEL_PCIE_IOSF_PRR_FLR;
+
+	bt_dev_info(hdev, "Reset triggered: %s",
+		    data->reset_type == BTINTEL_PCIE_IOSF_PRR_PLDR ? "PLDR" : "FLR");
+
 	pci_dev_get(data->pdev);
 	schedule_work(&data->reset_work);
 }
@@ -2729,7 +2735,7 @@ static void btintel_pcie_hw_error(struct hci_dev *hdev, u8 code)
 		return;
 	}
 	btintel_pcie_inc_recovery_count(pdev, &hdev->dev);
-	btintel_pcie_reset(hdev);
+	btintel_pcie_reset(hdev, (code == 0x13) ? 1 : 0);
 }
 
 static bool btintel_pcie_wakeup(struct hci_dev *hdev)
@@ -3111,8 +3117,7 @@ static int btintel_pcie_resume(struct device *dev)
 	if (data->pm_sx_event == PM_EVENT_FREEZE ||
 	    data->pm_sx_event == PM_EVENT_HIBERNATE) {
 		set_bit(BTINTEL_PCIE_CORE_HALTED, &data->flags);
-		data->reset_type = BTINTEL_PCIE_IOSF_PRR_FLR;
-		btintel_pcie_reset(data->hdev);
+		btintel_pcie_reset(data->hdev, 0);
 		return 0;
 	}
 
@@ -3143,7 +3148,7 @@ static int btintel_pcie_resume(struct device *dev)
 			queue_work(data->coredump_workqueue, &data->coredump_work);
 		}
 		set_bit(BTINTEL_PCIE_CORE_HALTED, &data->flags);
-		btintel_pcie_reset(data->hdev);
+		btintel_pcie_reset(data->hdev, 0);
 	}
 	return err;
 }
diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
index 02a96342e964..641f62912f63 100644
--- a/drivers/bluetooth/btmtk.c
+++ b/drivers/bluetooth/btmtk.c
@@ -104,7 +104,7 @@ static void btmtk_coredump_notify(struct hci_dev *hdev, int state)
 	case HCI_DEVCOREDUMP_ABORT:
 	case HCI_DEVCOREDUMP_DONE:
 		data->cd_info.state = HCI_DEVCOREDUMP_IDLE;
-		btmtk_reset_sync(hdev);
+		btmtk_reset_sync(hdev, 0);
 		break;
 	}
 }
@@ -384,7 +384,7 @@ int btmtk_set_bdaddr(struct hci_dev *hdev, const bdaddr_t *bdaddr)
 }
 EXPORT_SYMBOL_GPL(btmtk_set_bdaddr);
 
-void btmtk_reset_sync(struct hci_dev *hdev)
+void btmtk_reset_sync(struct hci_dev *hdev, u8 reset_type)
 {
 	struct btmtk_data *reset_work = hci_get_priv(hdev);
 	int err;
@@ -1403,7 +1403,7 @@ int btmtk_usb_setup(struct hci_dev *hdev)
 		if (err < 0) {
 			/* retry once if setup firmware error */
 			if (!test_and_set_bit(BTMTK_FIRMWARE_DL_RETRY, &btmtk_data->flags))
-				btmtk_reset_sync(hdev);
+				btmtk_reset_sync(hdev, 0);
 			bt_dev_err(hdev, "Failed to set up firmware (%d)", err);
 			return err;
 		}
diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
index c83c24897c95..5cda42444e94 100644
--- a/drivers/bluetooth/btmtk.h
+++ b/drivers/bluetooth/btmtk.h
@@ -196,7 +196,7 @@ int btmtk_setup_firmware_79xx(struct hci_dev *hdev, const char *fwname,
 int btmtk_setup_firmware(struct hci_dev *hdev, const char *fwname,
 			 wmt_cmd_sync_func_t wmt_cmd_sync);
 
-void btmtk_reset_sync(struct hci_dev *hdev);
+void btmtk_reset_sync(struct hci_dev *hdev, u8 reset_type);
 
 int btmtk_register_coredump(struct hci_dev *hdev, const char *name,
 			    u32 fw_version);
@@ -244,7 +244,7 @@ static inline int btmtk_setup_firmware(struct hci_dev *hdev, const char *fwname,
 	return -EOPNOTSUPP;
 }
 
-static inline void btmtk_reset_sync(struct hci_dev *hdev)
+static inline void btmtk_reset_sync(struct hci_dev *hdev, u8 reset_type)
 {
 }
 
diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c
index c6f80c419e90..3c4401ce1e00 100644
--- a/drivers/bluetooth/btmtksdio.c
+++ b/drivers/bluetooth/btmtksdio.c
@@ -1269,7 +1269,7 @@ static int btmtksdio_send_frame(struct hci_dev *hdev, struct sk_buff *skb)
 	return 0;
 }
 
-static void btmtksdio_reset(struct hci_dev *hdev)
+static void btmtksdio_reset(struct hci_dev *hdev, u8 reset_type)
 {
 	struct btmtksdio_dev *bdev = hci_get_drvdata(hdev);
 	u32 status;
diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index e7036a48ce48..e2416b48116c 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -1539,7 +1539,7 @@ static bool nxp_wakeup(struct hci_dev *hdev)
 	return false;
 }
 
-static void nxp_reset(struct hci_dev *hdev)
+static void nxp_reset(struct hci_dev *hdev, u8 reset_type)
 {
 	struct btnxpuart_dev *nxpdev = hci_get_drvdata(hdev);
 
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 7f14ce96319b..ffe109b3b587 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -1045,7 +1045,7 @@ static void btusb_reset(struct hci_dev *hdev)
 	usb_queue_reset_device(data->intf);
 }
 
-static void btusb_intel_reset(struct hci_dev *hdev)
+static void btusb_intel_reset(struct hci_dev *hdev, u8 reset_type)
 {
 	struct btusb_data *data = hci_get_drvdata(hdev);
 	struct gpio_desc *reset_gpio = data->reset_gpio;
@@ -1123,7 +1123,7 @@ static inline void btusb_rtl_alloc_devcoredump(struct hci_dev *hdev,
 	}
 }
 
-static void btusb_rtl_reset(struct hci_dev *hdev)
+static void btusb_rtl_reset(struct hci_dev *hdev, u8 reset_type)
 {
 	struct btusb_data *data = hci_get_drvdata(hdev);
 	struct gpio_desc *reset_gpio = data->reset_gpio;
@@ -1167,7 +1167,7 @@ static void btusb_rtl_hw_error(struct hci_dev *hdev, u8 code)
 	btusb_rtl_alloc_devcoredump(hdev, &hdr, NULL, 0);
 }
 
-static void btusb_qca_reset(struct hci_dev *hdev)
+static void btusb_qca_reset(struct hci_dev *hdev, u8 reset_type)
 {
 	struct btusb_data *data = hci_get_drvdata(hdev);
 	struct gpio_desc *reset_gpio = data->reset_gpio;
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 244447195619..02b4afe77669 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1693,7 +1693,7 @@ static void qca_hw_error(struct hci_dev *hdev, u8 code)
 	clear_bit(QCA_HW_ERROR_EVENT, &qca->flags);
 }
 
-static void qca_reset(struct hci_dev *hdev)
+static void qca_reset(struct hci_dev *hdev, u8 reset_type)
 {
 	struct hci_uart *hu = hci_get_drvdata(hdev);
 	struct qca_data *qca = hu->priv;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 7e15da47fe3a..00421352fcb5 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -650,7 +650,7 @@ struct hci_dev {
 	int (*post_init)(struct hci_dev *hdev);
 	int (*set_diag)(struct hci_dev *hdev, bool enable);
 	int (*set_bdaddr)(struct hci_dev *hdev, const bdaddr_t *bdaddr);
-	void (*reset)(struct hci_dev *hdev);
+	void (*reset)(struct hci_dev *hdev, u8 reset_type);
 	bool (*wakeup)(struct hci_dev *hdev);
 	int (*set_quality_report)(struct hci_dev *hdev, bool enable);
 	int (*get_data_path_id)(struct hci_dev *hdev, __u8 *data_path);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 5ba9fe8261ec..360b329ae6da 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1435,7 +1435,7 @@ static void hci_cmd_timeout(struct work_struct *work)
 	}
 
 	if (hdev->reset)
-		hdev->reset(hdev);
+		hdev->reset(hdev, 0);
 
 	atomic_set(&hdev->cmd_cnt, 1);
 	queue_work(hdev->workqueue, &hdev->cmd_work);
diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
index 8957ce7c21b7..a4fe329158cf 100644
--- a/net/bluetooth/hci_sysfs.c
+++ b/net/bluetooth/hci_sysfs.c
@@ -97,8 +97,14 @@ static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
 {
 	struct hci_dev *hdev = to_hci_dev(dev);
 
-	if (hdev->reset)
-		hdev->reset(hdev);
+	if (hdev->reset) {
+		int val;
+
+		if (kstrtoint(buf, 10, &val) || val != 1)
+			hdev->reset(hdev, 0);
+		else
+			hdev->reset(hdev, 1);
+	}
 
 	return count;
 }
-- 
2.43.0

[PATCH v2 2/2] Bluetooth: btintel_pcie: Fix TOCTOU race in reset path

[Chandrashekar Devegowda]

Move the test_and_set_bit(BTINTEL_PCIE_RECOVERY_IN_PROGRESS) check
before the SETUP_DONE check to fix a Time-Of-Check to Time-Of-Use
race. Previously, multiple callers could pass the SETUP_DONE check
concurrently and then race on the RECOVERY_IN_PROGRESS flag,
potentially scheduling conflicting removal work.

By reordering the existing atomic guard to execute first, concurrent
reset requests are atomically rejected before any state is inspected.
The RECOVERY_IN_PROGRESS flag is cleared on the early-exit path
(setup not done) to allow future reset attempts.

Signed-off-by: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>
---
v2:
  - No changes
v1:
  - Initial version
---
 drivers/bluetooth/btintel_pcie.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
index d3a03cf96421..f4f400421833 100644
--- a/drivers/bluetooth/btintel_pcie.c
+++ b/drivers/bluetooth/btintel_pcie.c
@@ -2686,11 +2686,15 @@ static void btintel_pcie_reset(struct hci_dev *hdev, u8 reset_type)
 
 	data = hci_get_drvdata(hdev);
 
-	if (!test_bit(BTINTEL_PCIE_SETUP_DONE, &data->flags))
+	if (test_and_set_bit(BTINTEL_PCIE_RECOVERY_IN_PROGRESS, &data->flags)) {
+		bt_dev_warn(hdev, "Reset rejected: recovery already in progress");
 		return;
+	}
 
-	if (test_and_set_bit(BTINTEL_PCIE_RECOVERY_IN_PROGRESS, &data->flags))
+	if (!test_bit(BTINTEL_PCIE_SETUP_DONE, &data->flags)) {
+		clear_bit(BTINTEL_PCIE_RECOVERY_IN_PROGRESS, &data->flags);
 		return;
+	}
 
 	data->reset_type = (reset_type == 1) ?
 			    BTINTEL_PCIE_IOSF_PRR_PLDR : BTINTEL_PCIE_IOSF_PRR_FLR;
-- 
2.43.0

Re: [PATCH v2 1/2] Bluetooth: hci_core: Add reset_type parameter to hdev->reset() callback

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] UAPI Breakage: Redefining '1' to mean 'deeper reset' breaks existing userspace scripts.

Pre-existing issues:
- [High] Data Race: Unguarded write to `dev_data->reset_type` in `btintel_pcie_hw_error()` clobbers concurrent sysfs resets.
- [High] TOCTOU Race Condition: Concurrent driver unbind causes NULL pointer dereference in `reset_store()`.
--

commit 4b14894786f1236a0de480b4c65121356669aba4
Author: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>

Bluetooth: hci_core: Add reset_type parameter to hdev->reset() callback

Add a u8 reset_type parameter to the hdev->reset() callback to allow
userspace to select the reset level via sysfs. Each driver interprets
the level according to its own capabilities.

> diff --git a/Documentation/ABI/stable/sysfs-class-bluetooth b/Documentation/ABI/stable/sysfs-class-bluetooth
> index 36be024711747..fb445e20f9726 100644
> --- a/Documentation/ABI/stable/sysfs-class-bluetooth
> +++ b/Documentation/ABI/stable/sysfs-class-bluetooth
> @@ -3,7 +3,14 @@ Date:		14-Jan-2025
>  KernelVersion:	6.13
>  Contact:	linux-bluetooth@vger.kernel.org
>  Description: 	This write-only attribute allows users to trigger the vendor reset
> -		method on the Bluetooth device when arbitrary data is written.
> +		method on the Bluetooth device. The value written selects the
> +		reset level. Each driver interprets the level according to its
> +		own capabilities:
> +
> +		  - 0: default reset
> +		  - 1: deeper reset
> +
> +		Writing any value other than 1 defaults to level 0.

[Severity: High]
Will redefining 1 to mean a deeper reset break existing userspace scripts? 

Historically, the reset sysfs attribute triggered the default reset when
arbitrary data was written, and in the Linux ecosystem, userspace typically
writes 1 to trigger boolean actions.

If existing scripts use echo 1 > reset, wouldn't they unknowingly trigger a
much more disruptive reset than before? Could we map 1 to the default reset
and use a distinct value (e.g., 2) for the deeper reset to maintain backward
compatibility?

[ ... ]

> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index 9e39327dc1fe2..d3a03cf964216 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
[ ... ]
> @@ -2729,7 +2735,7 @@ static void btintel_pcie_hw_error(struct hci_dev *hdev, u8 code)
>  		return;
>  	}
>  	btintel_pcie_inc_recovery_count(pdev, &hdev->dev);
> -	btintel_pcie_reset(hdev);
> +	btintel_pcie_reset(hdev, (code == 0x13) ? 1 : 0);
>  }

[Severity: High]
This is a pre-existing issue, but does the unguarded write to
dev_data->reset_type earlier in btintel_pcie_hw_error() cause a data race?

While this patch updates btintel_pcie_reset() to safely assign the reset
type under the BTINTEL_PCIE_RECOVERY_IN_PROGRESS bitlock,
btintel_pcie_hw_error() still directly modifies dev_data->reset_type
without holding any structural locks or exclusion bits.

If a sysfs reset (FLR) is concurrently triggered and acquires the bitlock,
could this unguarded write in btintel_pcie_hw_error() silently overwrite the
pending reset type to PLDR, causing the asynchronously scheduled reset work
to read the corrupted type and perform the wrong level of reset?

[ ... ]

> diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c
> index 8957ce7c21b76..a4fe329158cf7 100644
> --- a/net/bluetooth/hci_sysfs.c
> +++ b/net/bluetooth/hci_sysfs.c
> @@ -97,8 +97,14 @@ static ssize_t reset_store(struct device *dev, struct device_attribute *attr,
>  {
>  	struct hci_dev *hdev = to_hci_dev(dev);
>  
> -	if (hdev->reset)
> -		hdev->reset(hdev);
> +	if (hdev->reset) {
> +		int val;
> +
> +		if (kstrtoint(buf, 10, &val) || val != 1)
> +			hdev->reset(hdev, 0);
> +		else
> +			hdev->reset(hdev, 1);
> +	}
>  
>  	return count;
>  }

[Severity: High]
This is a pre-existing issue, but does checking hdev->reset and then calling
it after kstrtoint() introduce a TOCTOU race condition?

Because kstrtoint() is an external function call, it acts as a compiler
barrier, forcing the compiler to reload hdev->reset from memory before
invoking it as a function pointer.

If a concurrent teardown path sets hdev->reset = NULL between the check and
the execution, could reset_store() dereference a NULL pointer and crash the
kernel? While a smaller race window existed before, this patch significantly
widens it by inserting the kstrtoint() call. Should the function pointer be
cached in a local variable before the NULL check?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618085016.9173-1-chandrashekar.devegowda@intel.com?part=1

RE: [v2,1/2] Bluetooth: hci_core: Add reset_type parameter to hdev->reset() callback

[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=1113309

---Test result---

Test Summary:
CheckPatch                    PASS      3.18 seconds
VerifyFixes                   PASS      0.07 seconds
VerifySignedoff               PASS      0.07 seconds
GitLint                       PASS      0.40 seconds
SubjectPrefix                 PASS      0.12 seconds
BuildKernel                   PASS      26.91 seconds
CheckAllWarning               PASS      30.25 seconds
CheckSparse                   PASS      28.70 seconds
BuildKernel32                 PASS      26.46 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      581.35 seconds
TestRunner_l2cap-tester       PASS      58.38 seconds
TestRunner_iso-tester         PASS      92.91 seconds
TestRunner_bnep-tester        PASS      18.82 seconds
TestRunner_mgmt-tester        FAIL      208.89 seconds
TestRunner_rfcomm-tester      PASS      25.41 seconds
TestRunner_sco-tester         PASS      33.27 seconds
TestRunner_ioctl-tester       PASS      25.48 seconds
TestRunner_mesh-tester        FAIL      25.95 seconds
TestRunner_smp-tester         PASS      23.08 seconds
TestRunner_userchan-tester    PASS      19.83 seconds
TestRunner_6lowpan-tester     PASS      22.69 seconds
IncrementalBuild              PASS      27.01 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.236 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.705 seconds
Mesh - Send cancel - 2                               Timed out    1.985 seconds


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

---
Regards,
Linux Bluetooth

Re: [PATCH v2 2/2] Bluetooth: btintel_pcie: Fix TOCTOU race in reset path

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Kernel panic due to unconditionally dereferencing a NULL `hdev` pointer during PM resume if a prior reset failed.
- [High] Non-atomic modification of data->flags causes data race with atomic bit operations, risking lost coredumps and permanent lockouts.
- [High] Unprotected assignment to dev_data->reset_type in btintel_pcie_hw_error can corrupt an ongoing reset.
--

commit c9025f5524eb70c6235927f87e97777af4f0b9ab
Author: Chandrashekar Devegowda <chandrashekar.devegowda@intel.com>

Bluetooth: btintel_pcie: Fix TOCTOU race in reset path

This commit fixes a Time-Of-Check to Time-Of-Use race condition in the
reset path of the btintel_pcie driver. It reorders the RECOVERY_IN_PROGRESS
atomic guard to execute before checking the SETUP_DONE flag, preventing
concurrent reset requests from racing.

> diff --git a/drivers/bluetooth/btintel_pcie.c b/drivers/bluetooth/btintel_pcie.c
> index d3a03cf964216..f4f400421833b 100644
> --- a/drivers/bluetooth/btintel_pcie.c
> +++ b/drivers/bluetooth/btintel_pcie.c
> @@ -2686,11 +2686,15 @@ static void btintel_pcie_reset(struct hci_dev *hdev, u8 reset_type)
>  
>  	data = hci_get_drvdata(hdev);

[Severity: Critical]
This is a pre-existing issue, but can this dereference cause a kernel panic
during PM resume if a prior reset failed?

If a reset fails (e.g., in btintel_pcie_setup_hdev()), the device is
unregistered and data->hdev is left permanently set to NULL. If the system
goes into a suspend/hibernate cycle, the resume handler unconditionally
calls this reset function:

btintel_pcie_resume()
    set_bit(BTINTEL_PCIE_CORE_HALTED, &data->flags);
    btintel_pcie_reset(data->hdev, 0);

When btintel_pcie_reset() runs, it calls hci_get_drvdata(hdev), which blindly
dereferences hdev. Should there be a NULL pointer check for hdev here?

> -	if (!test_bit(BTINTEL_PCIE_SETUP_DONE, &data->flags))
> +	if (test_and_set_bit(BTINTEL_PCIE_RECOVERY_IN_PROGRESS, &data->flags)) {
> +		bt_dev_warn(hdev, "Reset rejected: recovery already in progress");
>  		return;
> +	}

[Severity: High]
This isn't a bug introduced by this patch, but since we are modifying atomic
flags operations, I noticed a potential data race with data->flags in the
worker thread:

btintel_pcie_reset_work()
    btintel_pcie_start_rx(data);
    data->flags = 0;
    err = btintel_pcie_setup_hdev(data);

Because data->flags is cleared non-atomically immediately after interrupts are
re-enabled, could this erase an atomic modification from an incoming MSI-X
interrupt, such as test_and_set_bit(BTINTEL_PCIE_HWEXP_INPROGRESS, &data->flags)?

Would it be safer to clear the specific bits using atomic operations instead?

> -	if (test_and_set_bit(BTINTEL_PCIE_RECOVERY_IN_PROGRESS, &data->flags))
> +	if (!test_bit(BTINTEL_PCIE_SETUP_DONE, &data->flags)) {
> +		clear_bit(BTINTEL_PCIE_RECOVERY_IN_PROGRESS, &data->flags);
>  		return;
> +	}
>  
>  	data->reset_type = (reset_type == 1) ?
>  			    BTINTEL_PCIE_IOSF_PRR_PLDR : BTINTEL_PCIE_IOSF_PRR_FLR;

[Severity: High]
This is a pre-existing issue, but I have a question about how the shared
reset_type state is handled concurrently.

In the hardware error handler, dev_data->reset_type is overwritten outside
of any lock before the atomic guard in btintel_pcie_reset() is reached:

btintel_pcie_hw_error()
    if (code == 0x13)
        dev_data->reset_type = BTINTEL_PCIE_IOSF_PRR_PLDR;
    else
        dev_data->reset_type = BTINTEL_PCIE_IOSF_PRR_FLR;
        
    btintel_pcie_reset(hdev, ...);

If btintel_pcie_reset_work() is actively executing a recovery and reads
data->reset_type locklessly, could this unprotected assignment cause the
worker thread to observe an inconsistent state and corrupt the recovery
sequence?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260618085016.9173-1-chandrashekar.devegowda@intel.com?part=2