Skip to content

Commit

Permalink
Merge tag 'for-net-2022-04-27' of git://git.kernel.org/pub/scm/linux/…
Browse files Browse the repository at this point in the history 
…kernel/git/bluetooth/bluetooth

Luiz Augusto von Dentz says:

====================
bluetooth pull request for net:

 - Fix regression causing some HCI events to be discarded when they
   shouldn't.

* tag 'for-net-2022-04-27' of git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth:
  Bluetooth: hci_sync: Cleanup hci_conn if it cannot be aborted
  Bluetooth: hci_event: Fix creating hci_conn object on error status
  Bluetooth: hci_event: Fix checking for invalid handle on error status
====================

Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Paolo Abeni <[email protected]>
  • Loading branch information
Paolo Abeni committed Apr 28, 2022
2 parents d2b52ec + 9b3628d commit febb2d2
Show file tree
Hide file tree
Showing 5 changed files with 83 additions and 43 deletions.
1 change: 1 addition & 0 deletions include/net/bluetooth/hci.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,7 @@ enum {
#define HCI_ERROR_CONNECTION_TIMEOUT 0x08
#define HCI_ERROR_REJ_LIMITED_RESOURCES 0x0d
#define HCI_ERROR_REJ_BAD_ADDR 0x0f
#define HCI_ERROR_INVALID_PARAMETERS 0x12
#define HCI_ERROR_REMOTE_USER_TERM 0x13
#define HCI_ERROR_REMOTE_LOW_RESOURCES 0x14
#define HCI_ERROR_REMOTE_POWER_OFF 0x15
Expand Down
2 changes: 1 addition & 1 deletion include/net/bluetooth/hci_core.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1156,7 +1156,7 @@ int hci_conn_switch_role(struct hci_conn *conn, __u8 role);

void hci_conn_enter_active_mode(struct hci_conn *conn, __u8 force_active);

void hci_le_conn_failed(struct hci_conn *conn, u8 status);
void hci_conn_failed(struct hci_conn *conn, u8 status);

/*
* hci_conn_get() and hci_conn_put() are used to control the life-time of an
Expand Down
32 changes: 24 additions & 8 deletions net/bluetooth/hci_conn.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,7 @@ static void le_conn_timeout(struct work_struct *work)
/* Disable LE Advertising */
le_disable_advertising(hdev);
hci_dev_lock(hdev);
hci_le_conn_failed(conn, HCI_ERROR_ADVERTISING_TIMEOUT);
hci_conn_failed(conn, HCI_ERROR_ADVERTISING_TIMEOUT);
hci_dev_unlock(hdev);
return;
}
Expand Down Expand Up @@ -873,7 +873,7 @@ struct hci_dev *hci_get_route(bdaddr_t *dst, bdaddr_t *src, uint8_t src_type)
EXPORT_SYMBOL(hci_get_route);

/* This function requires the caller holds hdev->lock */
void hci_le_conn_failed(struct hci_conn *conn, u8 status)
static void hci_le_conn_failed(struct hci_conn *conn, u8 status)
{
struct hci_dev *hdev = conn->hdev;
struct hci_conn_params *params;
Expand All @@ -886,8 +886,6 @@ void hci_le_conn_failed(struct hci_conn *conn, u8 status)
params->conn = NULL;
}

conn->state = BT_CLOSED;

/* If the status indicates successful cancellation of
* the attempt (i.e. Unknown Connection Id) there's no point of
* notifying failure since we'll go back to keep trying to
Expand All @@ -899,10 +897,6 @@ void hci_le_conn_failed(struct hci_conn *conn, u8 status)
mgmt_connect_failed(hdev, &conn->dst, conn->type,
conn->dst_type, status);

hci_connect_cfm(conn, status);

hci_conn_del(conn);

/* Since we may have temporarily stopped the background scanning in
* favor of connection establishment, we should restart it.
*/
Expand All @@ -914,6 +908,28 @@ void hci_le_conn_failed(struct hci_conn *conn, u8 status)
hci_enable_advertising(hdev);
}

/* This function requires the caller holds hdev->lock */
void hci_conn_failed(struct hci_conn *conn, u8 status)
{
struct hci_dev *hdev = conn->hdev;

bt_dev_dbg(hdev, "status 0x%2.2x", status);

switch (conn->type) {
case LE_LINK:
hci_le_conn_failed(conn, status);
break;
case ACL_LINK:
mgmt_connect_failed(hdev, &conn->dst, conn->type,
conn->dst_type, status);
break;
}

conn->state = BT_CLOSED;
hci_connect_cfm(conn, status);
hci_conn_del(conn);
}

static void create_le_conn_complete(struct hci_dev *hdev, void *data, int err)
{
struct hci_conn *conn = data;
Expand Down
80 changes: 47 additions & 33 deletions net/bluetooth/hci_event.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2834,7 +2834,7 @@ static void hci_cs_le_create_conn(struct hci_dev *hdev, u8 status)
bt_dev_dbg(hdev, "status 0x%2.2x", status);

/* All connection failure handling is taken care of by the
* hci_le_conn_failed function which is triggered by the HCI
* hci_conn_failed function which is triggered by the HCI
* request completion callbacks used for connecting.
*/
if (status)
Expand All @@ -2859,7 +2859,7 @@ static void hci_cs_le_ext_create_conn(struct hci_dev *hdev, u8 status)
bt_dev_dbg(hdev, "status 0x%2.2x", status);

/* All connection failure handling is taken care of by the
* hci_le_conn_failed function which is triggered by the HCI
* hci_conn_failed function which is triggered by the HCI
* request completion callbacks used for connecting.
*/
if (status)
Expand Down Expand Up @@ -3067,18 +3067,20 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data,
{
struct hci_ev_conn_complete *ev = data;
struct hci_conn *conn;
u8 status = ev->status;

if (__le16_to_cpu(ev->handle) > HCI_CONN_HANDLE_MAX) {
bt_dev_err(hdev, "Ignoring HCI_Connection_Complete for invalid handle");
return;
}

bt_dev_dbg(hdev, "status 0x%2.2x", ev->status);
bt_dev_dbg(hdev, "status 0x%2.2x", status);

hci_dev_lock(hdev);

conn = hci_conn_hash_lookup_ba(hdev, ev->link_type, &ev->bdaddr);
if (!conn) {
/* In case of error status and there is no connection pending
* just unlock as there is nothing to cleanup.
*/
if (ev->status)
goto unlock;

/* Connection may not exist if auto-connected. Check the bredr
* allowlist to see if this device is allowed to auto connect.
* If link is an ACL type, create a connection class
Expand Down Expand Up @@ -3122,8 +3124,14 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data,
goto unlock;
}

if (!ev->status) {
if (!status) {
conn->handle = __le16_to_cpu(ev->handle);
if (conn->handle > HCI_CONN_HANDLE_MAX) {
bt_dev_err(hdev, "Invalid handle: 0x%4.4x > 0x%4.4x",
conn->handle, HCI_CONN_HANDLE_MAX);
status = HCI_ERROR_INVALID_PARAMETERS;
goto done;
}

if (conn->type == ACL_LINK) {
conn->state = BT_CONFIG;
Expand Down Expand Up @@ -3164,19 +3172,14 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data,
hci_send_cmd(hdev, HCI_OP_CHANGE_CONN_PTYPE, sizeof(cp),
&cp);
}
} else {
conn->state = BT_CLOSED;
if (conn->type == ACL_LINK)
mgmt_connect_failed(hdev, &conn->dst, conn->type,
conn->dst_type, ev->status);
}

if (conn->type == ACL_LINK)
hci_sco_setup(conn, ev->status);

if (ev->status) {
hci_connect_cfm(conn, ev->status);
hci_conn_del(conn);
done:
if (status) {
hci_conn_failed(conn, status);
} else if (ev->link_type == SCO_LINK) {
switch (conn->setting & SCO_AIRMODE_MASK) {
case SCO_AIRMODE_CVSD:
Expand All @@ -3185,7 +3188,7 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, void *data,
break;
}

hci_connect_cfm(conn, ev->status);
hci_connect_cfm(conn, status);
}

unlock:
Expand Down Expand Up @@ -4676,6 +4679,7 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev, void *data,
{
struct hci_ev_sync_conn_complete *ev = data;
struct hci_conn *conn;
u8 status = ev->status;

switch (ev->link_type) {
case SCO_LINK:
Expand All @@ -4690,12 +4694,7 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev, void *data,
return;
}

if (__le16_to_cpu(ev->handle) > HCI_CONN_HANDLE_MAX) {
bt_dev_err(hdev, "Ignoring HCI_Sync_Conn_Complete for invalid handle");
return;
}

bt_dev_dbg(hdev, "status 0x%2.2x", ev->status);
bt_dev_dbg(hdev, "status 0x%2.2x", status);

hci_dev_lock(hdev);

Expand Down Expand Up @@ -4729,9 +4728,17 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev, void *data,
goto unlock;
}

switch (ev->status) {
switch (status) {
case 0x00:
conn->handle = __le16_to_cpu(ev->handle);
if (conn->handle > HCI_CONN_HANDLE_MAX) {
bt_dev_err(hdev, "Invalid handle: 0x%4.4x > 0x%4.4x",
conn->handle, HCI_CONN_HANDLE_MAX);
status = HCI_ERROR_INVALID_PARAMETERS;
conn->state = BT_CLOSED;
break;
}

conn->state = BT_CONNECTED;
conn->type = ev->link_type;

Expand Down Expand Up @@ -4775,8 +4782,8 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev, void *data,
}
}

hci_connect_cfm(conn, ev->status);
if (ev->status)
hci_connect_cfm(conn, status);
if (status)
hci_conn_del(conn);

unlock:
Expand Down Expand Up @@ -5527,11 +5534,6 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status,
struct smp_irk *irk;
u8 addr_type;

if (handle > HCI_CONN_HANDLE_MAX) {
bt_dev_err(hdev, "Ignoring HCI_LE_Connection_Complete for invalid handle");
return;
}

hci_dev_lock(hdev);

/* All controllers implicitly stop advertising in the event of a
Expand All @@ -5541,6 +5543,12 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status,

conn = hci_lookup_le_connect(hdev);
if (!conn) {
/* In case of error status and there is no connection pending
* just unlock as there is nothing to cleanup.
*/
if (status)
goto unlock;

conn = hci_conn_add(hdev, LE_LINK, bdaddr, role);
if (!conn) {
bt_dev_err(hdev, "no memory for new connection");
Expand Down Expand Up @@ -5603,8 +5611,14 @@ static void le_conn_complete_evt(struct hci_dev *hdev, u8 status,

conn->dst_type = ev_bdaddr_type(hdev, conn->dst_type, NULL);

if (handle > HCI_CONN_HANDLE_MAX) {
bt_dev_err(hdev, "Invalid handle: 0x%4.4x > 0x%4.4x", handle,
HCI_CONN_HANDLE_MAX);
status = HCI_ERROR_INVALID_PARAMETERS;
}

if (status) {
hci_le_conn_failed(conn, status);
hci_conn_failed(conn, status);
goto unlock;
}

Expand Down
11 changes: 10 additions & 1 deletion net/bluetooth/hci_sync.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4408,12 +4408,21 @@ static int hci_reject_conn_sync(struct hci_dev *hdev, struct hci_conn *conn,
static int hci_abort_conn_sync(struct hci_dev *hdev, struct hci_conn *conn,
u8 reason)
{
int err;

switch (conn->state) {
case BT_CONNECTED:
case BT_CONFIG:
return hci_disconnect_sync(hdev, conn, reason);
case BT_CONNECT:
return hci_connect_cancel_sync(hdev, conn);
err = hci_connect_cancel_sync(hdev, conn);
/* Cleanup hci_conn object if it cannot be cancelled as it
* likelly means the controller and host stack are out of sync.
*/
if (err)
hci_conn_failed(conn, err);

return err;
case BT_CONNECT2:
return hci_reject_conn_sync(hdev, conn, reason);
default:
Expand Down

0 comments on commit febb2d2

Please sign in to comment.