Hi Marcel,
On Sat, Mar 13, 2021 at 3:02 AM Marcel Holtmann <marcel@xxxxxxxxxxxx> wrote:
>
> Hi Luiz,
>
> > When receiving L2CAP_CREDIT_BASED_CONNECTION_REQ the remote may request
> > more channels than allowed by the spec (10 octecs = 5 CIDs) so this
> > truncates the response allowing it to create only the maximum allowed.
>
> so what does the spec say the behavior should be? Truncate or actually respond with an error?
The spec is not very clear about this, well except by saying:
'The Source CID is an array of up to 5 two-octet values and represents the
channel endpoints on the device sending the request.'
So I guess responding with an error would still conform to the above
statement so we would just strictly follow the maximum number of
channels allowed.
> > Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@xxxxxxxxx>
> > ---
> > include/net/bluetooth/l2cap.h | 1 +
> > net/bluetooth/l2cap_core.c | 4 ++--
> > 2 files changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> > index 61800a7b6192..3c4f550e5a8b 100644
> > --- a/include/net/bluetooth/l2cap.h
> > +++ b/include/net/bluetooth/l2cap.h
> > @@ -494,6 +494,7 @@ struct l2cap_le_credits {
> >
> > #define L2CAP_ECRED_MIN_MTU 64
> > #define L2CAP_ECRED_MIN_MPS 64
> > +#define L2CAP_ECRED_MAX_CID 5
> >
> > struct l2cap_ecred_conn_req {
> > __le16 psm;
> > diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> > index 72c2f5226d67..6325d4a89b31 100644
> > --- a/net/bluetooth/l2cap_core.c
> > +++ b/net/bluetooth/l2cap_core.c
> > @@ -5921,7 +5921,7 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
> > struct l2cap_ecred_conn_req *req = (void *) data;
> > struct {
> > struct l2cap_ecred_conn_rsp rsp;
> > - __le16 dcid[5];
> > + __le16 dcid[L2CAP_ECRED_MAX_CID];
> > } __packed pdu;
> > struct l2cap_chan *chan, *pchan;
> > u16 mtu, mps;
> > @@ -5973,7 +5973,7 @@ static inline int l2cap_ecred_conn_req(struct l2cap_conn *conn,
> > cmd_len -= sizeof(*req);
> > num_scid = cmd_len / sizeof(u16);
> >
> > - for (i = 0; i < num_scid; i++) {
> > + for (i = 0; i < num_scid && i < ARRAY_SIZE(pdu.dcid); i++) {
> > u16 scid = __le16_to_cpu(req->scid[i]);
> >
> > BT_DBG("scid[%d] 0x%4.4x", i, scid);
>
> Is this really a good idea? I would prefer if we check the size first and then just respond with an error.
Right, I will change it to just fail with L2CAP_CR_LE_INVALID_PARAMS instead.
> Regards
>
> Marcel
>
--
Luiz Augusto von Dentz
