[PATCH v2 0/2] Bluetooth: L2CAP: fix zero txwin_size handling and repeated CONFIG_RSP re-init

Michael Bommarito posted 2 patches 1 month, 3 weeks ago
Only 0 patches received!
net/bluetooth/l2cap_core.c | 39 +++++++++++++++++++++++++++-----------
net/bluetooth/l2cap_sock.c |  3 +++
2 files changed, 31 insertions(+), 11 deletions(-)
[PATCH v2 0/2] Bluetooth: L2CAP: fix zero txwin_size handling and repeated CONFIG_RSP re-init
Posted by Michael Bommarito 1 month, 3 weeks ago
Hi Luiz,

Thanks for the review.

v1 mixed the original zero-txwin fix with a defensive
l2cap_seq_list_init(size == 0) error return. Re-checking that path
made it clear the reroll should be split:

  1. keep the original report focused on zero txwin_size, but fix it by
     normalizing zero window values at the actual inputs to the ERTM
     state machine and by making sequence-list teardown idempotent,
     rather than by introducing a new init-time failure path

  2. handle the separate repeated-CONFIG_RSP ERTM re-init bug in its
     own patch, matching the BT_CONNECTED guard that commit 25f420a0d4cf
     already added on the CONFIG_REQ side

While auditing the Sashiko comments, I also checked the claimed
CONF_EWS_RECV bypass. I did not carry that theory into v2: in this tree
CONF_EWS_RECV is declared but never set, so the concrete zero-window
paths are the plain RFC option, the local L2CAP_OPTIONS socket setting,
and the CONFIG_RSP / conf_rfc_get negotiation state.

To make sure the reroll covered all the zero-entry sites (and only the
zero-entry sites), I enumerated every assignment to tx_win / ack_win /
remote_tx_win / txwin_size in net/bluetooth/l2cap_core.c and confirmed
patch 1 normalizes the four reachable input paths -- the local
L2CAP_OPTIONS setsockopt, l2cap_txwin_setup() on the outgoing RFC, the
L2CAP_MODE_ERTM branch of l2cap_parse_conf_req(), and the EWS / RFC
branches of l2cap_parse_conf_rsp() and l2cap_conf_rfc_get(). The only
other writers are the channel-create defaults in l2cap_chan_create()
(hard-coded to L2CAP_DEFAULT_TX_WINDOW) and the outgoing rfc.txwin_size
= 0 assignments in L2CAP_MODE_BASIC / L2CAP_MODE_STREAMING branches,
where a zero is spec-correct and never feeds ack_win.

l2cap_seq_list_free()'s callers are l2cap_chan_del() (channel teardown)
and l2cap_ertm_init()'s error path; clearing the list metadata after
kfree() is safe for both and is what makes the partially-initialized
re-entry in the failure path no longer a double-free source.

For patch 2, the BT_CONNECTED guard added to l2cap_config_rsp() mirrors
the existing one in l2cap_config_req() (lines 4339..4348 in this tree)
so both sides of the CONFIG exchange now refuse to re-enter
l2cap_ertm_init() once the channel is already connected. I did not
pin down a single introducing commit for the CONFIG_RSP side during
this pass, so patch 2 keeps Cc: stable@ without a speculative Fixes:
tag; happy to add one if you'd prefer a specific reference.

Changes since v1
----------------

  - Split the reroll into two patches: the zero-txwin fix and the
    separate repeated-CONFIG_RSP ERTM re-init fix.
  - Dropped the v1 l2cap_seq_list_init(size == 0) -> -EINVAL defence and
    instead normalize zero tx window values where they enter negotiated
    ERTM state.
  - Clamp the local L2CAP_OPTIONS txwin_size = 0 case back to
    L2CAP_DEFAULT_TX_WINDOW.
  - Normalize zero tx window values seen during CONFIG_RSP / RFC parsing
    so ack_win does not collapse to 0.
  - Make l2cap_seq_list_free() clear list metadata after kfree so later
    teardown cannot trip over a previously freed list.

Michael Bommarito (2):
  Bluetooth: L2CAP: handle zero txwin_size in ERTM RFC option
  Bluetooth: L2CAP: skip ERTM re-init on repeated CONFIG_RSP

 net/bluetooth/l2cap_core.c | 39 +++++++++++++++++++++++++++-----------
 net/bluetooth/l2cap_sock.c |  3 +++
 2 files changed, 31 insertions(+), 11 deletions(-)

-- 
2.53.0