Skip to content

feat(L2CAP): add disconnect API and harden CoC send/error handling #1139

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
h2zero merged 1 commit into from
Mar 31, 2026
Merged

feat(L2CAP): add disconnect API and harden CoC send/error handling #1139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 29 additions & 1 deletion src/NimBLEL2CAPChannel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,14 @@ int NimBLEL2CAPChannel::writeFragment(std::vector<uint8_t>::const_iterator begin

case BLE_HS_ENOMEM:
case BLE_HS_EAGAIN:
/* ble_l2cap_send already consumed and freed txd on these errors */
NIMBLE_LOGD(LOG_TAG, "ble_l2cap_send returned %d (consumed buffer). Retrying shortly...", res);
ble_npl_time_delay(ble_npl_time_ms_to_ticks32(RetryTimeout));
continue;

case BLE_HS_EBUSY:
NIMBLE_LOGD(LOG_TAG, "ble_l2cap_send returned %d. Retrying shortly...", res);
/* Channel busy; txd not consumed */
NIMBLE_LOGD(LOG_TAG, "ble_l2cap_send returned %d (busy). Retrying shortly...", res);
os_mbuf_free_chain(txd);
ble_npl_time_delay(ble_npl_time_ms_to_ticks32(RetryTimeout));
continue;
Comment on lines 135 to 147
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 31, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🌐 Web query:

NimBLE ble_l2cap_send mbuf buffer ownership on BLE_HS_ENOMEM BLE_HS_EAGAIN error codes

💡 Result:

In NimBLE, for ble_l2cap_send(struct ble_l2cap_chan *chan, struct os_mbuf *sdu), the function consumes (frees) the supplied mbuf only on success. On error returns such as BLE_HS_ENOMEM (heap exhaustion, no memory) or BLE_HS_EAGAIN (operation would block), the caller retains ownership of the mbuf and must free it using os_mbuf_free_chain(sdu) to avoid leaks. BLE_HS_ENOMEM typically occurs due to mbuf pool exhaustion when the stack cannot allocate additional buffers for transmission or fragmentation. BLE_HS_EAGAIN indicates the controller cannot accept the full packet immediately (e.g., transmit queue full), so the remainder may be enqueued, but the input mbuf is not consumed on this error.

Citations:

Shiver me timbers! Ye've got a memory leak on yer hands, cap'n!

The code be mistaken about buffer ownership. NimBLE's ble_l2cap_send only frees the mbuf on success; on errors like BLE_HS_ENOMEM and BLE_HS_EAGAIN, the caller retains ownership and must free it. Yet yer code be skippin' os_mbuf_free_chain(txd) on those very errors while properly freeein' it for BLE_HS_EBUSY. This be a memory leak that'll drain yer treasure chest over time, matey!

Add os_mbuf_free_chain(txd) before continue in both the BLE_HS_ENOMEM and BLE_HS_EAGAIN cases, just like ye do fer BLE_HS_EBUSY.

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@src/NimBLEL2CAPChannel.cpp` around lines 135 - 147, The BLE send-error
handling incorrectly assumes nimble frees the mbuf for BLE_HS_ENOMEM and
BLE_HS_EAGAIN; add os_mbuf_free_chain(txd) before the continue in the
BLE_HS_ENOMEM and BLE_HS_EAGAIN case blocks (the same cleanup already present in
the BLE_HS_EBUSY branch) in the switch that handles ble_l2cap_send results so
txd is freed on those error paths.

Expand Down Expand Up @@ -197,6 +203,28 @@ bool NimBLEL2CAPChannel::write(const std::vector<uint8_t>& bytes) {
return true;
}

bool NimBLEL2CAPChannel::disconnect() {
if (!this->channel) {
NIMBLE_LOGW(LOG_TAG, "L2CAP Channel not open");
return false;
}

int rc = ble_l2cap_disconnect(this->channel);
if (rc != 0 && rc != BLE_HS_ENOTCONN && rc != BLE_HS_EALREADY) {
NIMBLE_LOGE(LOG_TAG, "ble_l2cap_disconnect failed: rc=%d %s", rc, NimBLEUtils::returnCodeToString(rc));
return false;
}

return true;
}

uint16_t NimBLEL2CAPChannel::getConnHandle() const {
if (!this->channel) {
return BLE_HS_CONN_HANDLE_NONE;
}
return ble_l2cap_get_conn_handle(this->channel);
}

// private
int NimBLEL2CAPChannel::handleConnectionEvent(struct ble_l2cap_event* event) {
channel = event->connect.chan;
Expand Down
8 changes: 8 additions & 0 deletions src/NimBLEL2CAPChannel.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,14 @@ class NimBLEL2CAPChannel {
/// NOTE: This function will block until the data has been sent or an error occurred.
bool write(const std::vector<uint8_t>& bytes);

/// @brief Disconnect this L2CAP channel.
/// @return true on success, false on failure.
bool disconnect();

/// @brief Get the connection handle associated with this channel.
/// @return Connection handle, or BLE_HS_CONN_HANDLE_NONE if not connected.
uint16_t getConnHandle() const;

/// @return True, if the channel is connected. False, otherwise.
bool isConnected() const { return !!channel; }

Expand Down
Loading