k_work_reschedule() withdraws a pending timeout, not a submission that
has already been made: once work_timeout() has fired it clears
K_WORK_DELAYED_BIT and queues the work, and a later reschedule arms a
new timeout while that submission stays queued. delayable_msg_handler()
can therefore run for an expiry that another context has already
consumed, and it popped the head of busy_ctx without looking at
fired_time.
A msg is then transmitted before its deadline, which truncates the
random delay the Access layer transmission rules require and defeats
the collision avoidance the module exists to provide. Any call to
reschedule_delayable_msg() between the timeout firing and the system
workqueue running leaves such a stale invocation behind; the purge path
in bt_mesh_delayable_msg_manage() is the easiest way to hit it, since
it consumes the head itself.
Split the two ways the queue is drained. take_expired_msg() pops the
head only if it is due, and the handler uses it, so a stale invocation
finds nothing to send and only re-arms. take_earliest_msg() pops the
head whatever its fired_time and keeps the purge path forcing out the
oldest msg as before. The due test runs in the same critical section as
the dequeue, so a concurrent context cannot swap the head in between,
and the sending half of push_msg_from_delayable_msgs() becomes
send_delayable_msg() for both callers.
The handler still sends at most one msg per invocation and re-arms;
when the next msg is already due the delay is K_NO_WAIT and the work
runs again straight away, as before.
msg_expired() and reschedule_delayable_msg() compare with a signed
delta, which the previous curr_time < fired_time test got wrong at the
32-bit uptime wrap.
Assisted-by: Claude:claude-opus-4.7
Signed-off-by: Aleksandr Khromykh <aleksandr.khromykh@nordicsemi.no>
free_ctx, busy_ctx, free_chunks and the k_work_delayable are mutated
from two contexts: bt_mesh_delayable_msg_manage() on the BT RX
workqueue and delayable_msg_handler() on the system workqueue. Both are
cooperative, but the enqueuer blocks in bt_mesh_access_send() and
bt_rand(), which lets the handler interleave on unsynchronized state.
That produced two NULL dereferences. delayable_msg_handler() used the
result of sys_slist_get() on busy_ctx unchecked, so once the other
context had drained the list, the fired_time update faulted at a
NULL-based address. allocate_delayable_msg_ctx() tested
sys_slist_is_empty() and then assumed a separate sys_slist_get() would
succeed.
Guard every list mutation, and the fired_time of any ctx on busy_ctx,
with a k_spinlock that is never held across a blocking call.
push_msg_from_delayable_msgs() detaches the head atomically, so
concurrent pushers cannot send the same msg, and the handler no longer
touches busy_ctx.
Detaching before the send has a consequence. A msg being sent is on no
list, so the drain in bt_mesh_delayable_msg_stop() cannot see it, and
on -EBUSY or -ENOBUFS it was queued again behind that drain. On the
suspend path that never resolves, as adv_create_from_pool() refuses to
allocate while suspended. Drop a msg that reaches the front of the
queue while the stack is down instead, so nothing is handed to the
transport after a suspend or a reset, however it got queued.
bt_mesh_suspend() sets BT_MESH_SUSPENDED and bt_mesh_reset() clears
BT_MESH_VALID before stopping the module.
The release-and-report sequence this shares with the send path and with
the drain moves into complete_delayable_msg().
The unit test initialises the mesh flags in tc_setup(), since the
module only sends for a provisioned, running node.
Assisted-by: Claude:claude-opus-4.7
Signed-off-by: Aleksandr Khromykh <aleksandr.khromykh@nordicsemi.no>
release_delayable_msg_ctx() used sys_slist_find_and_remove() and only
appended the ctx to free_ctx when it had been found on busy_ctx. But
bt_mesh_delayable_msg_manage()'s ENOMEM error path releases a ctx that
was popped from free_ctx and never inserted on busy_ctx, so
find_and_remove() returns false and the ctx is silently lost from both
lists, permanently reducing the pool.
Append unconditionally; find_and_remove() is a no-op when the ctx is
not on busy_ctx, so the behaviour on the normal release path (ctx
present on busy_ctx) is preserved.
Assisted-by: Claude:claude-opus-4.7
Signed-off-by: Aleksandr Khromykh <aleksandr.khromykh@nordicsemi.no>
Refactor `sdp_client_get_total()` to add comprehensive error handling
and validation for SDP sequence parsing.
Check the remaining buffer length before pulling data from the buffer.
Using `BT_SDP_SEQ_UNSPEC` to check the SEQ type. And only handle the
frame if the total length is valid.
The function now properly validates input data and returns appropriate
error codes, preventing potential buffer underflow and invalid data
processing in SDP client operations.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Migrate the BR/EDR (Classic) host's internal work items from the system
workqueue to the dedicated Bluetooth workqueue by using the
bt_work_submit(), bt_work_schedule() and bt_work_reschedule() helpers.
This covers AVCTP, AVDTP, BR, HFP AG state-machine, SLC and call work,
HID Device (INTR connection timeout and Virtual Cable Unplug
disconnect), L2CAP BR, and RFCOMM.
Keep HFP AG TX-notification work on the system workqueue because it
releases TX contexts that Bluetooth-workqueue handlers may wait for.
Since the Bluetooth workqueue stays alive across bt_enable()/bt_disable()
cycles, a delayable item armed at teardown time can fire after its
connection is gone. Add the same ACL connection guard to
l2cap_br_rtx_timeout() that the retransmission and monitor timeout
handlers already have, and a corresponding guard to
rfcomm_session_rtx_timeout(), so that a stale timeout cannot attempt
signalling (including a potentially blocking K_FOREVER PDU allocation)
on a connection that no longer exists.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Migrate the LE host's internal work items from the system workqueue to the
dedicated Bluetooth workqueue by using the bt_work_schedule(),
bt_work_reschedule() and bt_work_submit() helpers.
This covers advertising (limited advertising timeout), ATT (channel
timeout, EATT connection work), connections (deferred work and
auto-initiated procedures), GATT (service changed, delayed store,
database hash, multiple notifications), identity (RPA update), L2CAP (RTX
and RX work), settings (ID and IRK store), and SMP (timeout).
Migrating the per-connection deferred work is what keeps the existing
teardown code correct: that work item performs the L2CAP channel and
profile teardown (including the ATT and SMP disconnected callbacks), and
the non-blocking k_work_cancel*() calls in those paths only reliably stop
a concurrently running work item when both run on the same workqueue.
Keeping teardown on the system workqueue while the canceled work items
run on the Bluetooth workqueue would turn those cancellations into races.
For the same reason the SMP timeout moves along with it.
Additionally, cancel a dynamic L2CAP channel's RX work in
l2cap_chan_destroy() so that a queued work item cannot be left behind
with a reference to a channel object that the application may free or
re-use after teardown.
Update the id and conn host unit test mocks accordingly: id.c now uses
bt_work_schedule() and conn.c uses bt_work_submit(), bt_work_schedule()
and bt_work_reschedule(), so add fakes for these to the respective
hci_core.c mocks (and drop the now-unused k_work_schedule() and
k_work_reschedule() fakes from the kernel mocks).
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Add bt_work_submit(), bt_work_schedule() and bt_work_reschedule()
helpers that submit internal host work to the dedicated Bluetooth RX
workqueue instead of the shared system workqueue.
Start the workqueue on the first bt_enable() call and keep it alive for
the lifetime of the system. Delayable host work can outlive an individual
Bluetooth enable/disable cycle, so aborting and reinitializing the queue
could strand work item state.
The RX thread abort that bt_disable() used to perform is replaced with a
graceful equivalent: before resetting and closing the transport,
bt_disable() now stops queuing new low-priority HCI packets, discards
the ones already queued, and cancels the RX work synchronously. An
in-flight RX work item is thereby waited for while the transport is
still able to serve any HCI commands it may issue, and a packet that
slips into the queue during the teardown is dropped by the handler
instead of being dispatched after the transport is gone. Unlike the
abort, this cannot terminate the RX handler mid-execution with a buffer
still referenced.
The RX workqueue and dedicated TX processor thread are intentionally kept
separate. Merging them would allow a blocking application callback in the
RX context to stall command processing and reintroduce the deadlock the
TX processor thread was created to avoid.
Subsequent commits migrate selected delayed and immediate host work items
to these helpers.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Remove the BT_RECV_CONTEXT Kconfig choice and its BT_RECV_WORKQ_SYS
and BT_RECV_WORKQ_BT options. Low-priority HCI packets now always run
on the dedicated Bluetooth RX workqueue.
BT_RECV_WORKQ_SYS existed to save the RX thread stack on
memory-constrained targets, primarily nRF51. Removing the alternate path
simplifies the host threading model and removes a configuration-dependent
RX context and its associated test matrix. The following commit retunes
the affected in-tree nRF51 sample configurations.
This breaking Kconfig change is tracked by RFC #113007. Update the public
GATT callback context documentation, release notes and migration guide,
and remove tests specific to the system-workqueue RX path.
Assisted-by: Claude:claude-fable-5
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
If an ASE is already in a pending state change, we reject
any operations on it, as that would, from the point of view
of the client, result in invalid state changes.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Add support for handling disconnects during CAP procedures.
We treat a disconnect the same way as a write reject / error,
and stop the procedure when it happens.
This explicit handling is needed since many of the subprocedures
in CAP are notification based, and thus we won't get an error
from GATT when there's a disconnect.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
att_write_rsp did 2 things:
1) Performed the write (calling the attr->write callback)
2) When called from att_write_req, it did also allocate
and send a response.
1) is kept in the function, and the function has been
renamed to better fit the functionality of the function.
2) Has been moved from the function to att_write_req, as
that code was only used for that function. Additionally
the code was refactored to allocate the right buffer when
needed, instead of always allocating a BT_ATT_OP_WRITE_RSP
and then unref this and allocate a BT_ATT_OP_ERROR_RSP on
error.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
The local and remote CIS Create procedures retain an RX node for the
later host notification, for example when accepting a CIS_IND with a
future instant, or while awaiting the Host reply to a CIS request.
If an unexpected LL Control PDU arrives before that retained node is
consumed, it is routed to the active procedure and takes the invalid
PDU path in llcp_lp_cc_rx() or llcp_rp_cc_rx(). Those paths
completed the procedure without releasing the retained RX node,
violating the invariant checked in llcp_lr_check_done() and
llcp_rr_check_done(), and leaking the referenced RX node memory.
Release the retained RX node before completing the procedure,
mirroring the fix already applied to the remote Connection Update and
the PHY Update procedures.
Add unit tests to tests/bluetooth/controller/ctrl_cis_create that
inject an unexpected LL Control PDU while a retained RX node is
pending. Without the fix each test triggers the retained node
assertion in ull_llcp_local.c or ull_llcp_remote.c; with the fix the
connection is terminated with BT_HCI_ERR_LMP_PDU_NOT_ALLOWED and all
procedure contexts are released.
Assisted-by: GitHub Copilot: Claude Opus 4.8
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
The local and remote PHY Update procedures retain the RX node for the
later host notification, for example when accepting a LL_PHY_UPDATE_IND
with a future instant.
If an unexpected LL Control PDU arrives before that retained node is
consumed, it is routed to the active procedure and takes the invalid
PDU path in llcp_lp_pu_rx() / llcp_rp_pu_rx(). Those paths completed
the procedure without releasing the retained RX node, violating the
invariant checked in llcp_lr_check_done() / llcp_rr_check_done() and
leaking the referenced RX node memory.
Release the retained RX node and clear the reference before completing
the procedure, mirroring the same fix in the Connection Update
procedure.
Add a unit test to tests/bluetooth/controller/ctrl_phy_update that
completes the remote PHY Update request with a future instant, then
sends an unexpected LL_LENGTH_REQ before the instant is reached.
Without the fix the test triggers the retained node assertion in
ull_llcp_remote.c; with the fix the connection is terminated with
BT_HCI_ERR_LMP_PDU_NOT_ALLOWED and all procedure contexts are released.
Add tests/bluetooth/controller/ctrl_phy_update case
test_phy_update_periph_loc_unexpected_pdu_awaiting_instant: a
peripheral initiates a local PHY Update, the central replies with a
LL_PHY_UPDATE_IND at a future instant so the local procedure retains
the RX node, and then an unexpected LL_REJECT_IND is routed to the
active local procedure before the instant is reached.
Assisted-by: GitHub Copilot: Claude Opus 5
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
The remote Connection Update / Connection Parameter procedure retains
the RX node for later host notification, for example when accepting a
LL_CONNECTION_UPDATE_IND with a future instant.
If an unexpected LL Control PDU arrives before that retained node is
consumed, it is routed to the active remote procedure and takes the
invalid PDU path in llcp_rp_cu_rx(). That path completed the procedure
without releasing the retained RX node, violating the invariant checked
in llcp_rr_check_done() and leaking the referenced RX node memory.
Release the retained RX node and clear the reference before completing
the procedure.
Add a unit test to tests/bluetooth/controller/ctrl_conn_update that
sends a valid LL_CONNECTION_UPDATE_IND with a future instant, followed
by an unexpected LL_LENGTH_REQ before the instant is reached. Without
the fix the test triggers the retained node assertion in
ull_llcp_remote.c; with the fix the connection is terminated with
BT_HCI_ERR_LMP_PDU_NOT_ALLOWED and all procedure contexts are released.
Verified with the ctrl_conn_update, ctrl_invalid, ctrl_collision and
ctrl_terminate controller unit tests, all passing.
Assisted-by: GitHub Copilot: Claude Opus 5
Signed-off-by: Vinayak Kariappa Chettimada <vich@nordicsemi.no>
ISO now verifies that the data received has the same length
as the length field in the BT_ISO_SINGLE or BT_ISO_START
fragment. If it does not, it is dropped in the same way
that ACL drops data.
`bt_conn.rx_len` was only used by ISO, so the field was moved
to `bt_conn_iso` instead. Unlike ACL where the buffer contains
the L2CAP header, the ISO `conn->rx` only contains the data
without the header, so we need to store this information
somewhere, and cannot just rely on the header within conn->rx.
Lastly, there was also a check added for BT_ISO_START to check
if it should have been a BT_ISO_SINGLE instead, effectively
disallowing BT_ISO_CONT or BT_ISO_END with length = 0.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
att_prep_write_rsp() declared its len parameter as uint8_t while its only
caller, att_prepare_write_req(), passes buf->len which is a uint16_t. Any
prepare write carrying more than 255 octets of value data therefore had
its length silently truncated to the low byte before being stored in the
uint16_t len member of struct prep_data.
This is only reachable once the negotiated ATT MTU is allowed to exceed
255 via CONFIG_BT_L2CAP_TX_MTU, in which case long writes corrupt the
data handed to the attribute write callback, and a value length whose low
byte happens to be zero degenerates into repeated zero-length writes.
Widen the parameter to uint16_t so that it matches both the caller and
the struct member it is assigned to.
Fixes: #108227
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Assisted-by: Claude:claude-opus-5
when an S-frame needs to be sent, the function
`l2cap_br_ret_fc_data_pull()` calls `l2cap_br_send_i_frame()` to
determine whether an I-frame should be sent instead. If an I-frame
needs to be sent, the S-frame is not transmitted; instead, the
S-frame information is sent within the I-frame.
Currently, when `L2CAP_FLAG_RET_I_FRAME` was set, the code would
block a S-frame transmission if any outstanding PDUs existed, and
try to perform a retransmission I-frame, regardless of whether
they needed retransmission. Similarly, when
`L2CAP_FLAG_RET_I_FRAMES` was set, the S-frame transmission was
always blocked without checking if I-frame retransmission was
actually needed.
It leads to an issue that no any I-frames or S-frames are performed.
Fix the L2CAP BR/EDR I-frame transmission logic to properly handle
retransmission scenarios by checking if retransmission is actually
required before blocking new frame transmission.
The change introduces a helper function
`l2cap_br_retransmit_is_required()` that checks if any outstanding
PDUs in the transmission window actually need retransmission by
examining the retransmit flag.
With the changes, the code checks if the outstanding queue is
non-empty for the single frame retransmission case, and uses the new
helper to verify if I-frame retransmission is truly required for the
multiple frames case. This prevents unnecessary blocking of S-frame
transmission when outstanding PDUs don't require retransmission.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
For the `!is_p_to_c && !is_c_to_p` check we did not log the
index of which parameter was using an invalid paramater.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Currently IRK generation via bt_rand() is open-coded in id_create().
Extract a shared bt_gen_irk() helper so that all IRK generation call
sites use the same code path and will always have the same behavior.
This also prepares for the bt_id_reset_irk() API (PR #114930) which
will use the same helper.
Assisted-by: Augment:GLM-5.2
Signed-off-by: Sharon Lin <slin@atmosic.com>
BT_HCI_ERR_LOCALHOST_TERM_CONN is not a valid reason to
send to the remote.
Changed to BT_HCI_ERR_REMOTE_USER_TERM_CONN
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Since we may receive a BASE often, some of the log statements
could end up flooding the log. Add RATELIMIT to those that either
directly discards the BASE or logs as WRN.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Adds checks for the number of subgroups as well as the size
of the metadata when we parse the BASE, in the case that the
bt_bap_scan_delegator_mod_src_param does not support the number
of subgroups or the metadata.
If the number of subgroups is the issue, then we discard the BASE
as we would otherwise write incorrect data to the receive state.
If the size of the metadata is the issue, then we just don't write
it to the receive state, as that is not mandatory as per BASS.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Remove the special handling of metadata_len == 0 to keep existing
metadata, as this prevents applications from actually setting the
metadata_len to 0.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Use #include <> instead of #include "" to include a header file which path
is not relative to the directory path of the file emitting the #include
directive.
This change was made running scripts/check_quoted_includes.py script
proposed in https://github.com/zephyrproject-rtos/zephyr/pull/112135
with the Linux shell command below and manually selecting the applicable
changes:
$ ./scripts/check_quoted_includes.py -w subsys/bluetooth/host/classic/
Signed-off-by: Etienne Carriere <etienne.carriere@st.com>
The interval validation in valid_adv_ext_param() capped the maximum
interval at 0x4000 (the legacy advertising limit). Extended advertising
uses a 24-bit interval field with a permitted range of 0x000020–0xFFFFFF
per Core Spec Vol 4, Part E, 7.8.53. Use BT_HCI_LE_PRIM_ADV_INTERVAL_MAX
when BT_LE_ADV_OPT_EXT_ADV is set, and remove the now-unused
adv_interval_max_get() helper.
Assisted-by: Augment:claude-sonnet-4.6
Signed-off-by: Sharon Lin <slin@atmosic.com>
Change the registration of the PA sync callbacks to be
more lazy (only do it when we are trying to do any PA
related activities).
This not only defers such operations until they are useful,
but also helps reduce noise when using the LE Audio shell.
This also reduces the scope of per_adv_sync_cb as well
as moving the related functions to a single guard.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Added help text for the BT_CONN, BT_AUDIO_RX, and BT_AUDIO_TX options.
While these are promptless, they are worth explaining and should show up
in Kconfig reference doc page.
Signed-off-by: Benjamin Cabé <benjamin@zephyrproject.org>
Add additional check to avoid printing the same error
for each SDU. If the SDU length changes, then it is still
printed every time to notify the user about
such behavior from the remote (useful for debugging), so it
is only if the SDU size is wrong and does not change that
we rate limit it.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
The member_params variable can be quite large, and scales with
CONFIG_BT_MAX_CONN. Make it static to avoid requiring a large
stack size.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
This commit resolves -Wpointer-sign warnings in the Bluetooth Mesh
subsystem by updating comp_data_pages[] to store a const char *path and
updating bt_mesh_k5 to natively accept const char *p, avoiding the need
for pointer casts.
This is part of a larger effort to fix pointer-sign warnings across the
codebase in preparation for dropping -Wno-pointer-sign globally.
Related to #8921
Signed-off-by: harshit kudhial <harshitkudhial@gmail.com>
The last callers of bt_testing_tx_tid_get() were removed by
commit 28be8909a6 ("Bluetooth: host: remove TX thread"), which also
changed the function to return the system workqueue thread. That return
value has in turn been stale since
commit f101976e31 ("Bluetooth: Host: Run tx processor on its own thread")
moved TX processing to a dedicated thread whenever
CONFIG_BT_TX_PROCESSOR_THREAD is enabled (the default).
Instead of fixing the return value of a function nobody calls, remove
it, together with the leftover extern declarations in the bsim tests.
The att/pipeline test enabled CONFIG_BT_TESTING solely for this
function, so drop it there. The att/sequential test claimed the same in
a comment, but actually also depends on CONFIG_BT_TESTING for
bt_conn_suspend_tx(), so only the stale comment is corrected there.
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Assisted-by: Claude:claude-fable-5
If we are pending PAST, we do not have a pa_sync object to delete, but
we should not rejec the request to stop the sync, as that indicates
that we should just stop waiting.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Two CCC callbacks used K_FOREVER instead of MUTEX_TIMEOUT
as the mutex timeout. Change to be consistent and to avoid
blocking everything in case that it could not lock the mutex.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Remove the pa_sync_state parsing from
cmd_bap_scan_delegator_bis_synced as it was not used by the command,
and caused the parsing of the bis_synced value to fail.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
In the sync_state_get_or_new we should compare with the src_id instead
of the pointer. If the receive state was added locally via
cmd_bap_scan_delegator_add_src then the src_id is assigned, but not
the pointer. This caused any locally added receive states to be
unavailable, and thus causing error prints when using the
shell module.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Mark static variables bip_app, rfcomm_server, and l2cap_server with
ZTESTABLE_STATIC to allow them to be accessed from unit tests while
maintaining static linkage in production builds.
Signed-off-by: Cheng Chang <cheng.chang@nxp.com>
Fix typo in `BT_AVDTP_UNSUPPORTED_CONFIGURATION` constant name.
The constant was misspelled as `BT_AVDTP_UNSUPPORTED_CONFIGURAION`.
Update the enum definition in `avdtp.h` and all references in `a2dp.c`
to use the correct spelling.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Handling Previous Track and Next Track in this state
allowed mpl to dereference and advance track state,
then transition to Paused. This behavior contradicts
the MCS state model and causes failures.
Reject Next Track and Previous Track commands while
the Media Player is in the Inactive state. Handle
both opcodes like the other commands that are invalid
while the player is inactive.
Signed-off-by: Alex Ciascai <ext-alexandru.ciascai@nordicsemi.no>
Rename some struct fields from ase to ase_id[s] where
the value reflect an ASE ID. This is to avoid any confusion
with `struct bt_ascs_ase *ase` variables.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
