[PATCH 0/3] Fix L2CAP command reject PDUs

[PATCH 0/3] Fix L2CAP command reject PDUs

[johan.hedberg]

Hi,

The L2CAP command reject PDU is required to have some extra data for
"invalid MTU" and "invalid CID" cases. These patches fixes the code to
always return the right kind of PDU.

Johan

----------------------------------------------------------------
Johan Hedberg (3):
      Bluetooth: Fix L2CAP "Command Reject: Invalid CID" response
      Bluetooth: Remove unused command reject mapping for EMSGSIZE
      Bluetooth: Remove useless l2cap_err_to_reason function

 net/bluetooth/l2cap_core.c | 54 +++++++++++++++++++++------------------------
 1 file changed, 25 insertions(+), 29 deletions(-)

[PATCH 1/3] Bluetooth: Fix L2CAP "Command Reject: Invalid CID" response

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

When the reason code in the L2CAP command reject is "invalid CID" there
should be four additional bytes of data in the PDU, namely the source
and destination CIDs (which should be zero if one or both are not
applicable). This patch fixes all occurrences of such errors to return
the right kind of PDU.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/l2cap_core.c | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 72ce21a..7d25ec5 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3971,6 +3971,18 @@ static void l2cap_send_efs_conf_rsp(struct l2cap_chan *chan, void *data,
 					    L2CAP_CONF_SUCCESS, flags), data);
 }
 
+static void cmd_reject_invalid_cid(struct l2cap_conn *conn, u8 ident,
+				   u16 scid, u16 dcid)
+{
+	struct l2cap_cmd_rej_cid rej;
+
+	rej.reason = __constant_cpu_to_le16(L2CAP_REJ_INVALID_CID);
+	rej.scid = __cpu_to_le16(scid);
+	rej.dcid = __cpu_to_le16(dcid);
+
+	l2cap_send_cmd(conn, ident, L2CAP_COMMAND_REJ, sizeof(rej), &rej);
+}
+
 static inline int l2cap_config_req(struct l2cap_conn *conn,
 				   struct l2cap_cmd_hdr *cmd, u16 cmd_len,
 				   u8 *data)
@@ -3990,18 +4002,14 @@ static inline int l2cap_config_req(struct l2cap_conn *conn,
 	BT_DBG("dcid 0x%4.4x flags 0x%2.2x", dcid, flags);
 
 	chan = l2cap_get_chan_by_scid(conn, dcid);
-	if (!chan)
-		return -EBADSLT;
+	if (!chan) {
+		cmd_reject_invalid_cid(conn, cmd->ident, dcid, 0);
+		return 0;
+	}
 
 	if (chan->state != BT_CONFIG && chan->state != BT_CONNECT2) {
-		struct l2cap_cmd_rej_cid rej;
-
-		rej.reason = __constant_cpu_to_le16(L2CAP_REJ_INVALID_CID);
-		rej.scid = cpu_to_le16(chan->scid);
-		rej.dcid = cpu_to_le16(chan->dcid);
-
-		l2cap_send_cmd(conn, cmd->ident, L2CAP_COMMAND_REJ,
-			       sizeof(rej), &rej);
+		cmd_reject_invalid_cid(conn, cmd->ident, chan->scid,
+				       chan->dcid);
 		goto unlock;
 	}
 
@@ -4217,8 +4225,8 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn,
 
 	chan = __l2cap_get_chan_by_scid(conn, dcid);
 	if (!chan) {
-		mutex_unlock(&conn->chan_lock);
-		return -EBADSLT;
+		cmd_reject_invalid_cid(conn, cmd->ident, dcid, scid);
+		return 0;
 	}
 
 	l2cap_chan_lock(chan);
@@ -4447,7 +4455,9 @@ static int l2cap_create_channel_req(struct l2cap_conn *conn,
 						  &conn->hcon->dst);
 		if (!hs_hcon) {
 			hci_dev_put(hdev);
-			return -EBADSLT;
+			cmd_reject_invalid_cid(conn, cmd->ident, chan->scid,
+					       chan->dcid);
+			return 0;
 		}
 
 		BT_DBG("mgr %p bredr_chan %p hs_hcon %p", mgr, chan, hs_hcon);
@@ -5306,8 +5316,6 @@ static inline int l2cap_le_sig_cmd(struct l2cap_conn *conn,
 static __le16 l2cap_err_to_reason(int err)
 {
 	switch (err) {
-	case -EBADSLT:
-		return __constant_cpu_to_le16(L2CAP_REJ_INVALID_CID);
 	case -EMSGSIZE:
 		return __constant_cpu_to_le16(L2CAP_REJ_MTU_EXCEEDED);
 	case -EINVAL:
-- 
1.8.3.1

Re: [PATCH 1/3] Bluetooth: Fix L2CAP "Command Reject: Invalid CID" response

[Anderson Lizardo]

Hi Johan,

On Wed, Oct 16, 2013 at 4:06 AM,  <johan.hedberg@gmail.com> wrote:
> @@ -4217,8 +4225,8 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn,
>
>         chan = __l2cap_get_chan_by_scid(conn, dcid);
>         if (!chan) {
> -               mutex_unlock(&conn->chan_lock);
> -               return -EBADSLT;
> +               cmd_reject_invalid_cid(conn, cmd->ident, dcid, scid);
> +               return 0;
>         }
>
>         l2cap_chan_lock(chan);

Are you sure this mutex_unlock() call should be removed?


Best Regards,
-- 
Anderson Lizardo
Instituto Nokia de Tecnologia - INdT
Manaus - Brazil

Re: [PATCH 1/3] Bluetooth: Fix L2CAP "Command Reject: Invalid CID" response

[Johan Hedberg]

Hi Lizardo,

On Wed, Oct 16, 2013, Anderson Lizardo wrote:
> On Wed, Oct 16, 2013 at 4:06 AM,  <johan.hedberg@gmail.com> wrote:
> > @@ -4217,8 +4225,8 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn,
> >
> >         chan = __l2cap_get_chan_by_scid(conn, dcid);
> >         if (!chan) {
> > -               mutex_unlock(&conn->chan_lock);
> > -               return -EBADSLT;
> > +               cmd_reject_invalid_cid(conn, cmd->ident, dcid, scid);
> > +               return 0;
> >         }
> >
> >         l2cap_chan_lock(chan);
> 
> Are you sure this mutex_unlock() call should be removed?

It was indeed a bug which got fixed before this went upstream. Good that
you spotted it too though.

Johan

[PATCH 2/3] Bluetooth: Remove unused command reject mapping for EMSGSIZE

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

There is no command handler that would return an EMSGSIZE error, so just
remove this mapping from the l2cap_err_to_reason function.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/l2cap_core.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 7d25ec5..5f2720c 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5316,8 +5316,6 @@ static inline int l2cap_le_sig_cmd(struct l2cap_conn *conn,
 static __le16 l2cap_err_to_reason(int err)
 {
 	switch (err) {
-	case -EMSGSIZE:
-		return __constant_cpu_to_le16(L2CAP_REJ_MTU_EXCEEDED);
 	case -EINVAL:
 	case -EPROTO:
 	default:
-- 
1.8.3.1

[PATCH 3/3] Bluetooth: Remove useless l2cap_err_to_reason function

[johan.hedberg]

From: Johan Hedberg <johan.hedberg@intel.com>

Now that the only reason code this function can return is
L2CAP_REJ_NOT_UNDERSTOOD we can just do the necessary assignment without
needing a separate function at all.

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
---
 net/bluetooth/l2cap_core.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 5f2720c..5cbcd4a 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5313,16 +5313,6 @@ static inline int l2cap_le_sig_cmd(struct l2cap_conn *conn,
 	}
 }
 
-static __le16 l2cap_err_to_reason(int err)
-{
-	switch (err) {
-	case -EINVAL:
-	case -EPROTO:
-	default:
-		return __constant_cpu_to_le16(L2CAP_REJ_NOT_UNDERSTOOD);
-	}
-}
-
 static inline void l2cap_le_sig_channel(struct l2cap_conn *conn,
 					struct sk_buff *skb)
 {
@@ -5355,7 +5345,7 @@ static inline void l2cap_le_sig_channel(struct l2cap_conn *conn,
 
 		BT_ERR("Wrong link type (%d)", err);
 
-		rej.reason = l2cap_err_to_reason(err);
+		rej.reason = __constant_cpu_to_le16(L2CAP_REJ_NOT_UNDERSTOOD);
 		l2cap_send_cmd(conn, cmd->ident, L2CAP_COMMAND_REJ,
 			       sizeof(rej), &rej);
 	}
@@ -5400,7 +5390,7 @@ static inline void l2cap_sig_channel(struct l2cap_conn *conn,
 
 			BT_ERR("Wrong link type (%d)", err);
 
-			rej.reason = l2cap_err_to_reason(err);
+			rej.reason = __constant_cpu_to_le16(L2CAP_REJ_NOT_UNDERSTOOD);
 			l2cap_send_cmd(conn, cmd.ident, L2CAP_COMMAND_REJ,
 				       sizeof(rej), &rej);
 		}
-- 
1.8.3.1

Re: [PATCH 0/3] Fix L2CAP command reject PDUs

[Marcel Holtmann]

Hi Johan,

> The L2CAP command reject PDU is required to have some extra data for
> "invalid MTU" and "invalid CID" cases. These patches fixes the code to
> always return the right kind of PDU.
> 
> Johan
> 
> ----------------------------------------------------------------
> Johan Hedberg (3):
>      Bluetooth: Fix L2CAP "Command Reject: Invalid CID" response
>      Bluetooth: Remove unused command reject mapping for EMSGSIZE
>      Bluetooth: Remove useless l2cap_err_to_reason function
> 
> net/bluetooth/l2cap_core.c | 54 +++++++++++++++++++++------------------------
> 1 file changed, 25 insertions(+), 29 deletions(-)

all 3 patches have been applied to bluetooth-next tree.

Regards

Marcel