The `err` variable was overwritten by the k_mutex functions
before being logged, so it did not log the actual error.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Apply the U suffix to unsigned integer literal constants used in
contexts involving unsigned types (uint8_t, uint16_t, uint32_t,
uint64_t, size_t, etc.) with coding guideline 40 which requires that
"A u or U suffix shall be applied to all integer constants that are
represented in an unsigned type".
In addition to this requirement, this commit also ads U to any unsigned
literal integer for the sake of completeness.
Changes are limited to literal values in assignments, initializations,
comparisons, and for-loop bounds where the context is unambiguously
unsigned.
Assisted-by: GitHub Copilot:claude-sonnet-4.6
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
The broadcast ID is printed in the shell various places. Often the
shell user wants to copy this value and use it for future commands.
Remove any following commas, so that the user can easier click-to-mark
the value to copy it.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
If the server wrongly sends us a QoS state notification before
the stream was ever added to a group, we should just ignore it
rather than hitting the assert or dereferencing the group
later.
Additionally, LOG_ERR should not be used when validating input
from a remote device.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Instead of just checking the lower limit, use
is_valid_num_ases to also test the upper limit.
This approach is used by all other control point operations.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
Add missing type check in sdp_attr_parse() before recursively parsing
data element sequences. When checking the type of the next data if it
is a sequence, it did not first check if the current type is a
sequence.
It could lead to incorrect recursive parsing when the current type is
not a sequence but the next byte was incorrectly identified as a
sequence.
Fix the issue by checking the current data type firstly.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
bt_hci_le_enh_conn_complete() obtains a reference via
find_pending_connect() but drops it immediately after the
bt_conn_connected() callback, while the function still dereferences
conn->role afterwards in the central-role scanner restart block.
Using a pointer after its reference has been dropped is a
use-after-free in API contract terms, even if in practice the
connection survives because other references (the conn array's
intrinsic reference, L2CAP, etc.) keep the object alive at this
point.
Move the bt_conn_unref() call to the end of the function so the
locally held reference outlives all uses of conn.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
bt_avrcp_cover_art_ct_l2cap_connect() obtains a reference to the ACL
connection via bt_avrcp_ct_get_acl_conn() but then immediately drops
that reference before continuing to use the conn pointer for
bt_conn_index() and bt_bip_l2cap_connect(). Using a pointer after
its reference has been dropped is a use-after-free in API contract
terms, even if in practice the underlying connection survives because
other references (e.g. the L2CAP channel, the ACL link itself) keep
the object alive while AVRCP cover art is in use.
Move the bt_conn_unref() call to after the last use of conn on each
return path so that the locally held reference outlives all uses.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the two-line pattern:
bt_conn_unref(conn);
conn = NULL;
(and equivalent if-guarded variants) with bt_conn_drop(&conn) which
performs both operations atomically in a single step. The guard is
unnecessary because bt_conn_drop() handles a NULL pointer internally.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the two-line pattern:
bt_conn_unref(conn);
conn = NULL;
(and equivalent if-guarded variants) with bt_conn_drop(&conn) which
performs both operations atomically in a single step. The guard is
unnecessary because bt_conn_drop() handles a NULL pointer internally.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the two-line pattern:
bt_conn_unref(conn);
conn = NULL;
(and equivalent if-guarded variants) with bt_conn_drop(&conn) which
performs both operations atomically in a single step. The guard is
unnecessary because bt_conn_drop() handles a NULL pointer internally.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the two-line pattern:
bt_conn_unref(conn);
conn = NULL;
(and equivalent if-guarded or take-then-unref-then-NULL variants) with
bt_conn_drop(&conn) which performs both operations atomically in a
single step. The guard is unnecessary because bt_conn_drop() handles a
NULL pointer internally.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Add a new bt_conn_drop() function that atomically drops a connection
reference and clears the source pointer in a single step. This mirrors
the recently added net_buf_drop() API and replaces the common pattern:
bt_conn_unref(conn);
conn = NULL;
The new API handles a NULL pointer internally, so any guard around it
is unnecessary.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the manual two-step pointer transfer:
struct bt_conn *acl = iso->iso.acl;
if (acl != NULL) {
iso->iso.acl = NULL;
...
}
with bt_conn_take(), which atomically extracts the reference and clears
the original pointer in a single step.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Add calculations to set the cig->c_latency and cig->p_latency
so that the CIS established event contains valid values.
Signed-off-by: Emil Gydesen <emil.gydesen@nordicsemi.no>
There was a missing check in le_ecred_reconf_req() that the provided CIDs
are dynamic (only those can be reconfigured). Add proper handling in case
a non-dynamic CID is encountered in the PDU.
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
The HCI LE LTK Request event parameters for `rand` and `ediv` should be
byte arrays, not integer types, to match the HCI specification and avoid
unaligned structure member access.
- Change `rand` from uint64_t to uint8_t[8]
- Change `ediv` from uint16_t to uint8_t[2]
- Add local variables in le_ltk_request() to convert byte arrays to
integer types before passing to bt_smp_request_ltk()
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
The HCI LE Start Encryption command parameters for `rand` and `ediv`
should be byte arrays, not integer types, to match the HCI specification
and avoid unaligned structure member access warnings.
- Change `rand` from uint64_t to uint8_t[8]
- Change `ediv` from uint16_t to uint8_t[2]
- Update memcpy calls to remove unnecessary address-of operators
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Replace the two-line pattern:
net_buf_unref(buf);
buf = NULL;
with net_buf_drop(&buf) which performs both operations
atomically in a single step.
Assisted-by: GitHub Copilot:claude-sonnet-4.6
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
In bt_set_adv_param() and bt_br_init(), response buffers were
unreferenced but the pointer left live for significant amounts of
subsequent code. Use net_buf_drop() to make the intent explicit and
guard against accidental use-after-free.
Assisted-by: GitHub Copilot:claude-sonnet-4.6
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the two-line pattern:
net_buf_unref(buf);
buf = NULL;
with net_buf_drop(&buf) which performs both operations
atomically in a single step.
Assisted-by: GitHub Copilot:claude-sonnet-4.6
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the two-line pattern:
net_buf_unref(buf);
buf = NULL;
with net_buf_drop(&buf) which performs both operations
atomically in a single step.
Assisted-by: GitHub Copilot:claude-sonnet-4.6
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
g_pbap_app->tx_buf is a global struct member. On the error paths
the buffer was unconditionally unreferenced and cleared to NULL via
two separate statements with intervening code. Replace with
net_buf_drop() to combine both into a single atomic step.
Assisted-by: GitHub Copilot:claude-sonnet-4.6
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the two-line pattern:
net_buf_unref(br_chan->_sdu);
br_chan->_sdu = NULL;
with net_buf_drop(&br_chan->_sdu) which performs both operations
atomically in a single step.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the if-guarded two-line pattern:
if (avctp->reassembly_buf != NULL) {
net_buf_unref(avctp->reassembly_buf);
avctp->reassembly_buf = NULL;
}
with net_buf_drop(&avctp->reassembly_buf) which performs both operations
atomically in a single step. The guard is unnecessary when there is no
other logic conditional on the pointer being non-NULL, because
net_buf_drop() handles a NULL pointer internally. Where the if-body
contains additional logging, the guard is kept.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the if-guarded two-line pattern:
if (session->reasm_buf != NULL) {
net_buf_unref(session->reasm_buf);
session->reasm_buf = NULL;
}
with net_buf_drop(&session->reasm_buf) which performs both operations
atomically in a single step. The guard is unnecessary when there is no
other logic conditional on the pointer being non-NULL, because
net_buf_drop() handles a NULL pointer internally. Where the if-body
contains additional logging, the guard is kept.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the if-guarded two-line pattern:
if (session->rec_buf != NULL) {
net_buf_unref(session->rec_buf);
session->rec_buf = NULL;
}
with net_buf_drop(&session->rec_buf) which performs both operations
atomically in a single step. The guard is unnecessary because
net_buf_drop() handles a NULL pointer internally.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the two-line pattern:
net_buf_unref(*buf);
*buf = NULL;
with net_buf_drop(buf) which performs both operations atomically in a
single step.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the if-guarded two-line pattern:
if (*buf) {
net_buf_unref(*buf);
*buf = NULL;
}
with net_buf_drop(buf) which performs both operations atomically in a
single step. The guard is unnecessary because net_buf_drop() handles a
NULL pointer internally.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the two-line pattern:
net_buf_unref(buf);
buf = NULL;
(and equivalent if-guarded variants) with net_buf_drop(&buf) which
performs both operations atomically in a single step. The guard is
unnecessary because net_buf_drop() handles a NULL pointer internally.
Assisted-by: GitHub Copilot:claude-opus-4.7
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the two-line pattern:
net_buf_unref(buf);
buf = NULL;
with net_buf_drop(&buf) which performs both operations
atomically in a single step.
Assisted-by: GitHub Copilot:claude-sonnet-4.6
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
When the peer disconnects mid-transfer, the bt_att and bt_att_chan are
freed (att_chan_detach()/att_reset(), then bt_att_released()) while a
deferred unenhanced-ATT "sent" callback is still queued on the system
workqueue: the ATT TX buffer destroy work runs att_on_sent_cb() ->
bt_att_sent(), which dereferences the freed context and bus-faults in
sys_slist_get(&att->reqs).
att_on_sent_cb() already guards against a detached bearer, but it does
so by dereferencing meta->att_chan, which by then is a dangling pointer
(and faults once the channel slab slot has been reused).
Null any pending ATT TX metadata referencing the channel in
bt_att_released(), before the channel is freed, so the guard drops the
deferred callback instead of dereferencing freed memory. Teardown and
the destroy work both run on the cooperative system workqueue, so this
is serialized and needs no lock.
Assisted-by: Claude:claude-opus-4.8
Signed-off-by: Hayden Ball <hayden@playerdata.com>
Add shell commands to retrieve and clear LPN friendship timing
statistics. Available as mesh stat lpn_get and mesh stat lpn_clear
when CONFIG_BT_MESH_LOW_POWER is enabled. Rename existing frame
statistic commands to mesh stat adv_get and mesh stat adv_clear.
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Aleksandr Khromykh <aleksandr.khromykh@nordicsemi.no>
Add timestamp-based measurement of LPN friendship timing parameters
(ReceiveDelay and ReceiveWindow) to the statistic module. Timestamps
are captured at poll TX end, scanner enable, and response RX/timeout
using k_uptime_ticks(). Measured values are stored in microseconds.
Instrument the LPN state machine in lpn.c to call the statistic
update functions at each protocol event.
Assisted-by: Claude:claude-opus-4.6
Signed-off-by: Aleksandr Khromykh <aleksandr.khromykh@nordicsemi.no>
Most HCI drivers have never handled the return value from the
driver-to-host receive path in any way, and they haven't really needed to
either since the buffer was always consumed regardless of success or
failure.
To be aligned with Zephyr coding guidelines without having to add
minimal value code to every driver, change bt_hci_recv() to return void
instead. A new bt_hci_recv_err() API is also added for any use case where
access to the error is needed.
There's also a related semantic change to the recv callback (passed to
bt_hci_open()) in that it's now expected to *not* consume the buffer in
case of error. There was only a single place in the host that needed an
update for this, while e.g. HCI raw always returned success.
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
There's no point in having driver specific data structs if the only thing
they contain is the common data struct. Just declare the common data struct
directly as dev->data.
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Now that we have a common data struct, we can take advantage of it and
move the recv callback into it. This way the common layer takes care of
assigning, dispatching and clearing the callback. Drivers now have to use
the new bt_hci_recv() API to pass data to the layer above them (typically
a host stack). Additionally, the drivers no longer get a recv parameter as
part of their open() callback, since that's all handled by the common
layer.
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Make these static (no need to export them) and also name them in a way that
doesn't class with the HCI driver API prefix.
Signed-off-by: Johan Hedberg <johan.hedberg@silabs.com>
Replace the local PDU access macros in net.c with static helper
functions.
Using functions avoids preprocessor namespace pollution from common
macro names such as SRC while preserving the same behavior. The
functions are local to net.c and are expected to be inlined by the
compiler, resulting in no functional change.
Fixes#109401
Signed-off-by: Gobi M <gobi@aerlync.com>
subnet_keys_destroy() guarded the destroy of keys->priv_beacon with
#if defined(CONFIG_BT_MESH_V1d1), while net_keys_create() guards the
matching import with #if defined(CONFIG_BT_MESH_PRIV_BEACONS).
Commit 981c79b7ce ("Bluetooth: Mesh: Drop explicit support for
Bluetooth Mesh 1.0.1") removed the CONFIG_BT_MESH_V1d1 Kconfig but
left this stray reference behind. With V1d1 gone, the destroy branch
is permanently dead code, so every successful net_keys_create() leaks
one PSA key slot.
In a long-running test that repeatedly provisions and resets a node
(no persistent settings, in-RAM CDB only), the leak accumulates one
PSA volatile-key slot per provisioning round. With the default
CONFIG_MBEDTLS_PSA_KEY_SLOT_COUNT=16, bt_mesh_private_beacon_key()
fails after ~12 rounds with:
<err> bt_mesh_net_keys: Unable to generate private beacon key
<err> bt_mesh_net: Failed creating subnet
<err> bt_mesh_main: Failed to create network
Align the destroy guard with the import guard so the leak goes away.
Signed-off-by: Lingao Meng <menglingao@xiaomi.com>
The S-frame should only be sent when there are no I-frames to be sent,
as I-frames have higher priority. Previously, the S-frame flag was
checked in the same condition as other flags, which could cause S-frames
to be sent even when I-frames were available.
Fix by moving the S-frame check to a separate condition that first
verifies no I-frames are pending via `l2cap_br_send_i_frame()`. Also
move the clearing of `L2CAP_FLAG_SEND_S_FRAME` to the end of the
function to ensure it is cleared after any frame is sent, not just
when an S-frame is explicitly sent.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Introduce the helper macros `BT_GOEP_INIT_V1` and `BT_GOEP_INIT_V2`
to simplify and standardize GOEP and GOEP transport initialization for
the upper layer.
Add two new initialization macros that encapsulate the repetitive
pattern of setting up mutual back-pointers between `bt_goep` instances
and related GOEP version-specific transport structures:
- `BT_GOEP_INIT_V1`: Initializes GOEP v1 (RFCOMM) transport by setting
the back-pointer from transport to GOEP, assigning the v1 pointer,
and clearing the v2 pointer.
- `BT_GOEP_INIT_V2`: Initializes GOEP v2 (L2CAP) transport by setting
the back-pointer from transport to GOEP, assigning the v2 pointer,
and clearing the v1 pointer.
And adding type checking via `BUILD_ASSERT` and `SAME_TYPE` in these
two macros to ensure type consistent in building time.
Update all GOEP-based profiles (BIP, PBAP, MAP) and shell code to use
these new added macros.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Introduce a unified `bt_goep_transport` structure to simplify GOEP
transport handling across all profiles.
The added new `bt_goep_transport` structure contains both `v1` and
`v2` transport instances. The upper layer profiles could include the
structure instead of requiring separate structure members when both
GOEP V1.1 and V2 transport are required at the same time.
Update all GOEP-based profiles (BIP, PBAP, MAP) and shell code to use
the unified `goep_transport` structure member instead of separate
`goep_transport_v1` and `goep_transport_v2` members. This reduces code
duplication and provides a consistent pattern across all profiles.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Add explicit GOEP v1 and v2 transport structures to PBAP instances
to support the refactored GOEP transport version handling.
Add `bt_goep_transport_v1` and `bt_goep_transport_v2` structure
members to both PBAP client (PCE) and server (PSE) instance types to
align with the refactored GOEP transport handling that uses explicit
version pointers instead of a boolean flag and union.
Initialize the appropriate v1 or v2 structure and set the
corresponding pointer in the parent GOEP instance based on the
transport type (RFCOMM for v1, L2CAP for v2) during connection
establishment and accept operations.
Add NULL parameter validation in PSE accept functions to prevent
potential null pointer dereferences before initializing the GOEP
transport structures.
The change completes the PBAP integration with the new GOEP transport
architecture.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
Add explicit GOEP v1 and v2 transport structures to MAP instances
to support the refactored GOEP transport version handling.
Add `bt_goep_transport_v1` and `bt_goep_transport_v2` structure
members to all MAP instance types (MCE MAS, MCE MNS, MSE MAS, MSE
MNS) to align with the refactored GOEP transport handling that uses
explicit version pointers instead of a boolean flag and union.
Initialize the appropriate v1 or v2 structure and set the
corresponding pointer in the parent GOEP instance based on the
transport type (RFCOMM for v1, L2CAP for v2) during connection
establishment and accept operations.
The change completes the MAP integration with the new GOEP transport
architecture.
Signed-off-by: Lyle Zhu <lyle.zhu@nxp.com>
