[RESEND PATCH v2] Bluetooth: Retry configure request if result is L2CAP_CONF_UNKNOWN
Due to:
* Current implementation of l2cap_config_rsp() dropping BT
connection if sender of configuration response replied with unknown
option failure (Result=0x0003/L2CAP_CONF_UNKNOWN)
* Current implementation of l2cap_build_conf_req() adding
L2CAP_CONF_RFC(0x04) option to initial configure request sent by
the Linux host.
devices that do no recongninze L2CAP_CONF_RFC, such as Xbox One S
controllers, will get stuck in endless connect -> configure ->
disconnect loop, never connect and be generaly unusable.
To avoid this problem add code to do the following:
1. Parse the body of response L2CAP_CONF_UNKNOWN and, in case of
unsupported option being RFC, clear L2CAP_FEAT_ERTM and
L2CAP_FEAT_STREAMING from connection's feature mask (in order to
prevent RFC option from being added going forward)
2. Retry configuration step the same way it's done for
L2CAP_CONF_UNACCEPT
Signed-off-by: Andrey Smirnov
Cc: Pierre-Loup A. Griffais
Cc: Florian Dollinger
Cc: Marcel Holtmann
Cc: Johan Hedberg
Cc: linux-blueto...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
Changes since [v1]:
- Patch simplified to simply clear L2CAP_FEAT_ERTM |
L2CAP_FEAT_STREAMING from feat_mask when device flags RFC options
as unknown
[v1] lore.kernel.org/r/20190208025828.30901-1-andrew.smir...@gmail.com
net/bluetooth/l2cap_core.c | 58 ++
1 file changed, 58 insertions(+)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index dfc1edb168b7..77b65870b064 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4216,6 +4216,49 @@ static inline int l2cap_config_req(struct l2cap_conn
*conn,
return err;
}
+static inline int l2cap_config_rsp_unknown(struct l2cap_conn *conn,
+ struct l2cap_chan *chan,
+ const u8 *data,
+ int len)
+{
+ char req[64];
+
+ if (!len || len > sizeof(req) - sizeof(struct l2cap_conf_req))
+ return -ECONNRESET;
+
+ while (len--) {
+ const u8 option_type = *data++;
+
+ BT_DBG("chan %p, unknown option type: %u", chan, option_type);
+
+ /* "...Hints shall not be included in the Response and
+* shall not be the sole cause for rejecting the
+* Request.."
+*/
+ if (option_type & L2CAP_CONF_HINT)
+ return -ECONNRESET;
+
+ switch (option_type) {
+ case L2CAP_CONF_RFC:
+ /* Clearing the following feature should
+* prevent RFC option from being added next
+* connection attempt
+*/
+ conn->feat_mask &= ~(L2CAP_FEAT_ERTM |
+L2CAP_FEAT_STREAMING);
+ break;
+ default:
+ return -ECONNRESET;
+ }
+ }
+
+ len = l2cap_build_conf_req(chan, req, sizeof(req));
+ l2cap_send_cmd(conn, l2cap_get_ident(conn), L2CAP_CONF_REQ, len, req);
+ chan->num_conf_req++;
+
+ return 0;
+}
+
static inline int l2cap_config_rsp(struct l2cap_conn *conn,
struct l2cap_cmd_hdr *cmd, u16 cmd_len,
u8 *data)
@@ -4271,6 +4314,21 @@ static inline int l2cap_config_rsp(struct l2cap_conn
*conn,
}
goto done;
+ case L2CAP_CONF_UNKNOWN:
+ if (chan->num_conf_rsp <= L2CAP_CONF_MAX_CONF_RSP) {
+ if (l2cap_config_rsp_unknown(conn, chan, rsp->data,
+len) < 0) {
+ l2cap_send_disconn_req(chan, ECONNRESET);
+ goto done;
+ }
+ break;
+ }
+ /* Once, chan->num_conf_rsp goes above
+* L2CAP_CONF_MAX_CONF_RSP we want to go down all the
+* way to default label (just like L2CAP_CONF_UNACCEPT
+* below)
+*/
+ /* fall through */
case L2CAP_CONF_UNACCEPT:
if (chan->num_conf_rsp <= L2CAP_CONF_MAX_CONF_RSP) {
char req[64];
--
2.21.0
Re: [PATCH v2] Bluetooth: Retry configure request if result is L2CAP_CONF_UNKNOWN
On Tue, May 21, 2019 at 6:31 PM Andrey Smirnov wrote: > > Due to: > > * Current implementation of l2cap_config_rsp() dropping BT >connection if sender of configuration response replied with unknown >option failure (Result=0x0003/L2CAP_CONF_UNKNOWN) > > * Current implementation of l2cap_build_conf_req() adding >L2CAP_CONF_RFC(0x04) option to initial configure request sent by >the Linux host. > > devices that do no recongninze L2CAP_CONF_RFC, such as Xbox One S > controllers, will get stuck in endless connect -> configure -> > disconnect loop, never connect and be generaly unusable. > > To avoid this problem add code to do the following: > > 1. Parse the body of response L2CAP_CONF_UNKNOWN and, in case of > unsupported option being RFC, clear L2CAP_FEAT_ERTM and > L2CAP_FEAT_STREAMING from connection's feature mask (in order to > prevent RFC option from being added going forward) > > 2. Retry configuration step the same way it's done for > L2CAP_CONF_UNACCEPT > > Signed-off-by: Andrey Smirnov > Cc: Pierre-Loup A. Griffais > Cc: Florian Dollinger > Cc: Marcel Holtmann > Cc: Johan Hedberg > Cc: linux-blueto...@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > --- > > Changes since [v1]: > >- Patch simplified to simply clear L2CAP_FEAT_ERTM | > L2CAP_FEAT_STREAMING from feat_mask when device flags RFC options > as unknown > > [v1] lore.kernel.org/r/20190208025828.30901-1-andrew.smir...@gmail.com > Pinging the status of this. Marcel, do you have any feedback on v2? Thanks, Andrey Smirnov
[PATCH v2] Bluetooth: Retry configure request if result is L2CAP_CONF_UNKNOWN
Due to:
* Current implementation of l2cap_config_rsp() dropping BT
connection if sender of configuration response replied with unknown
option failure (Result=0x0003/L2CAP_CONF_UNKNOWN)
* Current implementation of l2cap_build_conf_req() adding
L2CAP_CONF_RFC(0x04) option to initial configure request sent by
the Linux host.
devices that do no recongninze L2CAP_CONF_RFC, such as Xbox One S
controllers, will get stuck in endless connect -> configure ->
disconnect loop, never connect and be generaly unusable.
To avoid this problem add code to do the following:
1. Parse the body of response L2CAP_CONF_UNKNOWN and, in case of
unsupported option being RFC, clear L2CAP_FEAT_ERTM and
L2CAP_FEAT_STREAMING from connection's feature mask (in order to
prevent RFC option from being added going forward)
2. Retry configuration step the same way it's done for
L2CAP_CONF_UNACCEPT
Signed-off-by: Andrey Smirnov
Cc: Pierre-Loup A. Griffais
Cc: Florian Dollinger
Cc: Marcel Holtmann
Cc: Johan Hedberg
Cc: linux-blueto...@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---
Changes since [v1]:
- Patch simplified to simply clear L2CAP_FEAT_ERTM |
L2CAP_FEAT_STREAMING from feat_mask when device flags RFC options
as unknown
[v1] lore.kernel.org/r/20190208025828.30901-1-andrew.smir...@gmail.com
net/bluetooth/l2cap_core.c | 59 ++
1 file changed, 59 insertions(+)
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index b53acd6c9a3d..d5d682679128 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -4185,6 +4185,50 @@ static inline int l2cap_config_req(struct l2cap_conn
*conn,
return err;
}
+static inline int l2cap_config_rsp_unknown(struct l2cap_conn *conn,
+ struct l2cap_chan *chan,
+ const u8 *data,
+ int len)
+{
+ int o;
+ char req[64];
+
+ if (!len || len > sizeof(req) - sizeof(struct l2cap_conf_req))
+ return -ECONNRESET;
+
+ while (len--) {
+ const u8 option_type = *data++;
+
+ BT_DBG("chan %p, unknown option type: %u", chan, option_type);
+
+ /* "...Hints shall not be included in the Response and
+* shall not be the sole cause for rejecting the
+* Request.."
+*/
+ if (option_type & L2CAP_CONF_HINT)
+ return -ECONNRESET;
+
+ switch (option_type) {
+ case L2CAP_CONF_RFC:
+ /* Clearing the following feature should
+* prevent RFC option from being added next
+* connection attempt
+*/
+ conn->feat_mask &= ~(L2CAP_FEAT_ERTM |
+L2CAP_FEAT_STREAMING);
+ break;
+ default:
+ return -ECONNRESET;
+ }
+ }
+
+ len = l2cap_build_conf_req(chan, req, sizeof(req));
+ l2cap_send_cmd(conn, l2cap_get_ident(conn), L2CAP_CONF_REQ, len, req);
+ chan->num_conf_req++;
+
+ return 0;
+}
+
static inline int l2cap_config_rsp(struct l2cap_conn *conn,
struct l2cap_cmd_hdr *cmd, u16 cmd_len,
u8 *data)
@@ -4240,6 +4284,21 @@ static inline int l2cap_config_rsp(struct l2cap_conn
*conn,
}
goto done;
+ case L2CAP_CONF_UNKNOWN:
+ if (chan->num_conf_rsp <= L2CAP_CONF_MAX_CONF_RSP) {
+ if (l2cap_config_rsp_unknown(conn, chan, rsp->data,
+len) < 0) {
+ l2cap_send_disconn_req(chan, ECONNRESET);
+ goto done;
+ }
+ break;
+ }
+ /* Once, chan->num_conf_rsp goes above
+* L2CAP_CONF_MAX_CONF_RSP we want to go down all the
+* way to default label (just like L2CAP_CONF_UNACCEPT
+* below)
+*/
+ /* fall through */
case L2CAP_CONF_UNACCEPT:
if (chan->num_conf_rsp <= L2CAP_CONF_MAX_CONF_RSP) {
char req[64];
--
2.21.0
