Re: [PATCH v2] Bluetooth: L2CAP: reject BR/EDR signaling packets over MTUsig

[Luiz Augusto von Dentz]

Hi Michael,

On Wed, May 20, 2026 at 9:50 AM Michael Bommarito
<michael.bommarito@xxxxxxxxx> wrote:
>
> net/bluetooth/l2cap_core.c:l2cap_sig_channel() accepts BR/EDR
> signaling packets up to the channel MTU and dispatches each command
> without enforcing the signaling MTU (MTUsig). A Bluetooth BR/EDR peer
> within radio range can send a fixed-channel CID 0x0001 packet that is
> larger than MTUsig and contains many L2CAP_ECHO_REQ commands before
> pairing.
>
> In a real-radio stock-kernel run, one 681-byte signaling
> packet containing 168 zero-length ECHO_REQ commands made the target
> transmit 168 ECHO_RSP frames over about 220 ms.
>
> Define Linux's BR/EDR signaling MTU as the spec minimum of 48 bytes and
> reject larger signaling packets before dispatching their commands. When
> the over-MTUsig packet contains a request command, send one
> L2CAP_COMMAND_REJECT_RSP with L2CAP_REJ_MTU_EXCEEDED and the first
> request identifier; packets for which no valid request command is found
> are dropped.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Suggested-by: Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx>
> Link: https://lore.kernel.org/r/20260518002800.1361430-1-michael.bommarito@xxxxxxxxx
> Assisted-by: Claude:claude-opus-4-7
> Assisted-by: Codex:gpt-5-5-xhigh
> Signed-off-by: Michael Bommarito <michael.bommarito@xxxxxxxxx>
> ---
> I reproduced the stock behavior with a real-radio BR/EDR ACL link and a
> harness that sends a single fixed-channel signaling packet containing
> packed zero-length ECHO_REQ commands. The patched code builds for
> net/bluetooth/l2cap_core.o on x86_64 defconfig. There are no in-tree
> Bluetooth selftests that reference l2cap_sig_channel(), L2CAP_SIG_MTU,
> or L2CAP_ECHO_REQ.
>
> The unrestricted BR/EDR signaling parser and ECHO_REQ response path both
> trace to the initial git import; no later introducing commit is
> available for a Fixes tag.
>
> Changes in v2:
> - Replace the per-PDU echo-count cap with the MTUsig direction from
>   review.
> - Reject the whole over-MTUsig signaling packet with one
>   L2CAP_REJ_MTU_EXCEEDED command reject.
> - Add L2CAP_SIG_MTU and drop over-MTUsig packets when no valid request
>   command identifier is found.
>
> v1: https://lore.kernel.org/r/20260518002800.1361430-1-michael.bommarito@xxxxxxxxx
> ---
>  include/net/bluetooth/l2cap.h |  1 +
>  net/bluetooth/l2cap_core.c    | 60 +++++++++++++++++++++++++++++++++++
>  2 files changed, 61 insertions(+)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index 5172afee54943..e0a1f2293679a 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -33,6 +33,7 @@
>  /* L2CAP defaults */
>  #define L2CAP_DEFAULT_MTU              672
>  #define L2CAP_DEFAULT_MIN_MTU          48
> +#define L2CAP_SIG_MTU                  48      /* BR/EDR signaling MTU */
>  #define L2CAP_DEFAULT_FLUSH_TO         0xFFFF
>  #define L2CAP_EFS_DEFAULT_FLUSH_TO     0xFFFFFFFF
>  #define L2CAP_DEFAULT_TX_WINDOW                63
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 77dec104a9c36..5417e3cb0636d 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -5626,6 +5626,55 @@ static inline void l2cap_sig_send_rej(struct l2cap_conn *conn, u16 ident)
>         l2cap_send_cmd(conn, ident, L2CAP_COMMAND_REJ, sizeof(rej), &rej);
>  }
>
> +static bool l2cap_sig_cmd_is_req(u8 code)
> +{
> +       switch (code) {
> +       case L2CAP_CONN_REQ:
> +       case L2CAP_CONF_REQ:
> +       case L2CAP_DISCONN_REQ:
> +       case L2CAP_ECHO_REQ:
> +       case L2CAP_INFO_REQ:
> +       case L2CAP_CONN_PARAM_UPDATE_REQ:
> +       case L2CAP_LE_CONN_REQ:
> +       case L2CAP_ECRED_CONN_REQ:
> +       case L2CAP_ECRED_RECONF_REQ:
> +               return true;
> +       }
> +
> +       return false;
> +}
> +
> +static u8 l2cap_sig_first_req_ident(const struct sk_buff *skb)
> +{
> +       const u8 *data = skb->data;
> +       unsigned int len = skb->len;
> +
> +       while (len >= L2CAP_CMD_HDR_SIZE) {
> +               const struct l2cap_cmd_hdr *cmd = (const void *)data;
> +               u16 cmd_len = le16_to_cpu(cmd->len);
> +
> +               if (cmd->ident && l2cap_sig_cmd_is_req(cmd->code))
> +                       return cmd->ident;
> +
> +               if (cmd_len > len - L2CAP_CMD_HDR_SIZE)
> +                       break;
> +
> +               data += L2CAP_CMD_HDR_SIZE + cmd_len;
> +               len -= L2CAP_CMD_HDR_SIZE + cmd_len;
> +       }

Weird, does the AI come up with this? The id is actually _not_
important because the error code will essentially indicate that the
entire packet was rejected. Therefore, it doesn't matter if the id is
for a request or a response, it still needs rejection if it exceeds
the MTU, so this seems overengineered.

> +       return 0;
> +}
> +
> +static inline void l2cap_sig_send_mtu_rej(struct l2cap_conn *conn, u8 ident)
> +{
> +       struct l2cap_cmd_rej_mtu rej;
> +
> +       rej.reason = cpu_to_le16(L2CAP_REJ_MTU_EXCEEDED);
> +       rej.max_mtu = cpu_to_le16(L2CAP_SIG_MTU);
> +       l2cap_send_cmd(conn, ident, L2CAP_COMMAND_REJ, sizeof(rej), &rej);
> +}
> +
>  static inline void l2cap_sig_channel(struct l2cap_conn *conn,
>                                      struct sk_buff *skb)
>  {
> @@ -5638,6 +5687,17 @@ static inline void l2cap_sig_channel(struct l2cap_conn *conn,
>         if (hcon->type != ACL_LINK)
>                 goto drop;
>
> +       if (skb->len > L2CAP_SIG_MTU) {
> +               u8 ident = l2cap_sig_first_req_ident(skb);
> +
> +               BT_DBG("signaling packet exceeds MTU");
> +
> +               if (ident)
> +                       l2cap_sig_send_mtu_rej(conn, ident);
> +
> +               goto drop;
> +       }
> +
>         while (skb->len >= L2CAP_CMD_HDR_SIZE) {
>                 u16 len;
>
> --
> 2.53.0
>


-- 
Luiz Augusto von Dentz