After a device-initiated Virtual Cable Unplug the initiator must
wait for the remote to tear down the L2CAP channels instead of
disconnecting them itself. The old code disconnected locally as
soon as the PDU was sent.
Change vcu_disconnect to a k_work_delayable and, on a successful send,
arm a 5S fallback timer (HID_VCU_DISCONNECT_TIMEOUT) as a safety net;
cleanup cancels it if the remote disconnects first. If the send fails,
keep disconnecting immediately since the remote may never have seen the
unplug.
Signed-off-by: Make Shi <make.shi@nxp.com>
Add support for testing RFCOMM credit-based flow control in the SPP
shell commands.
Add new optional parameters to SPP shell commands:
- `credit_limit`: Configure custom RX credit limit per DLC
- `hold_credit`: Enable asynchronous receive completion mode
When `hold_credit` is enabled:
- Received buffers are stored in a FIFO queue
- The recv callback returns `-EINPROGRESS` to hold credits
- New `recv_complete` command allows manual completion of receive
operations to test credit refill behavior
Update all SPP shell commands (register_with_channel,
register_with_uuid, connect_by_channel, connect_by_uuid) to accept
these optional parameters for flow control testing.
Add documentation with examples demonstrating flow control testing
scenarios for both server and client roles.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Reorder function definitions in rfcomm.c to remove the declaration of
the function `rfcomm_dlc_update_credits()`.
Move `rfcomm_dlc_get_available_credits()` and
`rfcomm_dlc_update_credits()` before `rfcomm_dlc_tx_worker()` since
the worker function calls `rfcomm_dlc_update_credits()`.
Move `rfcomm_dlc_tx_worker()` and `rfcomm_dlc_connected()` before
`rfcomm_handle_sabm()` since SABM handler calls
`rfcomm_dlc_connected()`.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Add support for asynchronous receive completion to allow applications
to control when RFCOMM data reception is considered complete.
Change the `recv` callback in `bt_rfcomm_dlc_ops` to return an `int`
instead of void. Applications can now return `-EINPROGRESS` to indicate
that data processing is asynchronous and will be completed later by
calling the new `bt_rfcomm_dlc_recv_complete()` API.
When `-EINPROGRESS` is returned:
- The application takes ownership of the buffer reference
- Increment `rx_credit_inprogress` counter
- For non-CFC sessions, send MSC command with FC bit set
- RX credits are decremented immediately but not refilled until
`bt_rfcomm_dlc_recv_complete()` is called
Update all existing RFCOMM recv callbacks (HFP AG, HFP HF, GOEP, SPP
shell, etc) to return 0 to maintain current behavior.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Add support for configurable default RX credits per DLC through the
new `rx_credit_limit` field in structure `bt_rfcomm_dlc` .
Previously, all DLCs used a fixed default credit value
RFCOMM_MAX_CREDITS. This change allows applications to specify a custom
initial RX credit count for each DLC, providing better control over
credit based flow control behavior.
When `rx_credit_limit` is set to 0, the rfcomm driver uses the
internal default value. Otherwise, the specified value (the upper limit
is RFCOMM_MAX_CREDITS) is used as the maximum credit count for the
specific DLC.
Also, limit the maximum value of RFCOMM_MAX_CREDITS to 255 to avoid
overflow.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
bt_disable() tore down the ISO and ACL connections before resetting the
controller. The teardown returns the packets in flight to their senders
with an error, but the controller still owns those packets at that point
and keeps reporting their completion until it is reset. With an active
BIS broadcaster the Number Of Completed Packets event for such a packet
then finds the connection's pending list empty, which triggers the
"packets count mismatch" assertion in hci_num_completed_packets().
Reset the controller first, once the low-priority RX processing has been
stopped, and tear down the connections afterwards: completions reported
up to the reset are accounted for as usual, and the host only returns
what the controller no longer holds. Neither the ISO nor the connection
cleanup sends HCI commands, so they do not need the controller to be
running. A failed reset now also leaves the connections in place, in
line with the ready state it restores.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
In case due to unlucky scheduling we ended up with queued buffers either in
the HCI command or connection queues, be sure to purge those queues cleanly
in the TX processor functions.
The command queue is additionally purged by bt_disable() itself right
after it marks the transport closed, and bt_hci_cmd_send() re-checks the
transport state after queuing a command: a sender that passed the state
check just before the transport was closed either finds it closed on the
re-check and takes its command back, failing with -EHOSTDOWN, or has the
command dropped by the purge, which completes synchronous senders with an
error. Either way no command is left behind to be lost by the next
bt_enable() or sent to the next controller session.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Since the HCI command sending APIs are public, we should have a check that
the transport is actually open before attempting to queue a command for
sending. If we don't do this it'll trigger an assert due to a command
timeout, which is less friendly (and harder to debug) for users.
The public command sending APIs document the new -EHOSTDOWN return
value. The buffer parameter may be NULL, so the early return only
releases a buffer that was actually passed in.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Make a clear distinction of flags intended to protect against race
conditions related to host stack state transitions and flags intended to
indicate the current availability of the HCI transport and Bluetooth APIs
in general.
In particular, the following flags protect against state transition race
conditions: BT_DEV_ENABLING and BT_DEV_DISABLING
Whereas the following indicate the availibilty of the HCI transport and
Bluetooth APIs in general: BT_DEV_OPEN and BT_DEV_READY.
We also do an additional simplification in bt_disable() where we don't
bother doing special mapping for -ENOSYS -> -ENOTSUP since this special
error code was never documented.
The transition flags also give the enable/disable APIs a consistent
error model: -EALREADY when the requested transition is already in
progress or complete, and -EAGAIN when the opposite transition is in
progress, i.e. bt_disable() before an asynchronous bt_enable() has
completed, or bt_enable() while bt_disable() is still running. Both
APIs now document their return values.
The transition flags are kept across the flag reset performed when an
HCI_Reset completes, which happens both during initialization and in
bt_disable(): clearing ENABLING there would prevent bt_finalize_init()
from ever setting READY, and clearing DISABLING would end the RX
teardown early. The RX teardown predicate that main gained in the
meantime is converted to the DISABLING flag as part of the rebase.
The k_work_busy_get() check that bt_disable() gained for the
asynchronous bt_enable() case is replaced by the ENABLING flag, which
covers that case together with a synchronous bt_enable() running in
another thread.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
The maximum length of an attribute value is 512 octets (Core
Specification Vol 3, Part F, Section 3.2.9), but the GATT client
delivered notification and indication values of any length permitted by
the ATT MTU to subscribe callbacks. Subscribers that size their buffers
according to the specification limit can then be handed more data than
they have room for.
Drop notifications, indications and multiple handle value notification
tuples whose value is larger than BT_ATT_MAX_ATTRIBUTE_LEN before
delivering them. The remaining tuples of a multiple handle value
notification are still delivered, and oversized indications are still
confirmed, since the confirmation only acknowledges receipt of the PDU.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
bt_sdp_discover_cancel() has been declared in sdp.h since the SDP
client API was introduced, but no implementation has ever existed: any
caller would fail to link. Without it, a profile that queues a
discovery and then hits an error on an unrelated path has no way to
take the request back, and must keep the discovery parameters object
alive and its callback armed until the discovery machinery gets around
to it.
Implement the semantics the declaration documents: a waiting request,
i.e. one queued with bt_sdp_discover() whose resolution has not yet
started, is removed from the session and its callback will not be
called. The request currently being resolved cannot be canceled
cleanly - its request PDU is out and the response may already be under
processing - so canceling it is refused with -EINPROGRESS and the
callback still fires. Requests parked for the next session round
(reqs_next) are canceled the same way as waiting ones.
The declaration took the parameters object as const, but canceling
inherently modifies it: the request node is unlinked from the
session's list. Drop the const qualifier rather than casting it away
in the implementation. Since the function never had an implementation,
no existing caller can be affected by the signature change.
For the "callback will not be called" guarantee to hold, the places
that take a request off the waiting list and act on it must do so
under the session lock so that they cannot race with the removal:
- sdp_client_discover() now marks the selected request active
(session->param) while still holding the lock, rather than after
releasing it in the per-type send functions.
- sdp_client_params_iterator() now holds the lock across the removal
of the completed request instead of only taking it afterwards.
- sdp_client_disconnected() now pops each request under the lock
before invoking its callback, instead of iterating the list
unlocked.
sdp_client_alloc_buf() additionally needs to handle an empty request
list, which can now happen when every queued request is canceled while
the channel is still being established. Returning NULL there makes
sdp_client_connected() disconnect the no-longer-needed channel; the
previous code dereferenced the head of the list unconditionally.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
bt_le_scan_start() sets up the explicit scan parameters and the scan
callback with scan_explicit_params_mutex held, but bt_le_scan_stop()
clears the callback and removes the scanner user without taking it, so
a stop running concurrently with a start can tear down the state the
start is in the middle of setting up.
Take the same lock in bt_le_scan_stop(). As in bt_le_scan_start(), the
lock is taken without waiting, so that a stop from the Bluetooth
workqueue cannot block on a start that is waiting for a command
completion.
Assisted-by: Claude:claude-opus-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
bt_settings_store_id() and bt_settings_store_irk() defer the actual
settings write to a work item that reads bt_dev.id_addr/bt_dev.irk with
a length derived from bt_dev.id_count at execution time.
bt_disable() resets bt_dev.id_count without waiting for those work
items. If a store is still pending at that point, the handler later
computes a zero-length record and persists an empty "bt/id" or
"bt/irk" entry, erasing the identity stored in the settings backend.
If the handler is mid-execution, it can read the identity data while
it is being reset, persisting a torn record.
Simply canceling the work items would trade one bug for another: an
identity created or updated shortly before bt_disable() would silently
never reach the settings backend, and a later bt_enable() would load
stale identity data. Instead, add bt_settings_flush(), which completes
any pending stores while the identity state is still valid, and call
it from bt_disable() before the identity state is reset.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
bt_avdtp_connect() sets session->br_chan.chan.conn before calling
bt_l2cap_chan_connect() in order to detect simultaneous local and
remote AVDTP signaling connection attempts. If bt_l2cap_chan_connect()
fails before the channel has been added to the connection, for example
with -ENOTCONN when the ACL is no longer connected, or through one of
the early argument and state checks in bt_l2cap_br_chan_connect(), no
disconnected callback will ever run for the channel and the marker is
never cleared. a2dp_get_connection() treats a non-NULL marker as a
session in use, so every subsequent bt_a2dp_connect() for that
connection index fails with -ENOMEM, including attempts on future ACL
connections that reuse the same connection object.
Clear the marker on the error path of bt_l2cap_chan_connect(), holding
avdtp_sem_lock like the other accesses to the marker. It is only
cleared if it still holds the value set above, since a failure that
occurs after the channel was added to the connection has already
cleared it in bt_l2cap_br_chan_del().
The accept side sets the same marker in bt_avdtp_l2cap_accept(), but
there the L2CAP server code owns the channel lifecycle after accept
returns and deletes the channel with bt_l2cap_br_chan_del() on the
failure paths, which clears the marker.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
a2dp_get_connection() wipes the whole bt_a2dp object, including the
embedded AVDTP session, with memset() when the object is not currently
associated with a connection. The session's _release_work, submitted
from bt_avdtp_l2cap_disconnected(), may still be queued at that point,
and wiping a queued work item corrupts the work queue's pending list.
Skip the reuse and return NULL while the release work is still
pending, mirroring the check on the AVDTP side in the preceding
commit. Both callers already handle a NULL return by rejecting the
connection attempt, which can be retried once the release work has
completed.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
bt_avdtp_l2cap_disconnected() submits session->_release_work to defer
the release of the session's endpoints. Both bt_avdtp_connect() and
bt_avdtp_l2cap_accept() unconditionally k_work_init() the same work
item when the session object is reused for a new connection.
Re-initializing a work item that is still queued corrupts the work
queue's pending list.
The work item cannot simply be canceled on reuse, since the deferred
endpoint release must always run. Instead, reject the session reuse
with an error while the release work is still pending; the connection
can be retried once the work has completed. The check is done under
avdtp_sem_lock, and since the session has no L2CAP connection at that
point, no new submission of the work can race with it.
A similar lifecycle issue with session->timeout_work being
re-initialized while potentially still scheduled is being addressed
separately.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
If the connection drops in the middle of reassembling a fragmented
vendor dependent response, ct->reassembly_buf is left referenced. The
buffer is only reclaimed if the same connection index later starts a
new reassembly, so in the meantime such disconnections can exhaust the
RX pool, which is only CONFIG_BT_MAX_CONN deep.
Drop the reassembly buffer on disconnection. This is the same class of
issue as the TG vendor response TX state fixed in the preceding
commit.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
avrcp_disconnected() neither cancels tg->vd_rsp_tx_work nor releases
the buffers queued in tg->vd_rsp_tx_pending. If the connection drops
in the middle of a fragmented vendor dependent response, the pending
buffer, from a pool that is only CONFIG_BT_MAX_CONN deep, is leaked
with its TX_ONGOING flag still set in the buffer user data. Since the
TG object is selected by connection index, a new connection on the
same index finds the stale buffer at the head of the pending list, and
the TX state machine remains permanently stalled with any new
responses queued behind the stale head and never sent.
Cancel the TX work and release all queued TX buffers on disconnection.
The per-buffer TX context, including the TX_ONGOING flag, lives in the
buffer user data, so releasing the buffers also resets the TX state.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Since this a "get" function that does not, and never should,
modify the provided advertising set pointer, modify it to be
const.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Use S8 flags when setting up Coded PHY reception.
This keeps receive timing consistent with other coded RX setup paths.
Pass selected advertising PHY flags to coded TX delay calculations.
This avoids treating S2/S8 selection as unused during delay setup.
Use unsigned zero literals for the zero-valued PHY macros.
Document the RX/TX coding flag choice in advertising paths.
Signed-off-by: Daniel Polimac <danielpolimac@gmail.com>
Replace literal Coded-PHY flags with the named S2 and S8 constants
throughout the Nordic and OpenISA controller paths.
Part of #37458
Signed-off-by: Daniel Polimac <danielpolimac@gmail.com>
Move channel state transition to config rsp sent callback to ensure
the response is actually transmitted before marking the channel as
connected. Previously, the state was updated with connected state and
the channel `connected` will also be involved immediately after queuing
the response. In the callback `connected`, the packets will be sent by
the upper layer profiles to the sending queue immediately. Because the
config RSP and the upper-layer profile send data through different
channels, the host cannot guarantee that these packets are sent in the
order of the sending function calls. Therefore, it is possible that
profile packets may precede config RSP packets on the HCI bus.
This change introduces `l2cap_br_config_rsp_sent_cb()` to handle the
`L2CAP_FLAG_CONN_RCONF_DONE` flag and state transition only after the
config response is successfully sent. Error responses continue to use
the non-callback send path as they don't affect channel state.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Replace programming error checking with runtime assertions in
`mce_mns_accept()` and `mse_mas_accept()` functions. The instance
and callback validation checks are converted to `__ASSERT()` calls
since these conditions indicate programming errors rather than
runtime failures.
Initialize mce_mns and mse_mas to NULL to ensure the assertions can
properly detect uninitialized instances passed from the accept
callbacks.
The change aligns with the error handling approach used in the GOEP
layers' accept callbacks, treating invalid instance parameters and
initialization failures as programming errors that should be caught
during development.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Replace programming error checking with runtime assertions in
`bip_rfcomm_accept()` and `bip_l2cap_accept()` functions. The bip
instance validation is converted to `__ASSERT()` calls since a NULL
instance indicates a programming error in the upper layer's accept
callback implementation rather than a runtime failure.
Initialize bip to NULL to ensure the assertion can properly detect
uninitialized instances passed from the accept callbacks.
The change aligns with the error handling approach used in the GOEP
layers' accept callbacks, treating invalid instance parameters as
programming errors that should be caught during development.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Replace programming errors with runtime assertions in
`pbap_pse_rfcomm_accept()` and `pbap_pse_l2cap_accept()` functions.
The pbap_pse instance validation is converted to `__ASSERT()` calls
since a NULL instance indicates a programming error in the upper
layer's accept callback implementation.
Initialize pbap_pse to NULL to ensure the assertion can properly
detect uninitialized instances passed from the accept callbacks.
The change aligns with the error handling approach used in the GOEP
layer's accept callbacks, treating invalid instance parameters as
programming errors that should be caught during development.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
In original implementation, there is a corner case that the instance
has been allocated by upper layer and passed it through `accept()`
callbacks. And normally the allocated instance will be destroyed after
the connection broken. While in the post-accept, due to some errors,
it is possible that the connection may not be accepted by
`goep_rfcomm_accept()` or `goep_l2cap_accept()` functions. And all of
these errors are programming errors.
Replace programming error checking with runtime assertions in
`goep_rfcomm_accept()` and `goep_l2cap_accept()` functions. The
parameter validation and initialization error checks are converted to
`__ASSERT()` calls since these conditions indicate programming errors
rather than runtime failures.
The change is used to treat invalid instance parameters as programming
errors that should be caught during development.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
The `goep_rfcomm_init()` and `goep_l2cap_init()` functions are used
internally during transport setup, and their error conditions
represent programming errors.
Convert both initialization functions from returning error codes to
using runtime assertions.
The change treats transport initialization failures as programming
errors that should be caught during development.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
The function `bt_obex_reg_transport()` is used internally by the
Bluetooth Classic Host, and its parameter should not be NULL unless
there is a programming error.
Convert `bt_obex_reg_transport()` from returning error codes to using
runtime assertions. Replace if-condition NULL pointer checks with
`__ASSERT()` to catch invalid parameters during development.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
For the BIS specific data configuration, we should check
stream_param->data for NULL before supplying that to memcpy,
even if stream_param->data_len == 0, as memcpy does not
support NULL pointers.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
The check was comparing `rank` to `rank`, rather than
the minimum and maxmimum values of rank.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
BT_MESH_IVU_HOURS is BT_MESH_IVU_MIN_HOURS divided by
CONFIG_BT_MESH_IVU_DIVIDER with integer truncation, so the step by
which bt_mesh.ivu_duration grows need not divide 96. For dividers 5,
7, 9, 10, 13, 17, 18 and 19 the step is 19, 13, 10, 9, 7, 5, 5 and
5, and the duration steps over the 96 hour boundary without landing
on it. The old exact-multiple test then never flushed the beacon
cache, so a beacon refused because of the limit stayed cached, every
identical retransmission was dropped as a duplicate, and the node
never ran IV Update or IV Index Recovery once the limit expired.
Assisted-by: GitHub Copilot:claude-opus-5
Signed-off-by: Aleksandr Khromykh <aleksandr.khromykh@nordicsemi.no>
bt_mesh_iv_update_test() lifts the 96-hour dwell time of the IV Update
procedure, but left the Secure Network beacon cache untouched. A
beacon that arrives while the limit is in force is cached by
net_beacon_resolve() and only then refused, so every identical
retransmission is filtered as a duplicate and the IV Update never
happens.
ivu_refresh() already clears the cache when the limit expires in
normal operation. Test mode lifts it without the timer running, so
apply the same invalidation there. Caching stays independent of test
mode, as required by MshPRT v1.1.1, section 3.11.5.1.
Assisted-by: GitHub Copilot:claude-opus-5
Signed-off-by: Aleksandr Khromykh <aleksandr.khromykh@nordicsemi.no>
hfp_hf_disconnected() cancels hf->work and hf->deferred_work, but not
hf->slc_work, which is submitted to the system work queue both from
hfp_hf_connected() and from the SDP discovery callback. Once the
object has been marked as free (hf->acl == NULL), hfp_hf_create()
wipes the whole object with memset() when the slot is reused. Wiping a
work item that is still queued corrupts the work queue's pending list.
Additionally, an ongoing SDP discovery cannot be canceled
(bt_sdp_discover_cancel() is declared but has no implementation), so
the discovery callback may fire after the RFCOMM DLC has been
disconnected and re-submit slc_work for an already released object.
Fix this by canceling slc_work in hfp_hf_disconnected(), and by making
the discovery callback check that the object has not been released by
hfp_hf_disconnected() while the discovery was still ongoing. To make
that check reliable also on the connecting path, assign hf->acl before
starting the discovery in hfp_hf_create(), since the callback may
otherwise run before the assignment and treat the object as released.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
The Passkey Entry protocol carries the 6-digit passkey one bit per
confirm/random round, over exactly 20 rounds. The peripheral however
re-armed BT_SMP_CMD_PAIRING_CONFIRM in smp_pairing_random() before
checking whether the last round had been reached, so a peer was able to
keep sending confirm/random pairs indefinitely once the real passkey bits
were exhausted.
Those extra rounds also passed validation: smp->passkey is a 20-bit value
held in a uint32_t, so rounds 20 through 31 simply compared the zero bits
above the passkey. Beyond round 31 the shift in smp_send_pairing_confirm()
and sc_smp_check_confirm() exceeds the width of the promoted operand and
is undefined behaviour.
Only arm BT_SMP_CMD_PAIRING_CONFIRM when a round actually remains, so that
a 21st Pairing Confirm is rejected as an unexpected command and pairing
fails. Additionally reject a round past the last one wherever the round
counter is incremented, so it can never exceed SMP_PASSKEY_ROUNDS and the
terminating comparisons stay exact. Replace the open-coded round count
with SMP_PASSKEY_ROUNDS.
Fixes#114792
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
The PAwR advertiser previously discarded fragmented Periodic Advertising
Response Reports, so applications never received responses that spanned
multiple HCI reports.
Add optional reassembly, guarded by the new
CONFIG_BT_PER_ADV_RSP_REASSEMBLY (disabled by default). Fragments are
buffered per advertising set and delivered to the pawr_response callback
once the COMPLETE report arrives. Buffer overflow, a fragment for a
different chain, or an RX_FAILED report resets the reassembly state.
Signed-off-by: Alex Zhou <alex.zhou@nxp.com>
The extended advertising reassembly timeout handler mutates the
reassembling_advertiser state, which is also read and written by the
extended advertising report processing without any locking. The report
processing runs on the Bluetooth workqueue, while the timeout handler
was scheduled with k_work_reschedule() and so ran on the system
workqueue.
This is serialized only implicitly, by cooperative scheduling: it
holds because both workqueues run at cooperative priority (the
Bluetooth subsystem clamps SYSTEM_WORKQUEUE_PRIORITY to the
cooperative range for this reason) and as long as they share a CPU.
The latter is not structural: on SMP builds cooperative threads on
different CPUs run concurrently, in which case a timeout firing
concurrently with report processing can flip the state mid-sequence,
e.g. letting a chain that should have been discarded be delivered.
Schedule the timeout on the Bluetooth workqueue instead, making the
serialization structural instead of relying on scheduler
characteristics. This also makes the non-blocking
k_work_cancel_delayable() performed by the report processing reliably
effective, as canceller and handler now share a workqueue. The
remaining reset callers (HCI reset complete, bt_finalize_init() and
scan teardown) run while no advertising reports are being processed.
This is host-internal work with no application callback, so there are
no application-visible context changes.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
bt_hfp_ag_connect() wipes the whole AG object with memset() when
bt_rfcomm_dlc_connect() fails. At that point the SDP discovery started
by hfp_ag_create() is still ongoing, and the embedded
ag->sdp_param._node is linked in the SDP client's request list, so the
memset() corrupts that list in place. It can also wipe a queued
ag->slc_work item, corrupting the work queue's pending list.
This is the same issue as the HF side one fixed in the preceding
commit; fix it in the same way by deferring the release of the object
to the SDP discovery callback, since an ongoing SDP discovery cannot
be canceled (bt_sdp_discover_cancel() is declared in the public header
but has no implementation).
Additionally, make the discovery callback check that the AG object has
not been released by hfp_ag_disconnected() while the discovery was
still ongoing. Since the discovery cannot be canceled, the callback
would otherwise set the DISCOVER_DONE flag and re-submit slc_work for
an already released object. To make that check reliable also on the
connecting path, assign ag->acl_conn before starting the discovery in
hfp_ag_create(), since the callback may otherwise run before the
assignment and treat the object as released.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
bt_hfp_hf_connect() wipes the whole HF object with memset() when
bt_rfcomm_dlc_connect() fails. At that point the SDP discovery started
by hfp_hf_create() is still ongoing, and the embedded
hf->sdp_param._node is linked in the SDP client's request list, so the
memset() corrupts that list in place. It can also wipe a queued
hf->slc_work item, corrupting the work queue's pending list.
An ongoing SDP discovery cannot be canceled (bt_sdp_discover_cancel()
is declared in the public header but has no implementation), so the
object cannot be safely torn down synchronously. Instead, keep the
object allocated and mark it with the new RELEASING flag, making the
SDP discovery callback release it once the discovery has completed,
rather than proceeding with SLC establishment. In the unlikely case
that the discovery has already completed when the RFCOMM connection
creation fails, release the object immediately.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
sc_save() only calls sc_store() when a bond already exists for the peer,
so a client that subscribes to Service Changed before it bonds gets an
sc_cfg entry in RAM that is never written to settings. iOS does exactly
this: it subscribes right after connecting and only pairs later, once the
user accepts the dialog.
bt_gatt_identity_resolved() does not cover it either, since its sc_store()
call is likewise conditional on the bond already existing, and identity
resolution happens during key distribution rather than after the bond has
been committed.
The result is that the subscription survives in RAM for the current
connection but is lost across a reboot, so a characteristic change made
while disconnected is never indicated and the client keeps a stale cache.
Reconnecting papers over it, because the client subscribes again and by
then the bond exists.
Store the Service Changed configuration from bt_gatt_pairing_complete(),
where the ccc and cf data is already persisted for the same reason.
Fixes: #111078
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Assisted-by: Claude:claude-opus-5
MISRA C:2012 Rule 8.2 requires every parameter in a function type to be
named, including the parameters of function pointer parameters.
The LLCP remote request helper, the LLL test and scan helpers and the
PSoC 6 BLESS receive thread left parameters unnamed. The BLESS thread
gets ARG_UNUSED() for the arguments it deliberately ignores.
Name them after the arguments the documentation or the
definitions already use. No functional change.
Signed-off-by: Anas Nashif <anas.nashif@intel.com>
Report the error code when bt_disable() fails, including the new
-EAGAIN case where initialization has not yet completed.
Signed-off-by: Guotao Zhang <guotao.zhang@nxp.com>
When bt_enable() is called with a ready callback, bt_init() runs
asynchronously on the system workqueue. If bt_disable() is invoked
before that work item completes, it races with bt_init() over
bt_dev.sent_cmd, which can be freed twice and trigger a buf.c:472
assert.
Fix this by checking k_work_busy_get(&bt_dev.init) at the top of
bt_disable(). If init is still queued or running, return -EAGAIN so
the caller can retry once the ready callback fires.
Note: the same race can occur with a synchronous bt_enable(NULL)
called concurrently from another thread; that case will be covered
by BT_DEV_ENABLING/BT_DEV_DISABLING transition flags introduced in
a follow-up change.
Document the new -EAGAIN return code in bt_disable()'s Doxygen.
Signed-off-by: Guotao Zhang <guotao.zhang@nxp.com>
The BAP stream callbacks does not mirror the ASCS states 1:1
so there are a few cases where the callbacks are called without
matching the ASCS state.
BAP stream ops are intended to be the same for sink and source
ASEs, even though they have different state machine,
but sink streams did
stopped()
disabled()
qos_configured()
when exiting the streaming state (without a release), and source
streams did
disabled()
stopped()
qos_configured()
Refactor unicast_client_ep_notify_app a bit to have the same
callbacks in the same order for sinks and sources:
disabled()
stopped()
qos_configured()
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
bt_le_ext_adv_get_info() handed out a pointer to the advertising set's
random_addr, which is only written when a per-set random address is
programmed. When the set advertises with a public identity address the
reported address was never populated, and with a controller without the
extended advertising feature it was never written at all.
Instead of adding a second address field alongside random_addr, widen
the meaning of the existing one: rename it to adv_addr, the address the
set advertises with regardless of type. The per-set random address
cases keep being updated in bt_id_set_adv_random_addr(), so the
reported address stays correct across RPA and NRPA rotation, and
le_update_private_addr() refreshes the legacy advertiser, which
advertises with the device-wide random address.
The cases that do not program a per-set address - a public identity
address, and the device-wide random address on controllers without the
extended advertising feature, e.g. an RPA set through
bt_id_set_private_addr() when privacy is enabled - are saved by a new
bt_id_save_adv_addr(), which the advertising parameter setting calls
once the controller has accepted the parameters. Saving only at that
point ensures a failed parameter update does not overwrite the reported
address with one the controller never took into use.
Widening the field is safe for the existing readers: the connection
responder address and OOB paths that read it only execute in
configurations where the set advertises with a random address, in which
case the value is unchanged, and a pending random address is programmed
from the field before bt_id_save_adv_addr() runs.
For the BT_HCI_OWN_ADDR_RPA_OR_* types the controller substitutes a
locally generated RPA whenever the peer is in the resolving list, which
the host cannot observe; the configured fallback address is reported in
that case, as documented on the info struct.
Fixes: #112667
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Move the responder role check outside the is_bip_primary_connect()
conditional so it applies to all connection types. Remove the
initiator role check from the secondary path since a responder role
is valid for secondary connections initiated by the remote device.
This ensures that a BIP instance configured as responder cannot
initiate any client connections, regardless of connection type.
Signed-off-by: Cheng Chang <cheng.chang@nxp.com>
bt_bap_unicast_group_reconfig did not properly set the fields
related to CONFIG_BT_ISO_TEST_PARAMS nor packing.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Add the CIG parameters of the unicast group (SDU intervals, transport
latencies, framing, packing and, when CONFIG_BT_ISO_TEST_PARAMS is
enabled, the flush timeouts and the ISO interval) as well as the
has_been_connected state to struct bt_bap_unicast_group_info, so that
applications can retrieve all relevant information about a group.
The framing is stored internally using the ISO values, and is converted
back to the BAP QoS configuration values when reported.
The existing bsim tests for bt_bap_unicast_group_get_info have been
expanded to verify the new values, including for a group with
asymmetric parameters and for a reconfigured group.
Assisted-by: Copilot coding agent
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
The ep->unicast_group was never assigned and thus
has_been_connected was never set for the BAP unicast groups.
Change to use the stream->group instead.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
