lkml.org 
[lkml]   [2019]   [Feb]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[RFC] Bluetooth: Retry configure request if result is L2CAP_CONF_UNKNOWN
Date
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. Store a mask of supported conf option types per connection

2. Parse the body of response L2CAP_CONF_UNKNOWN and adjust
connection's supported conf option types mask

3. Retry configuration step the same way it's done for
L2CAP_CONF_UNACCEPT

Signed-off-by: Andrey Smirnov <andrew.smirnov@gmail.com>
Cc: Pierre-Loup A. Griffais <pgriffais@valvesoftware.com>
Cc: Florian Dollinger <dollinger.florian@gmx.de>
Cc: Marcel Holtmann <marcel@holtmann.org>
Cc: Johan Hedberg <johan.hedberg@gmail.com>
Cc: linux-bluetooth@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
---

Everyone:

I marked this as an RFC, since I don't have a lot of experience with
Bluetooth subsystem and don't have hight degree of confidence about
choices made in this patch. I do, however, thins is is good enough to
start a discussion about the problem.

Thanks,
Andrey Smirnov

include/net/bluetooth/l2cap.h | 1 +
net/bluetooth/l2cap_core.c | 58 ++++++++++++++++++++++++++++++-----
2 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 093aedebdf0c..6898bba5d9a8 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -632,6 +632,7 @@ struct l2cap_conn {
unsigned int mtu;

__u32 feat_mask;
+ __u32 known_options;
__u8 remote_fixed_chan;
__u8 local_fixed_chan;

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index f17e393b43b4..49be98b6de72 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -3243,8 +3243,10 @@ static int l2cap_build_conf_req(struct l2cap_chan *chan, void *data, size_t data
rfc.monitor_timeout = 0;
rfc.max_pdu_size = 0;

- l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
- (unsigned long) &rfc, endptr - ptr);
+ if (chan->conn->known_options & BIT(L2CAP_CONF_RFC)) {
+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
+ (unsigned long)&rfc, endptr - ptr);
+ }
break;

case L2CAP_MODE_ERTM:
@@ -3263,8 +3265,10 @@ static int l2cap_build_conf_req(struct l2cap_chan *chan, void *data, size_t data
rfc.txwin_size = min_t(u16, chan->tx_win,
L2CAP_DEFAULT_TX_WINDOW);

- l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
- (unsigned long) &rfc, endptr - ptr);
+ if (chan->conn->known_options & BIT(L2CAP_CONF_RFC)) {
+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
+ (unsigned long)&rfc, endptr - ptr);
+ }

if (test_bit(FLAG_EFS_ENABLE, &chan->flags))
l2cap_add_opt_efs(&ptr, chan, endptr - ptr);
@@ -3295,8 +3299,10 @@ static int l2cap_build_conf_req(struct l2cap_chan *chan, void *data, size_t data
L2CAP_FCS_SIZE);
rfc.max_pdu_size = cpu_to_le16(size);

- l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
- (unsigned long) &rfc, endptr - ptr);
+ if (chan->conn->known_options & BIT(L2CAP_CONF_RFC)) {
+ l2cap_add_conf_opt(&ptr, L2CAP_CONF_RFC, sizeof(rfc),
+ (unsigned long)&rfc, endptr - ptr);
+ }

if (test_bit(FLAG_EFS_ENABLE, &chan->flags))
l2cap_add_opt_efs(&ptr, chan, endptr - ptr);
@@ -3550,11 +3556,47 @@ static int l2cap_parse_conf_rsp(struct l2cap_chan *chan, void *rsp, int len,
void *endptr = data + size;
int type, olen;
unsigned long val;
+ const bool unknown_options = *result == L2CAP_CONF_UNKNOWN;
struct l2cap_conf_rfc rfc = { .mode = L2CAP_MODE_BASIC };
struct l2cap_conf_efs efs;

BT_DBG("chan %p, rsp %p, len %d, req %p", chan, rsp, len, data);

+ /* throw out any old stored conf requests */
+ *result = L2CAP_CONF_SUCCESS;
+
+ if (unknown_options) {
+ const u8 *option_type = rsp;
+
+ if (!len) {
+ /* If no list of unknown option types is
+ * provided there's nothing for us to do
+ */
+ return -ECONNREFUSED;
+ }
+
+ while (len--) {
+ 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 -ECONNREFUSED;
+ /* Make sure option type is one of the types
+ * supported/used in configure requests
+ */
+ if (*option_type < L2CAP_CONF_MTU ||
+ *option_type > L2CAP_CONF_EWS)
+ return -ECONNREFUSED;
+
+ chan->conn->known_options &= ~BIT(*option_type++);
+ }
+
+ return l2cap_build_conf_req(chan, data, size);
+ }
+
while (len >= L2CAP_CONF_OPT_SIZE) {
len -= l2cap_get_conf_opt(&rsp, &type, &olen, &val);
if (len < 0)
@@ -4240,6 +4282,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn,
}
goto done;

+ case L2CAP_CONF_UNKNOWN:
case L2CAP_CONF_UNACCEPT:
if (chan->num_conf_rsp <= L2CAP_CONF_MAX_CONF_RSP) {
char req[64];
@@ -4249,8 +4292,6 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn,
goto done;
}

- /* throw out any old stored conf requests */
- result = L2CAP_CONF_SUCCESS;
len = l2cap_parse_conf_rsp(chan, rsp->data, len,
req, sizeof(req), &result);
if (len < 0) {
@@ -7067,6 +7108,7 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon)
hcon->l2cap_data = conn;
conn->hcon = hci_conn_get(hcon);
conn->hchan = hchan;
+ conn->known_options = U32_MAX;

BT_DBG("hcon %p conn %p hchan %p", hcon, conn, hchan);

--
2.20.1
\
 
 \ /
  Last update: 2019-02-08 03:59    [W:2.678 / U:0.452 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site