Skip to content

Conversation

HaavardRei
Copy link
Contributor

L2CAP channels will now, along with the ident, store the opcode of the pending request. This commit expands the ident lookup function to also compare received response types to this opcode, and will ignore unsolicited responses.

@HaavardRei HaavardRei force-pushed the l2cap_ident_matching branch from f701d07 to f2a963c Compare August 4, 2025 13:28
@HaavardRei HaavardRei marked this pull request as ready for review August 4, 2025 13:31
@zephyrbot zephyrbot added area: Bluetooth area: Bluetooth Host Bluetooth Host (excluding BR/EDR) labels Aug 4, 2025
@Thalley Thalley removed their request for review August 4, 2025 13:42
Copy link
Contributor

@alwa-nordic alwa-nordic left a comment

Choose a reason for hiding this comment

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

Looks good other than my change request below.

@HaavardRei HaavardRei force-pushed the l2cap_ident_matching branch from f2a963c to e1b49ab Compare August 5, 2025 08:55
alwa-nordic
alwa-nordic previously approved these changes Aug 5, 2025
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds request/response validation to L2CAP channels by storing the opcode of pending requests and validating that responses match the expected request type. This prevents processing of unsolicited responses and improves protocol robustness.

  • Expands the ident lookup function to compare received response types against stored request opcodes
  • Adds pending_req field to store the opcode of outgoing requests in L2CAP channels
  • Updates all request sending functions to set the pending request opcode and all response handlers to validate opcodes

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
subsys/bluetooth/host/l2cap.c Updates lookup functions to validate request/response matching and sets pending_req field when sending requests
include/zephyr/bluetooth/l2cap.h Adds pending_req field to bt_l2cap_le_chan structure for storing request opcodes
Comments suppressed due to low confidence (1)

sizeof(struct bt_l2cap_sig_hdr) +
sizeof(struct bt_l2cap_disconn_req)),
CONFIG_BT_CONN_TX_USER_DATA_SIZE, NULL);

Copy link
Preview

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

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

The magic number 0x100 for ANY_OPCODE lacks documentation explaining why this specific value was chosen and its significance in the context of L2CAP opcodes.

Suggested change
/*
* ANY_OPCODE is used as a sentinel value to represent a wildcard match for L2CAP opcodes.
* L2CAP opcodes are 1-byte values (0x00-0xFF); 0x100 is chosen as it is outside the valid range,
* ensuring it does not conflict with any real opcode.
*/

Copilot uses AI. Check for mistakes.

jhedberg
jhedberg previously approved these changes Aug 7, 2025
@HaavardRei HaavardRei force-pushed the l2cap_ident_matching branch from f8e625f to c291c14 Compare August 7, 2025 12:26
Copy link
Contributor

@alwa-nordic alwa-nordic left a comment

Choose a reason for hiding this comment

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

We must address the Copilot comment about le_disconn_rsp.

@zephyrbot zephyrbot requested a review from PavelVPV August 7, 2025 13:40
alwa-nordic
alwa-nordic previously approved these changes Aug 7, 2025
L2CAP channels will now, along with the ident, store the opcode of the
pending request. This commit expands the ident lookup function to also
compare received response types to this opcode, and will ignore
unsolicited responses.

Setting of idents for channels are moved after verification of buffer
allocation for the request to be sent. A TODO is added for improving
this functionality at a later time.

Signed-off-by: Håvard Reierstad <[email protected]>
Copy link

sonarqubecloud bot commented Aug 7, 2025

@HaavardRei HaavardRei requested a review from jhedberg August 7, 2025 14:03
@jhedberg jhedberg merged commit f231c5f into zephyrproject-rtos:main Aug 8, 2025
28 checks passed
@HaavardRei HaavardRei deleted the l2cap_ident_matching branch August 8, 2025 10:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Bluetooth Host Bluetooth Host (excluding BR/EDR) area: Bluetooth
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants