Re: [PATCH v2 1/2] Bluetooth: L2CAP: handle zero txwin_size in ERTM RFC option

[Luiz Augusto von Dentz]

Hi Michael,

On Tue, Apr 21, 2026 at 9:56 AM Michael Bommarito
<michael.bommarito@xxxxxxxxx> wrote:
>
> Peer-supplied ERTM RFC txwin_size = 0 can still propagate into the
> ERTM transmit-window state, and the same invalid value can be
> introduced locally through L2CAP_OPTIONS. In the request path that zero
> reaches l2cap_seq_list_init(..., 0); in the response path it can shrink
> ack_win to 0 and leave ERTM sequencing in a nonsensical state.
>
> Normalize zero tx window values back to L2CAP_DEFAULT_TX_WINDOW
> wherever they enter the ERTM state machine: local socket options,
> outgoing tx_win setup, incoming config requests, and config-response
> parsing. Also make l2cap_seq_list_free() clear its metadata after kfree
> so an init failure after freeing srej_list cannot be freed a second time
> during later channel teardown.
>
> Fixes: 3c588192b5e5 ("Bluetooth: Add the l2cap_seq_list structure for tracking frames")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Michael Bommarito <michael.bommarito@xxxxxxxxx>
> Assisted-by: Claude:claude-opus-4-7
> ---
> Changes in v2:
> - drop the v1 `l2cap_seq_list_init(size == 0) -> -EINVAL` approach
>   and instead normalize zero tx window values at the socket /
>   request / response inputs
> - clamp the local `L2CAP_OPTIONS` txwin_size = 0 case back to
>   `L2CAP_DEFAULT_TX_WINDOW`
> - make `l2cap_seq_list_free()` clear its metadata after `kfree()` so
>   later teardown cannot trip over a previously freed list
> - split the repeated `CONFIG_RSP` ERTM re-init fix into patch 2
>
>  net/bluetooth/l2cap_core.c | 23 +++++++++++++++++++----
>  net/bluetooth/l2cap_sock.c |  3 +++
>  2 files changed, 22 insertions(+), 4 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index 95c65fece39b..7ffafd117817 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -345,6 +345,10 @@ static int l2cap_seq_list_init(struct l2cap_seq_list *seq_list, u16 size)
>  static inline void l2cap_seq_list_free(struct l2cap_seq_list *seq_list)
>  {
>         kfree(seq_list->list);
> +       seq_list->list = NULL;
> +       seq_list->mask = 0;
> +       seq_list->head = L2CAP_SEQ_LIST_CLEAR;
> +       seq_list->tail = L2CAP_SEQ_LIST_CLEAR;
>  }
>
>  static inline bool l2cap_seq_list_contains(struct l2cap_seq_list *seq_list,
> @@ -3234,8 +3238,15 @@ static void __l2cap_set_ertm_timeouts(struct l2cap_chan *chan,
>         rfc->monitor_timeout = cpu_to_le16(L2CAP_DEFAULT_MONITOR_TO);
>  }
>
> +static inline u16 l2cap_txwin_default(u16 txwin)
> +{
> +       return txwin ? txwin : L2CAP_DEFAULT_TX_WINDOW;
> +}
> +
>  static inline void l2cap_txwin_setup(struct l2cap_chan *chan)
>  {
> +       chan->tx_win = l2cap_txwin_default(chan->tx_win);
> +
>         if (chan->tx_win > L2CAP_DEFAULT_TX_WINDOW &&
>             __l2cap_ews_supported(chan->conn)) {
>                 /* use extended control field */
> @@ -3593,6 +3604,8 @@ static int l2cap_parse_conf_req(struct l2cap_chan *chan, void *data, size_t data
>                         break;
>
>                 case L2CAP_MODE_ERTM:
> +                       rfc.txwin_size = l2cap_txwin_default(rfc.txwin_size);
> +
>                         if (!test_bit(CONF_EWS_RECV, &chan->conf_state))
>                                 chan->remote_tx_win = rfc.txwin_size;
>                         else
> @@ -3715,7 +3728,8 @@ static int l2cap_parse_conf_rsp(struct l2cap_chan *chan, void *rsp, int len,
>                 case L2CAP_CONF_EWS:
>                         if (olen != 2)
>                                 break;
> -                       chan->ack_win = min_t(u16, val, chan->ack_win);
> +                       chan->ack_win = min_t(u16, l2cap_txwin_default(val),
> +                                             chan->ack_win);
>                         l2cap_add_conf_opt(&ptr, L2CAP_CONF_EWS, 2,
>                                            chan->tx_win, endptr - ptr);
>                         break;
> @@ -3756,7 +3770,7 @@ static int l2cap_parse_conf_rsp(struct l2cap_chan *chan, void *rsp, int len,
>                         chan->mps    = le16_to_cpu(rfc.max_pdu_size);
>                         if (!test_bit(FLAG_EXT_CTRL, &chan->flags))
>                                 chan->ack_win = min_t(u16, chan->ack_win,
> -                                                     rfc.txwin_size);
> +                                                     l2cap_txwin_default(rfc.txwin_size));
>
>                         if (test_bit(FLAG_EFS_ENABLE, &chan->flags)) {
>                                 chan->local_msdu = le16_to_cpu(efs.msdu);
> @@ -3970,10 +3984,11 @@ static void l2cap_conf_rfc_get(struct l2cap_chan *chan, void *rsp, int len)
>                 chan->monitor_timeout = le16_to_cpu(rfc.monitor_timeout);
>                 chan->mps = le16_to_cpu(rfc.max_pdu_size);
>                 if (test_bit(FLAG_EXT_CTRL, &chan->flags))
> -                       chan->ack_win = min_t(u16, chan->ack_win, txwin_ext);
> +                       chan->ack_win = min_t(u16, chan->ack_win,
> +                                             l2cap_txwin_default(txwin_ext));
>                 else
>                         chan->ack_win = min_t(u16, chan->ack_win,
> -                                             rfc.txwin_size);
> +                                             l2cap_txwin_default(rfc.txwin_size));
>                 break;
>         case L2CAP_MODE_STREAMING:
>                 chan->mps    = le16_to_cpu(rfc.max_pdu_size);
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 71e8c1b45bce..3b53e967bf40 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -765,6 +765,9 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname,
>                         break;
>                 }
>
> +               if (!opts.txwin_size)
> +                       opts.txwin_size = L2CAP_DEFAULT_TX_WINDOW;
> +
>                 if (!l2cap_valid_mtu(chan, opts.imtu)) {
>                         err = -EINVAL;
>                         break;
> --
> 2.53.0

This seems to be going sideways:

https://sashiko.dev/#/patchset/CABBYNZ%2Bf3pur4cSsanQ1kvv-yORp2E0qmVLt9si_%2BFnnJup4Ng%40mail.gmail.com

Patch 2/2 seems totally broken.

-- 
Luiz Augusto von Dentz