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

[Luiz Augusto von Dentz]

Hi Michael,

On Wed, May 20, 2026 at 8:06 PM 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.
>
> Impact: a Bluetooth BR/EDR peer within radio range, before pairing, can
> force 168 ECHO_RSP frames from one 681-byte fixed-channel signaling
> packet containing packed ECHO_REQ commands.
>
> Define Linux's BR/EDR signaling MTU as the spec minimum of 48 bytes and
> reject any larger signaling packet with one L2CAP_COMMAND_REJECT_RSP
> carrying L2CAP_REJ_MTU_EXCEEDED before any command is dispatched.
>
> The Bluetooth Core spec wording for MTUExceeded says the reject
> identifier shall match the first request command in the packet, and
> that packets containing only responses shall be silently discarded.
> Linux intentionally deviates from that prescription: silently
> discarding desynchronizes the peer because the remote stack never
> learns its responses were dropped, and locating the first request
> command requires walking command headers past MTUsig, i.e. processing
> bytes from a packet we have already decided is too large to process.
> We therefore always emit one reject and use the identifier from the
> first command header (a single fixed-offset byte read), falling back
> to zero when the packet is too short to carry a header at all.
>
> 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.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Suggested-by: Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx>
> Link: https://lore.kernel.org/r/20260518002800.1361430-1-michael.bommarito@xxxxxxxxx
> Link: https://lore.kernel.org/r/20260520135034.1060859-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, and confirmed on a patched kernel
> that the same packet now produces one L2CAP_REJ_MTU_EXCEEDED command
> reject and zero ECHO_RSP frames. The patched code builds for
> net/bluetooth/l2cap_core.o on x86_64 defconfig with W=1. There are no
> in-tree Bluetooth selftests that reference l2cap_sig_channel(),
> L2CAP_SIG_MTU, or L2CAP_ECHO_REQ.
>
> Changes in v3:
> - Drop l2cap_sig_cmd_is_req() and l2cap_sig_first_req_ident(); the
>   reject is now unconditional and uses only the first command
>   header's identifier byte at a fixed offset. Per Luiz, the spec's
>   "match the first request command identifier" rule would require
>   parsing past MTUsig, and the spec's "silently discard if only
>   responses" rule desynchronizes the peer.
> - Replace the v2 walk with a verbose comment quoting the relevant
>   Bluetooth Core section and documenting why Linux deviates.
>
> 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
> v2: https://lore.kernel.org/r/20260520135034.1060859-1-michael.bommarito@xxxxxxxxx
> ---
>  include/net/bluetooth/l2cap.h |  1 +
>  net/bluetooth/l2cap_core.c    | 47 +++++++++++++++++++++++++++++++++++
>  2 files changed, 48 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 7701528f11677..0b1e062057695 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -5618,6 +5618,15 @@ 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 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)
>  {
> @@ -5630,6 +5639,44 @@ static inline void l2cap_sig_channel(struct l2cap_conn *conn,
>         if (hcon->type != ACL_LINK)
>                 goto drop;
>
> +       /*
> +        * Bluetooth Core v5.4, Vol 3, Part A, Section 4: the BR/EDR
> +        * signaling channel has a fixed signaling MTU (MTUsig) whose
> +        * minimum and default is 48 octets.  Section 4.1 says that on
> +        * an MTUExceeded command reject the identifier "shall match
> +        * the first request command in the L2CAP packet" and that
> +        * packets containing only response commands "shall be
> +        * silently discarded".
> +        *
> +        * Linux intentionally deviates from that prescription:
> +        *
> +        *   1. Silently discarding desynchronizes the peer.  The
> +        *      remote stack never learns its responses were dropped,
> +        *      so any state machine waiting on a paired response
> +        *      stalls until its own timer fires.
> +        *
> +        *   2. Locating "the first request command" requires walking
> +        *      command headers past MTUsig, i.e. processing bytes
> +        *      from a packet we have already decided is too large to
> +        *      process.
> +        *
> +        * Reject every over-MTUsig signaling packet with one
> +        * L2CAP_REJ_MTU_EXCEEDED command reject.  The reject's
> +        * reason field is what tells the peer that the whole packet
> +        * was discarded; the identifier value is informational, so
> +        * we use the identifier from the first command header (a
> +        * single fixed-offset byte read) or zero when the packet is
> +        * too short to carry even one header.
> +        */
> +       if (skb->len > L2CAP_SIG_MTU) {
> +               u8 ident = (skb->len >= L2CAP_CMD_HDR_SIZE) ?
> +                          skb->data[1] : 0;

Checking L2CAP_CMD_HDR_SIZE after L2CAP_SIG_MTU seems unnecessary, the
latter should always be large enough to accommodate a header.

> +
> +               BT_DBG("signaling packet exceeds MTU");

I'd make it print skb->len and L2CAP_SIG_MTU e.g. %u > %u, skb->len,
L2CAP_SIG_MTU.

> +               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