* [PATCH 0/2] Indicate PAST RX ready to userspace
@ 2026-10-11 20:39 Pauli Virtanen
2026-10-11 20:39 ` [PATCH 1/2] Bluetooth: hci_sync: add request wait hook for __hci_cmd_sync_sk Pauli Virtanen
2026-10-11 20:39 ` [PATCH 2/2] Bluetooth: ISO: Signal BIS PAST ready to userspace via POLLOUT Pauli Virtanen
0 siblings, 2 replies; 5+ messages in thread
From: Pauli Virtanen @ 2026-10-11 20:39 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Pauli Virtanen, marcel, luiz.dentz, linux-kernel
Currently there is the following race in BASS PAST:
1. Assistant: TX Add Source
2. Delegator user: RX Add Source, listen(), TX Bcast Rcv State Nfy
3a. Assistant: RX Bcast Rcv State Nfy, Send PAST
3b. Delegator kernel (hci_sync)
- HCI_OP_LE_PAST_PARAMS queued -> Command Complete
- start waiting for HCI_EV_LE_PAST_RECEIVED
Steps 3a and 3b are not ordered, so LE PAST may arrive when kernel or
controller is not ready to receive it.
This is why test_bass_past_transport_acquire functional test is flaky.
Solve this by new userspace API: listen() socket signals POLLOUT when
kernel is ready for PAST.
Userspace shall notify PAST ready only after that, fixed in separate
BlueZ patch series. Also add related iso-tester tests + uAPI
documentation there.
This needed an extension to the HCI_REQ handling, to set POLLOUT only
after hci_sync has started waiting for the PA events. Didn't see easy
ways to simplify this, without completely changing the
hci_connect_pa_sync() -> create_pa_complete() flow.
***
An alternative uAPI here could be to have listen() block until PAST
listener is ready. The time it blocks is however not well bounded if
hci_sync queue is stuck doing something else first. We probably want to
avoid stalls in bluetoothd, so went with async notification here.
***
Testing: test_bass_past_transport_acquire: 24/24 passed at -n 6
(pre-patch baseline: 6/24 failed). In --btvirt-debug delegator 0x205c
(LE Set PAST Parameters) is before assistant 0x205b (transfer) in every
run. Pre-patch failures show the inverse (0x205b first, or in same
btvirt instant).
Not tested on real hardware, only on the emulator.
LLM was used for triage + patch review + test writing. Design and
implementation is mine.
Assisted-by: opencode:qwen3.8-flash-next opencode:gpt-6-sol
Pauli Virtanen (2):
Bluetooth: hci_sync: add request wait hook for __hci_cmd_sync_sk
Bluetooth: ISO: Signal BIS PAST ready to userspace via POLLOUT
include/net/bluetooth/bluetooth.h | 2 +
include/net/bluetooth/hci_sync.h | 10 ++-
net/bluetooth/hci_core.c | 9 ++-
net/bluetooth/hci_sync.c | 109 ++++++++++++++++++++++++------
net/bluetooth/iso.c | 45 +++++++++++-
5 files changed, 150 insertions(+), 25 deletions(-)
--
2.56.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH 1/2] Bluetooth: hci_sync: add request wait hook for __hci_cmd_sync_sk
2026-10-11 20:39 [PATCH 0/2] Indicate PAST RX ready to userspace Pauli Virtanen
@ 2026-10-11 20:39 ` Pauli Virtanen
2026-10-11 22:04 ` Pauli Virtanen
2026-10-11 22:51 ` Indicate PAST RX ready to userspace bluez.test.bot
2026-10-11 20:39 ` [PATCH 2/2] Bluetooth: ISO: Signal BIS PAST ready to userspace via POLLOUT Pauli Virtanen
1 sibling, 2 replies; 5+ messages in thread
From: Pauli Virtanen @ 2026-10-11 20:39 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Pauli Virtanen, marcel, luiz.dentz, linux-kernel
Support doing something after hci_sync request event wait is armed, but
before wait starts. Currently hci_req_sync_run() queues cmd_work, which
sets hdev->req_skb and arms the wait, but hci_sync does not see when
this occurs.
This is needed by BIS PAST (separate patch) to signal userspace when
kernel starts waiting for PAST Received.
Introduce new pending hdev->req_status state HCI_REQ_QUEUE. cmd_work
transitions QUEUE -> PEND after setting hdev->req_skb which is where
event wait starts.
Fire the wait hook when __hci_cmd_sync_sk() has started waiting for its
event, and if that fails. If no wait hook, start directly with PEND, to
avoid the extra req_wait_q wakeup.
Rename the function to __hci_cmd_sync_sk_run(), add small wrappers to
preserve old API.
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
include/net/bluetooth/hci_sync.h | 10 +++-
net/bluetooth/hci_core.c | 9 +++-
net/bluetooth/hci_sync.c | 87 +++++++++++++++++++++++++-------
3 files changed, 86 insertions(+), 20 deletions(-)
diff --git a/include/net/bluetooth/hci_sync.h b/include/net/bluetooth/hci_sync.h
index 59cb918d1405..71736690ea7f 100644
--- a/include/net/bluetooth/hci_sync.h
+++ b/include/net/bluetooth/hci_sync.h
@@ -9,8 +9,11 @@
#define PTR_UINT(_ptr) ((uintptr_t)((void *)_ptr))
#define HCI_REQ_DONE 0
-#define HCI_REQ_PEND 1
-#define HCI_REQ_CANCELED 2
+#define HCI_REQ_QUEUE BIT(0)
+#define HCI_REQ_PEND (HCI_REQ_QUEUE | BIT(1))
+#define HCI_REQ_CANCELED BIT(2)
+
+#define HCI_REQ_PENDING(req_status) (!!((req_status) & HCI_REQ_PEND))
#define hci_req_sync_lock(hdev) mutex_lock(&hdev->req_lock)
#define hci_req_sync_unlock(hdev) mutex_unlock(&hdev->req_lock)
@@ -25,6 +28,9 @@ struct hci_request {
int err;
};
+typedef void (*hci_request_wait_start_t)(struct hci_dev *hdev, void *data,
+ int err);
+
typedef int (*hci_cmd_sync_work_func_t)(struct hci_dev *hdev, void *data);
typedef void (*hci_cmd_sync_work_destroy_t)(struct hci_dev *hdev, void *data,
int err);
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 209244fd9391..b9214eaffd2a 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -4183,11 +4183,18 @@ static int hci_send_cmd_sync(struct hci_dev *hdev, struct sk_buff *skb)
kfree_skb(skb);
}
- if (READ_ONCE(hdev->req_status) == HCI_REQ_PEND &&
+ if (HCI_REQ_PENDING(READ_ONCE(hdev->req_status)) &&
!hci_dev_test_and_set_flag(hdev, HCI_CMD_PENDING)) {
hci_dev_lock(hdev);
+
kfree_skb(hdev->req_skb);
hdev->req_skb = skb_get(hdev->sent_cmd);
+
+ if ((bt_cb(hdev->req_skb)->hci.req_flags & HCI_REQ_SKB) &&
+ cmpxchg(&hdev->req_status, HCI_REQ_QUEUE,
+ HCI_REQ_PEND) == HCI_REQ_QUEUE)
+ wake_up_interruptible(&hdev->req_wait_q);
+
hci_dev_unlock(hdev);
}
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index efbd2e5387c0..058daf1f16df 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -25,7 +25,7 @@ static void hci_cmd_sync_complete(struct hci_dev *hdev, u8 result, u16 opcode,
{
bt_dev_dbg(hdev, "result 0x%2.2x", result);
- if (READ_ONCE(hdev->req_status) != HCI_REQ_PEND)
+ if (!HCI_REQ_PENDING(READ_ONCE(hdev->req_status)))
return;
hdev->req_result = result;
@@ -155,13 +155,16 @@ static void hci_request_init(struct hci_request *req, struct hci_dev *hdev)
}
/* This function requires the caller holds hdev->req_lock. */
-struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
- const void *param, u8 event, u32 timeout,
- struct sock *sk)
+static struct sk_buff *__hci_cmd_sync_sk_run(struct hci_dev *hdev, u16 opcode,
+ u32 plen, const void *param,
+ u8 event, u32 timeout,
+ struct sock *sk,
+ hci_request_wait_start_t wait_func,
+ void *wait_data)
{
struct hci_request req;
struct sk_buff *skb;
- int err = 0;
+ long err = 0;
bt_dev_dbg(hdev, "Opcode 0x%4.4x", opcode);
@@ -169,29 +172,58 @@ struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
hci_cmd_sync_add(&req, opcode, plen, param, event, sk);
- WRITE_ONCE(hdev->req_status, HCI_REQ_PEND);
+ WRITE_ONCE(hdev->req_status, wait_func ? HCI_REQ_QUEUE : HCI_REQ_PEND);
err = hci_req_sync_run(&req);
- if (err < 0)
+ if (err < 0) {
+ if (wait_func)
+ wait_func(hdev, wait_data, err);
return ERR_PTR(err);
+ }
+
+ if (wait_func) {
+ /* Sleep until we have started waiting for the event */
+ err = wait_event_interruptible_timeout(hdev->req_wait_q,
+ READ_ONCE(hdev->req_status) != HCI_REQ_QUEUE,
+ timeout);
+ if (err <= 0 || !HCI_REQ_PENDING(READ_ONCE(hdev->req_status)))
+ goto check_state;
+
+ wait_func(hdev, wait_data, 0);
+ wait_func = NULL;
+
+ timeout = err;
+ }
err = wait_event_interruptible_timeout(hdev->req_wait_q,
- READ_ONCE(hdev->req_status) != HCI_REQ_PEND,
+ !HCI_REQ_PENDING(READ_ONCE(hdev->req_status)),
timeout);
- if (err == -ERESTARTSYS)
+check_state:
+ if (err == -ERESTARTSYS) {
+ if (wait_func)
+ wait_func(hdev, wait_data, -EINTR);
return ERR_PTR(-EINTR);
+ }
switch (READ_ONCE(hdev->req_status)) {
case HCI_REQ_DONE:
+ if (wait_func)
+ wait_func(hdev, wait_data, -EAGAIN);
err = -bt_to_errno(hdev->req_result);
break;
case HCI_REQ_CANCELED:
- err = -hdev->req_result;
+ if (wait_func)
+ wait_func(hdev, wait_data, -ECANCELED);
+ err = -(long)hdev->req_result;
+ if (!IS_ERR_VALUE(err))
+ err = -ECANCELED;
break;
default:
+ if (wait_func)
+ wait_func(hdev, wait_data, -ETIMEDOUT);
err = -ETIMEDOUT;
break;
}
@@ -201,7 +233,7 @@ struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
skb = hdev->req_rsp;
hdev->req_rsp = NULL;
- bt_dev_dbg(hdev, "end: err %d", err);
+ bt_dev_dbg(hdev, "end: err %ld", err);
if (err < 0) {
kfree_skb(skb);
@@ -216,6 +248,15 @@ struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
return skb;
}
+
+/* This function requires the caller holds hdev->req_lock. */
+struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
+ const void *param, u8 event, u32 timeout,
+ struct sock *sk)
+{
+ return __hci_cmd_sync_sk_run(hdev, opcode, plen, param, event, timeout,
+ sk, NULL, NULL);
+}
EXPORT_SYMBOL(__hci_cmd_sync_sk);
/* This function requires the caller holds hdev->req_lock. */
@@ -255,14 +296,17 @@ struct sk_buff *__hci_cmd_sync_ev(struct hci_dev *hdev, u16 opcode, u32 plen,
EXPORT_SYMBOL(__hci_cmd_sync_ev);
/* This function requires the caller holds hdev->req_lock. */
-int __hci_cmd_sync_status_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
- const void *param, u8 event, u32 timeout,
- struct sock *sk)
+static int __hci_cmd_sync_status_sk_run(struct hci_dev *hdev, u16 opcode,
+ u32 plen, const void *param, u8 event,
+ u32 timeout, struct sock *sk,
+ hci_request_wait_start_t wait_func,
+ void *wait_data)
{
struct sk_buff *skb;
u8 status;
- skb = __hci_cmd_sync_sk(hdev, opcode, plen, param, event, timeout, sk);
+ skb = __hci_cmd_sync_sk_run(hdev, opcode, plen, param, event, timeout,
+ sk, wait_func, wait_data);
/* If command return a status event, skb will be set to -ENODATA */
if (skb == ERR_PTR(-ENODATA))
@@ -281,6 +325,15 @@ int __hci_cmd_sync_status_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
return status;
}
+
+/* This function requires the caller holds hdev->req_lock. */
+int __hci_cmd_sync_status_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
+ const void *param, u8 event, u32 timeout,
+ struct sock *sk)
+{
+ return __hci_cmd_sync_status_sk_run(hdev, opcode, plen, param, event,
+ timeout, sk, NULL, NULL);
+}
EXPORT_SYMBOL(__hci_cmd_sync_status_sk);
int __hci_cmd_sync_status(struct hci_dev *hdev, u16 opcode, u32 plen,
@@ -679,7 +732,7 @@ void hci_cmd_sync_cancel(struct hci_dev *hdev, int err)
{
bt_dev_dbg(hdev, "err 0x%2.2x", err);
- if (READ_ONCE(hdev->req_status) == HCI_REQ_PEND) {
+ if (HCI_REQ_PENDING(READ_ONCE(hdev->req_status))) {
hdev->req_result = err;
WRITE_ONCE(hdev->req_status, HCI_REQ_CANCELED);
@@ -697,7 +750,7 @@ void hci_cmd_sync_cancel_sync(struct hci_dev *hdev, int err)
{
bt_dev_dbg(hdev, "err 0x%2.2x", err);
- if (READ_ONCE(hdev->req_status) == HCI_REQ_PEND) {
+ if (HCI_REQ_PENDING(READ_ONCE(hdev->req_status))) {
/* req_result is __u32 so error must be positive to be properly
* propagated.
*/
--
2.56.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] Bluetooth: ISO: Signal BIS PAST ready to userspace via POLLOUT
2026-10-11 20:39 [PATCH 0/2] Indicate PAST RX ready to userspace Pauli Virtanen
2026-10-11 20:39 ` [PATCH 1/2] Bluetooth: hci_sync: add request wait hook for __hci_cmd_sync_sk Pauli Virtanen
@ 2026-10-11 20:39 ` Pauli Virtanen
1 sibling, 0 replies; 5+ messages in thread
From: Pauli Virtanen @ 2026-10-11 20:39 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Pauli Virtanen, marcel, luiz.dentz, linux-kernel
Userspace needs to know when kernel is ready to handle LE PAST Received.
E.g. Scan Delegator should indicate PAST ready only after that (cf. BAP
v1.0.2 Sec 6.5.6).
Currently applications call listen(sk) to setup it, but kernel hci_sync
may be ready only some time after syscall return.
Add POLLOUT as notify for kernel ready for PAST after BIS socket
listen(). POLLOUT is not signaled for errors. Add BT_ISO_PAST_POLLOUT
socket option to indicate feature support.
POLLOUT was never set for Bluetooth listen sockets before, so this is
previously unused API. POSIX does not appear to specify behavior. This
only changes ISO BIS socket behavior. The API is vaguely similar to the
POLLOUT indicator for connect().
Assisted-by: opencode:qwen3.8-flash-next
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
include/net/bluetooth/bluetooth.h | 2 ++
net/bluetooth/hci_sync.c | 22 ++++++++++++---
net/bluetooth/iso.c | 45 +++++++++++++++++++++++++++++--
3 files changed, 64 insertions(+), 5 deletions(-)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index cd9189244566..d78e34c0e5ef 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -256,6 +256,8 @@ struct bt_codecs {
#define BT_SCM_PKT_SEQNUM 0x05
+#define BT_ISO_PAST_POLLOUT 23
+
__printf(1, 2)
void bt_info(const char *fmt, ...);
__printf(1, 2)
diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 058daf1f16df..ccf71e242677 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -7680,6 +7680,21 @@ static void create_pa_complete(struct hci_dev *hdev, void *data, int err)
hci_conn_put(conn);
}
+static void hci_le_past_wait_ready(struct hci_dev *hdev, void *data, int err)
+{
+ struct hci_conn *conn = data;
+
+ if (err)
+ return;
+
+ /* Inform socket layer we are waiting for LE PAST Received */
+ hci_dev_lock(hdev);
+ if (hci_conn_valid(hdev, conn) &&
+ test_bit(HCI_CONN_CREATE_PA_SYNC, &conn->flags))
+ hci_connect_cfm(conn, 0);
+ hci_dev_unlock(hdev);
+}
+
static int hci_le_past_params_sync(struct hci_dev *hdev, struct hci_conn *conn,
u16 acl_handle, struct bt_iso_qos *qos)
{
@@ -7706,9 +7721,10 @@ static int hci_le_past_params_sync(struct hci_dev *hdev, struct hci_conn *conn,
return err;
/* Wait for HCI_EV_LE_PAST_RECEIVED event */
- return __hci_cmd_sync_status_sk(hdev, HCI_OP_NOP, 0, NULL,
- HCI_EV_LE_PAST_RECEIVED,
- conn->conn_timeout, NULL);
+ return __hci_cmd_sync_status_sk_run(hdev, HCI_OP_NOP, 0, NULL,
+ HCI_EV_LE_PAST_RECEIVED,
+ conn->conn_timeout, NULL,
+ hci_le_past_wait_ready, conn);
}
static int hci_le_pa_create_sync(struct hci_dev *hdev, void *data)
diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index c1fdd9846eeb..6afbeb699386 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -62,6 +62,7 @@ enum {
BT_SK_PA_SYNC,
BT_SK_KILLED,
BT_SK_CONNECTING,
+ BT_SK_PAST_READY,
};
struct iso_pinfo {
@@ -2035,6 +2036,10 @@ static int iso_sock_setsockopt(struct socket *sock, int level, int optname,
break;
+ case BT_ISO_PAST_POLLOUT:
+ err = -EINVAL;
+ break;
+
default:
err = -ENOPROTOOPT;
break;
@@ -2106,6 +2111,13 @@ static int iso_sock_getsockopt(struct socket *sock, int level, int optname,
break;
+ case BT_ISO_PAST_POLLOUT:
+ val = 1;
+ if (copy_to_iter(&val, sizeof(val), &opt->iter_out) !=
+ sizeof(val))
+ err = -EFAULT;
+ break;
+
default:
err = -ENOPROTOOPT;
break;
@@ -2278,6 +2290,16 @@ static void iso_conn_ready(struct iso_conn *conn)
}
}
+ /* PA sync listen socket notification for PAST ready */
+ if (conn->hcon->type == PA_LINK &&
+ test_bit(HCI_CONN_CREATE_PA_SYNC, &conn->hcon->flags)) {
+ set_bit(BT_SK_PAST_READY, &iso_pi(sk)->flags);
+ sk->sk_state_change(sk);
+ release_sock(sk);
+ sock_put(sk);
+ return;
+ }
+
iso_sock_ready(sk);
release_sock(sk);
@@ -2289,7 +2311,10 @@ static void iso_conn_ready(struct iso_conn *conn)
hdev = hcon->hdev;
- if (test_bit(HCI_CONN_BIG_SYNC, &hcon->flags)) {
+ if (test_bit(HCI_CONN_CREATE_PA_SYNC, &conn->hcon->flags)) {
+ /* PAST ready, not a connection */
+ return;
+ } else if (test_bit(HCI_CONN_BIG_SYNC, &hcon->flags)) {
/* A BIS slave hcon is notified to the ISO layer
* after the Command Complete for the LE Setup
* ISO Data Path command is received. Get the
@@ -2900,6 +2925,22 @@ DEFINE_SHOW_ATTRIBUTE(iso_debugfs);
static struct dentry *iso_debugfs;
+static __poll_t iso_sock_poll(struct file *file, struct socket *sock,
+ poll_table *wait)
+{
+ struct sock *sk = sock->sk;
+ __poll_t mask = bt_sock_poll(file, sock, wait);
+
+ /* ISO-specific extension: POLLOUT on listen() socket when PAST setup
+ * has been completed.
+ */
+ if (sk->sk_state == BT_LISTEN &&
+ test_bit(BT_SK_PAST_READY, &iso_pi(sk)->flags))
+ mask |= EPOLLOUT | EPOLLWRNORM;
+
+ return mask;
+}
+
static const struct proto_ops iso_sock_ops = {
.family = PF_BLUETOOTH,
.owner = THIS_MODULE,
@@ -2911,7 +2952,7 @@ static const struct proto_ops iso_sock_ops = {
.getname = iso_sock_getname,
.sendmsg = iso_sock_sendmsg,
.recvmsg = iso_sock_recvmsg,
- .poll = bt_sock_poll,
+ .poll = iso_sock_poll,
.ioctl = bt_sock_ioctl,
.mmap = sock_no_mmap,
.socketpair = sock_no_socketpair,
--
2.56.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] Bluetooth: hci_sync: add request wait hook for __hci_cmd_sync_sk
2026-10-11 20:39 ` [PATCH 1/2] Bluetooth: hci_sync: add request wait hook for __hci_cmd_sync_sk Pauli Virtanen
@ 2026-10-11 22:04 ` Pauli Virtanen
2026-10-11 22:51 ` Indicate PAST RX ready to userspace bluez.test.bot
1 sibling, 0 replies; 5+ messages in thread
From: Pauli Virtanen @ 2026-10-11 22:04 UTC (permalink / raw)
To: linux-bluetooth; +Cc: marcel, luiz.dentz, linux-kernel
su, 2026-10-11 kello 23:39 +0300, Pauli Virtanen kirjoitti:
> Support doing something after hci_sync request event wait is armed, but
> before wait starts. Currently hci_req_sync_run() queues cmd_work, which
> sets hdev->req_skb and arms the wait, but hci_sync does not see when
> this occurs.
>
> This is needed by BIS PAST (separate patch) to signal userspace when
> kernel starts waiting for PAST Received.
>
> Introduce new pending hdev->req_status state HCI_REQ_QUEUE. cmd_work
> transitions QUEUE -> PEND after setting hdev->req_skb which is where
> event wait starts.
>
> Fire the wait hook when __hci_cmd_sync_sk() has started waiting for its
> event, and if that fails. If no wait hook, start directly with PEND, to
> avoid the extra req_wait_q wakeup.
>
> Rename the function to __hci_cmd_sync_sk_run(), add small wrappers to
> preserve old API.
On Sashiko review:
wait_func(hdev, wait_data, -EAGAIN); if the waited-for event arrives
before hci_sync sees QUEUE->PEND transition was intentional. Because
for PAST, I think we don't want to signal Syncinfo Request if we know
we already got the PA sync.
AFAICS it is not really issue more generally, as the wait callback can
do what it wants with the -EAGAIN.
***
Sashiko review: """Could the lack of memory barriers around req_status
lead to a data race here where we read stale data or a NULL req_rsp?
In hci_cmd_sync_complete(), the producer writes hdev->req_result =
result and updates req_status to HCI_REQ_DONE using WRITE_ONCE()
without a write memory barrier (like smp_store_release()).
Additionally, the payload hdev->req_rsp is assigned after req_status is
updated to HCI_REQ_DONE. When __hci_cmd_sync_sk_run() evaluates the
wait condition, it reads req_status without an acquire barrier
(smp_load_acquire()). If the CPU reorders the loads, could a concurrent
consumer observe HCI_REQ_DONE but read a NULL req_rsp because the
producer hasn't assigned it yet? If so, it seems the consumer would
erroneously return -ENODATA and leak the actual skb."""
I think the lack of explicit synchronization between
hci_cmd_sync_complete() in hdev->workqueue writing and
__hci_cmd_sync_sk() reading is potentially a real issue, but it is pre-
existing behavior and I don't think this patch touches that.
For memory barriers, Documentation/memory-barriers.txt:2294 seems to
say hci_cmd_sync_complete() is not right. Maybe the hdev->req_* should
have a guarding lock to make reasoning simpler.
> Signed-off-by: Pauli Virtanen <pav@iki.fi>
> ---
> include/net/bluetooth/hci_sync.h | 10 +++-
> net/bluetooth/hci_core.c | 9 +++-
> net/bluetooth/hci_sync.c | 87 +++++++++++++++++++++++++-------
> 3 files changed, 86 insertions(+), 20 deletions(-)
>
> diff --git a/include/net/bluetooth/hci_sync.h b/include/net/bluetooth/hci_sync.h
> index 59cb918d1405..71736690ea7f 100644
> --- a/include/net/bluetooth/hci_sync.h
> +++ b/include/net/bluetooth/hci_sync.h
> @@ -9,8 +9,11 @@
> #define PTR_UINT(_ptr) ((uintptr_t)((void *)_ptr))
>
> #define HCI_REQ_DONE 0
> -#define HCI_REQ_PEND 1
> -#define HCI_REQ_CANCELED 2
> +#define HCI_REQ_QUEUE BIT(0)
> +#define HCI_REQ_PEND (HCI_REQ_QUEUE | BIT(1))
> +#define HCI_REQ_CANCELED BIT(2)
> +
> +#define HCI_REQ_PENDING(req_status) (!!((req_status) & HCI_REQ_PEND))
>
> #define hci_req_sync_lock(hdev) mutex_lock(&hdev->req_lock)
> #define hci_req_sync_unlock(hdev) mutex_unlock(&hdev->req_lock)
> @@ -25,6 +28,9 @@ struct hci_request {
> int err;
> };
>
> +typedef void (*hci_request_wait_start_t)(struct hci_dev *hdev, void *data,
> + int err);
> +
> typedef int (*hci_cmd_sync_work_func_t)(struct hci_dev *hdev, void *data);
> typedef void (*hci_cmd_sync_work_destroy_t)(struct hci_dev *hdev, void *data,
> int err);
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index 209244fd9391..b9214eaffd2a 100644
> --- a/net/bluetooth/hci_core.c
> +++ b/net/bluetooth/hci_core.c
> @@ -4183,11 +4183,18 @@ static int hci_send_cmd_sync(struct hci_dev *hdev, struct sk_buff *skb)
> kfree_skb(skb);
> }
>
> - if (READ_ONCE(hdev->req_status) == HCI_REQ_PEND &&
> + if (HCI_REQ_PENDING(READ_ONCE(hdev->req_status)) &&
> !hci_dev_test_and_set_flag(hdev, HCI_CMD_PENDING)) {
> hci_dev_lock(hdev);
> +
> kfree_skb(hdev->req_skb);
> hdev->req_skb = skb_get(hdev->sent_cmd);
> +
> + if ((bt_cb(hdev->req_skb)->hci.req_flags & HCI_REQ_SKB) &&
> + cmpxchg(&hdev->req_status, HCI_REQ_QUEUE,
> + HCI_REQ_PEND) == HCI_REQ_QUEUE)
> + wake_up_interruptible(&hdev->req_wait_q);
> +
> hci_dev_unlock(hdev);
> }
>
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index efbd2e5387c0..058daf1f16df 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -25,7 +25,7 @@ static void hci_cmd_sync_complete(struct hci_dev *hdev, u8 result, u16 opcode,
> {
> bt_dev_dbg(hdev, "result 0x%2.2x", result);
>
> - if (READ_ONCE(hdev->req_status) != HCI_REQ_PEND)
> + if (!HCI_REQ_PENDING(READ_ONCE(hdev->req_status)))
> return;
>
> hdev->req_result = result;
> @@ -155,13 +155,16 @@ static void hci_request_init(struct hci_request *req, struct hci_dev *hdev)
> }
>
> /* This function requires the caller holds hdev->req_lock. */
> -struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
> - const void *param, u8 event, u32 timeout,
> - struct sock *sk)
> +static struct sk_buff *__hci_cmd_sync_sk_run(struct hci_dev *hdev, u16 opcode,
> + u32 plen, const void *param,
> + u8 event, u32 timeout,
> + struct sock *sk,
> + hci_request_wait_start_t wait_func,
> + void *wait_data)
> {
> struct hci_request req;
> struct sk_buff *skb;
> - int err = 0;
> + long err = 0;
>
> bt_dev_dbg(hdev, "Opcode 0x%4.4x", opcode);
>
> @@ -169,29 +172,58 @@ struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
>
> hci_cmd_sync_add(&req, opcode, plen, param, event, sk);
>
> - WRITE_ONCE(hdev->req_status, HCI_REQ_PEND);
> + WRITE_ONCE(hdev->req_status, wait_func ? HCI_REQ_QUEUE : HCI_REQ_PEND);
>
> err = hci_req_sync_run(&req);
> - if (err < 0)
> + if (err < 0) {
> + if (wait_func)
> + wait_func(hdev, wait_data, err);
> return ERR_PTR(err);
> + }
> +
> + if (wait_func) {
> + /* Sleep until we have started waiting for the event */
> + err = wait_event_interruptible_timeout(hdev->req_wait_q,
> + READ_ONCE(hdev->req_status) != HCI_REQ_QUEUE,
> + timeout);
> + if (err <= 0 || !HCI_REQ_PENDING(READ_ONCE(hdev->req_status)))
> + goto check_state;
> +
> + wait_func(hdev, wait_data, 0);
> + wait_func = NULL;
> +
> + timeout = err;
> + }
>
> err = wait_event_interruptible_timeout(hdev->req_wait_q,
> - READ_ONCE(hdev->req_status) != HCI_REQ_PEND,
> + !HCI_REQ_PENDING(READ_ONCE(hdev->req_status)),
> timeout);
>
> - if (err == -ERESTARTSYS)
> +check_state:
> + if (err == -ERESTARTSYS) {
> + if (wait_func)
> + wait_func(hdev, wait_data, -EINTR);
> return ERR_PTR(-EINTR);
> + }
>
> switch (READ_ONCE(hdev->req_status)) {
> case HCI_REQ_DONE:
> + if (wait_func)
> + wait_func(hdev, wait_data, -EAGAIN);
> err = -bt_to_errno(hdev->req_result);
> break;
>
> case HCI_REQ_CANCELED:
> - err = -hdev->req_result;
> + if (wait_func)
> + wait_func(hdev, wait_data, -ECANCELED);
> + err = -(long)hdev->req_result;
> + if (!IS_ERR_VALUE(err))
> + err = -ECANCELED;
> break;
>
> default:
> + if (wait_func)
> + wait_func(hdev, wait_data, -ETIMEDOUT);
> err = -ETIMEDOUT;
> break;
> }
> @@ -201,7 +233,7 @@ struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
> skb = hdev->req_rsp;
> hdev->req_rsp = NULL;
>
> - bt_dev_dbg(hdev, "end: err %d", err);
> + bt_dev_dbg(hdev, "end: err %ld", err);
>
> if (err < 0) {
> kfree_skb(skb);
> @@ -216,6 +248,15 @@ struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
>
> return skb;
> }
> +
> +/* This function requires the caller holds hdev->req_lock. */
> +struct sk_buff *__hci_cmd_sync_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
> + const void *param, u8 event, u32 timeout,
> + struct sock *sk)
> +{
> + return __hci_cmd_sync_sk_run(hdev, opcode, plen, param, event, timeout,
> + sk, NULL, NULL);
> +}
> EXPORT_SYMBOL(__hci_cmd_sync_sk);
>
> /* This function requires the caller holds hdev->req_lock. */
> @@ -255,14 +296,17 @@ struct sk_buff *__hci_cmd_sync_ev(struct hci_dev *hdev, u16 opcode, u32 plen,
> EXPORT_SYMBOL(__hci_cmd_sync_ev);
>
> /* This function requires the caller holds hdev->req_lock. */
> -int __hci_cmd_sync_status_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
> - const void *param, u8 event, u32 timeout,
> - struct sock *sk)
> +static int __hci_cmd_sync_status_sk_run(struct hci_dev *hdev, u16 opcode,
> + u32 plen, const void *param, u8 event,
> + u32 timeout, struct sock *sk,
> + hci_request_wait_start_t wait_func,
> + void *wait_data)
> {
> struct sk_buff *skb;
> u8 status;
>
> - skb = __hci_cmd_sync_sk(hdev, opcode, plen, param, event, timeout, sk);
> + skb = __hci_cmd_sync_sk_run(hdev, opcode, plen, param, event, timeout,
> + sk, wait_func, wait_data);
>
> /* If command return a status event, skb will be set to -ENODATA */
> if (skb == ERR_PTR(-ENODATA))
> @@ -281,6 +325,15 @@ int __hci_cmd_sync_status_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
>
> return status;
> }
> +
> +/* This function requires the caller holds hdev->req_lock. */
> +int __hci_cmd_sync_status_sk(struct hci_dev *hdev, u16 opcode, u32 plen,
> + const void *param, u8 event, u32 timeout,
> + struct sock *sk)
> +{
> + return __hci_cmd_sync_status_sk_run(hdev, opcode, plen, param, event,
> + timeout, sk, NULL, NULL);
> +}
> EXPORT_SYMBOL(__hci_cmd_sync_status_sk);
>
> int __hci_cmd_sync_status(struct hci_dev *hdev, u16 opcode, u32 plen,
> @@ -679,7 +732,7 @@ void hci_cmd_sync_cancel(struct hci_dev *hdev, int err)
> {
> bt_dev_dbg(hdev, "err 0x%2.2x", err);
>
> - if (READ_ONCE(hdev->req_status) == HCI_REQ_PEND) {
> + if (HCI_REQ_PENDING(READ_ONCE(hdev->req_status))) {
> hdev->req_result = err;
> WRITE_ONCE(hdev->req_status, HCI_REQ_CANCELED);
>
> @@ -697,7 +750,7 @@ void hci_cmd_sync_cancel_sync(struct hci_dev *hdev, int err)
> {
> bt_dev_dbg(hdev, "err 0x%2.2x", err);
>
> - if (READ_ONCE(hdev->req_status) == HCI_REQ_PEND) {
> + if (HCI_REQ_PENDING(READ_ONCE(hdev->req_status))) {
> /* req_result is __u32 so error must be positive to be properly
> * propagated.
> */
--
Pauli Virtanen
^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: Indicate PAST RX ready to userspace
2026-10-11 20:39 ` [PATCH 1/2] Bluetooth: hci_sync: add request wait hook for __hci_cmd_sync_sk Pauli Virtanen
2026-10-11 22:04 ` Pauli Virtanen
@ 2026-10-11 22:51 ` bluez.test.bot
1 sibling, 0 replies; 5+ messages in thread
From: bluez.test.bot @ 2026-10-11 22:51 UTC (permalink / raw)
To: linux-bluetooth, pav
[-- Attachment #1: Type: text/plain, Size: 1605 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/series/1183465/
---Test result---
Test Summary:
CheckPatch PASS 2.07 seconds
VerifyFixes PASS 0.07 seconds
VerifySignedoff PASS 0.06 seconds
GitLint PASS 0.40 seconds
SubjectPrefix PASS 0.10 seconds
BuildKernel PASS 29.46 seconds
CheckAllWarning PASS 33.16 seconds
CheckSparse PASS 36.03 seconds
BuildKernel32 PASS 29.22 seconds
CheckKernelLLVM PASS 32.85 seconds
TestRunnerSetup PASS 771.93 seconds
TestRunner_l2cap-tester PASS 12.40 seconds
TestRunner_iso-tester PASS 10.56 seconds
TestRunner_bnep-tester PASS 3.13 seconds
TestRunner_mgmt-tester PASS 61.09 seconds
TestRunner_rfcomm-tester PASS 4.37 seconds
TestRunner_sco-tester PASS 5.56 seconds
TestRunner_ioctl-tester PASS 4.58 seconds
TestRunner_mesh-tester PASS 5.43 seconds
TestRunner_smp-tester PASS 4.22 seconds
TestRunner_userchan-tester PASS 3.09 seconds
TestRunner_6lowpan-tester PASS 4.32 seconds
IncrementalBuild PASS 47.37 seconds
https://github.com/bluez/bluetooth-next/pull/897
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-10-11 22:51 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-11 20:39 [PATCH 0/2] Indicate PAST RX ready to userspace Pauli Virtanen
2026-10-11 20:39 ` [PATCH 1/2] Bluetooth: hci_sync: add request wait hook for __hci_cmd_sync_sk Pauli Virtanen
2026-10-11 22:04 ` Pauli Virtanen
2026-10-11 22:51 ` Indicate PAST RX ready to userspace bluez.test.bot
2026-10-11 20:39 ` [PATCH 2/2] Bluetooth: ISO: Signal BIS PAST ready to userspace via POLLOUT Pauli Virtanen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox