Bluetooth: L2CAP: Fix invalid response to L2CAP_ECRED_RECONF_REQ

This fixes responding with an invalid result caused by checking the
wrong size of CID which should have been (cmd_len - sizeof(*req)) and
on top of it the wrong result was use L2CAP_CR_LE_INVALID_PARAMS which
is invalid/reserved for reconf when running test like L2CAP/ECFC/BI-03-C:

> ACL Data RX: Handle 64 flags 0x02 dlen 14
      LE L2CAP: Enhanced Credit Reconfigure Request (0x19) ident 2 len 6
        MTU: 64
        MPS: 64
        Source CID: 64
< ACL Data TX: Handle 64 flags 0x00 dlen 10
      LE L2CAP: Enhanced Credit Reconfigure Respond (0x1a) ident 2 len 2
!        Result: Reserved (0x000c)
         Result: Reconfiguration failed - one or more Destination CIDs invalid (0x0003)

Fiix L2CAP/ECFC/BI-04-C which expects L2CAP_RECONF_INVALID_MPS (0x0002)
when more than one channel gets its MPS reduced:

> ACL Data RX: Handle 64 flags 0x02 dlen 16
      LE L2CAP: Enhanced Credit Reconfigure Request (0x19) ident 2 len 8
        MTU: 264
        MPS: 99
        Source CID: 64
!       Source CID: 65
< ACL Data TX: Handle 64 flags 0x00 dlen 10
      LE L2CAP: Enhanced Credit Reconfigure Respond (0x1a) ident 2 len 2
!        Result: Reconfiguration successful (0x0000)
         Result: Reconfiguration failed - reduction in size of MPS not allowed for more than one channel at a time (0x0002)

Fix L2CAP/ECFC/BI-05-C when SCID is invalid (85 unconnected):

> ACL Data RX: Handle 64 flags 0x02 dlen 14
      LE L2CAP: Enhanced Credit Reconfigure Request (0x19) ident 2 len 6
        MTU: 65
        MPS: 64
!        Source CID: 85
< ACL Data TX: Handle 64 flags 0x00 dlen 10
      LE L2CAP: Enhanced Credit Reconfigure Respond (0x1a) ident 2 len 2
!        Result: Reconfiguration successful (0x0000)
         Result: Reconfiguration failed - one or more Destination CIDs invalid (0x0003)

Fix L2CAP/ECFC/BI-06-C when MPS < L2CAP_ECRED_MIN_MPS (64):

> ACL Data RX: Handle 64 flags 0x02 dlen 14
      LE L2CAP: Enhanced Credit Reconfigure Request (0x19) ident 2 len 6
        MTU: 672
!       MPS: 63
        Source CID: 64
< ACL Data TX: Handle 64 flags 0x00 dlen 10
      LE L2CAP: Enhanced Credit Reconfigure Respond (0x1a) ident 2 len 2
!       Result: Reconfiguration failed - reduction in size of MPS not allowed for more than one channel at a time (0x0002)
        Result: Reconfiguration failed - other unacceptable parameters (0x0004)

Fix L2CAP/ECFC/BI-07-C when MPS reduced for more than one channel:

> ACL Data RX: Handle 64 flags 0x02 dlen 16
      LE L2CAP: Enhanced Credit Reconfigure Request (0x19) ident 3 len 8
        MTU: 84
!       MPS: 71
        Source CID: 64
!        Source CID: 65
< ACL Data TX: Handle 64 flags 0x00 dlen 10
      LE L2CAP: Enhanced Credit Reconfigure Respond (0x1a) ident 2 len 2
!       Result: Reconfiguration successful (0x0000)
        Result: Reconfiguration failed - reduction in size of MPS not allowed for more than one channel at a time (0x0002)

Link: https://github.com/bluez/bluez/issues/1865
Fixes: 15f02b910562 ("Bluetooth: L2CAP: Add initial code for Enhanced Credit Based Mode")
Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index ec3af01e4db9b504258afe57d99d64490cb7a122..6f9cf7a05986260a73462d9fff3fc17ecdef263b 100644 (file)
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -493,6 +493,8 @@ struct l2cap_ecred_reconf_req {
 #define L2CAP_RECONF_SUCCESS           0x0000
 #define L2CAP_RECONF_INVALID_MTU       0x0001
 #define L2CAP_RECONF_INVALID_MPS       0x0002
+#define L2CAP_RECONF_INVALID_CID       0x0003
+#define L2CAP_RECONF_INVALID_PARAMS    0x0004
 
 struct l2cap_ecred_reconf_rsp {
        __le16 result;

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index b628b0fa39b2109ced82e75a9369661e43ea9ffd..81038458be0cc37173cdae5e7c197dc392bcdf58 100644 (file)
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5310,14 +5310,14 @@ static inline int l2cap_ecred_reconf_req(struct l2cap_conn *conn,
        struct l2cap_ecred_reconf_req *req = (void *) data;
        struct l2cap_ecred_reconf_rsp rsp;
        u16 mtu, mps, result;
-       struct l2cap_chan *chan;
+       struct l2cap_chan *chan[L2CAP_ECRED_MAX_CID] = {};
        int i, num_scid;
 
        if (!enable_ecred)
                return -EINVAL;
 
-       if (cmd_len < sizeof(*req) || cmd_len - sizeof(*req) % sizeof(u16)) {
-               result = L2CAP_CR_LE_INVALID_PARAMS;
+       if (cmd_len < sizeof(*req) || (cmd_len - sizeof(*req)) % sizeof(u16)) {
+               result = L2CAP_RECONF_INVALID_CID;
                goto respond;
        }
 
@@ -5327,42 +5327,69 @@ static inline int l2cap_ecred_reconf_req(struct l2cap_conn *conn,
        BT_DBG("mtu %u mps %u", mtu, mps);
 
        if (mtu < L2CAP_ECRED_MIN_MTU) {
-               result = L2CAP_RECONF_INVALID_MTU;
+               result = L2CAP_RECONF_INVALID_PARAMS;
                goto respond;
        }
 
        if (mps < L2CAP_ECRED_MIN_MPS) {
-               result = L2CAP_RECONF_INVALID_MPS;
+               result = L2CAP_RECONF_INVALID_PARAMS;
                goto respond;
        }
 
        cmd_len -= sizeof(*req);
        num_scid = cmd_len / sizeof(u16);
+
+       if (num_scid > L2CAP_ECRED_MAX_CID) {
+               result = L2CAP_RECONF_INVALID_PARAMS;
+               goto respond;
+       }
+
        result = L2CAP_RECONF_SUCCESS;
 
+       /* Check if each SCID, MTU and MPS are valid */
        for (i = 0; i < num_scid; i++) {
                u16 scid;
 
                scid = __le16_to_cpu(req->scid[i]);
-               if (!scid)
-                       return -EPROTO;
+               if (!scid) {
+                       result = L2CAP_RECONF_INVALID_CID;
+                       goto respond;
+               }
 
-               chan = __l2cap_get_chan_by_dcid(conn, scid);
-               if (!chan)
-                       continue;
+               chan[i] = __l2cap_get_chan_by_dcid(conn, scid);
+               if (!chan[i]) {
+                       result = L2CAP_RECONF_INVALID_CID;
+                       goto respond;
+               }
 
-               /* If the MTU value is decreased for any of the included
-                * channels, then the receiver shall disconnect all
-                * included channels.
+               /* The MTU field shall be greater than or equal to the greatest
+                * current MTU size of these channels.
                 */
-               if (chan->omtu > mtu) {
-                       BT_ERR("chan %p decreased MTU %u -> %u", chan,
-                              chan->omtu, mtu);
+               if (chan[i]->omtu > mtu) {
+                       BT_ERR("chan %p decreased MTU %u -> %u", chan[i],
+                              chan[i]->omtu, mtu);
                        result = L2CAP_RECONF_INVALID_MTU;
+                       goto respond;
                }
 
-               chan->omtu = mtu;
-               chan->remote_mps = mps;
+               /* If more than one channel is being configured, the MPS field
+                * shall be greater than or equal to the current MPS size of
+                * each of these channels. If only one channel is being
+                * configured, the MPS field may be less than the current MPS
+                * of that channel.
+                */
+               if (chan[i]->remote_mps >= mps && i) {
+                       BT_ERR("chan %p decreased MPS %u -> %u", chan[i],
+                              chan[i]->remote_mps, mps);
+                       result = L2CAP_RECONF_INVALID_MPS;
+                       goto respond;
+               }
+       }
+
+       /* Commit the new MTU and MPS values after checking they are valid */
+       for (i = 0; i < num_scid; i++) {
+               chan[i]->omtu = mtu;
+               chan[i]->remote_mps = mps;
        }
 
 respond: