@@ -655,6 +655,7 @@ static inline bool iso_enabled(void)
int mgmt_init(void);
void mgmt_exit(void);
void mgmt_cleanup(struct sock *sk);
+void mgmt_pending_cleanup(struct hci_dev *hdev);
void bt_sock_reclassify_lock(struct sock *sk, int proto);
@@ -546,6 +546,7 @@ struct hci_dev {
struct list_head mesh_pending;
struct list_head mgmt_pending;
+ struct mutex mgmt_lock;
struct list_head reject_list;
struct list_head accept_list;
struct list_head uuids;
@@ -2492,6 +2492,7 @@ struct hci_dev *hci_alloc_dev_priv(int sizeof_priv)
INIT_LIST_HEAD(&hdev->mesh_pending);
INIT_LIST_HEAD(&hdev->mgmt_pending);
+ mutex_init(&hdev->mgmt_lock);
INIT_LIST_HEAD(&hdev->reject_list);
INIT_LIST_HEAD(&hdev->accept_list);
INIT_LIST_HEAD(&hdev->uuids);
@@ -2685,10 +2686,7 @@ void hci_unregister_dev(struct hci_dev *hdev)
hci_dev_unlock(hdev);
}
- /* mgmt_index_removed should take care of emptying the
- * pending list */
- BUG_ON(!list_empty(&hdev->mgmt_pending));
-
+ mgmt_pending_cleanup(hdev);
hci_sock_dev_event(hdev, HCI_DEV_UNREG);
if (hdev->rfkill) {
@@ -1444,25 +1444,30 @@ struct cmd_lookup {
static void settings_rsp(struct mgmt_pending_cmd *cmd, void *data)
{
struct cmd_lookup *match = data;
+ struct sock *sk = cmd->sk;
- send_settings_rsp(cmd->sk, cmd->opcode, match->hdev);
-
- list_del(&cmd->list);
+ sock_hold(sk);
- if (match->sk == NULL) {
- match->sk = cmd->sk;
- sock_hold(match->sk);
- }
+ send_settings_rsp(sk, cmd->opcode, match->hdev);
+ mgmt_pending_remove(cmd);
- mgmt_pending_free(cmd);
+ if (match->sk == NULL)
+ match->sk = sk;
+ else
+ sock_put(sk);
}
static void cmd_status_rsp(struct mgmt_pending_cmd *cmd, void *data)
{
+ struct sock *sk = cmd->sk;
u8 *status = data;
+ sock_hold(sk);
+
mgmt_cmd_status(cmd->sk, cmd->index, cmd->opcode, *status);
mgmt_pending_remove(cmd);
+
+ sock_put(sk);
}
static void cmd_complete_rsp(struct mgmt_pending_cmd *cmd, void *data)
@@ -2598,18 +2603,25 @@ static int mgmt_hci_cmd_sync(struct sock *sk, struct hci_dev *hdev,
static bool pending_eir_or_class(struct hci_dev *hdev)
{
struct mgmt_pending_cmd *cmd;
+ bool found = false;
+
+ rcu_read_lock();
- list_for_each_entry(cmd, &hdev->mgmt_pending, list) {
+ list_for_each_entry_rcu(cmd, &hdev->mgmt_pending, list) {
+ if (atomic_read(&cmd->deleted))
+ continue;
switch (cmd->opcode) {
case MGMT_OP_ADD_UUID:
case MGMT_OP_REMOVE_UUID:
case MGMT_OP_SET_DEV_CLASS:
case MGMT_OP_SET_POWERED:
- return true;
+ found = true;
+ break;
}
}
- return false;
+ rcu_read_unlock();
+ return found;
}
static const u8 bluetooth_base_uuid[] = {
@@ -3401,19 +3413,23 @@ static int set_io_capability(struct sock *sk, struct hci_dev *hdev, void *data,
static struct mgmt_pending_cmd *find_pairing(struct hci_conn *conn)
{
struct hci_dev *hdev = conn->hdev;
- struct mgmt_pending_cmd *cmd;
+ struct mgmt_pending_cmd *tmp, *cmd = NULL;
- list_for_each_entry(cmd, &hdev->mgmt_pending, list) {
- if (cmd->opcode != MGMT_OP_PAIR_DEVICE)
- continue;
+ rcu_read_lock();
- if (cmd->user_data != conn)
+ list_for_each_entry(tmp, &hdev->mgmt_pending, list) {
+ if (atomic_read(&tmp->deleted))
continue;
-
- return cmd;
+ if (tmp->opcode != MGMT_OP_PAIR_DEVICE)
+ continue;
+ if (tmp->user_data != conn)
+ continue;
+ cmd = tmp;
+ break;
}
- return NULL;
+ rcu_read_unlock();
+ return cmd;
}
static int pairing_complete(struct mgmt_pending_cmd *cmd, u8 status)
@@ -10476,3 +10492,14 @@ void mgmt_cleanup(struct sock *sk)
read_unlock(&hci_dev_list_lock);
}
+
+void mgmt_pending_cleanup(struct hci_dev *hdev)
+{
+ struct mgmt_pending_cmd *cmd, *tmp;
+
+ list_for_each_entry_safe(cmd, tmp, &hdev->mgmt_pending, list) {
+ BUG_ON(atomic_read(&cmd->deleted) == 0);
+ list_del_rcu(&cmd->list);
+ mgmt_pending_free(cmd);
+ }
+}
@@ -217,30 +217,45 @@ int mgmt_cmd_complete(struct sock *sk, u16 index, u16 cmd, u8 status,
struct mgmt_pending_cmd *mgmt_pending_find(unsigned short channel, u16 opcode,
struct hci_dev *hdev)
{
- struct mgmt_pending_cmd *cmd;
+ struct mgmt_pending_cmd *tmp, *cmd = NULL;
+
+ rcu_read_lock();
- list_for_each_entry(cmd, &hdev->mgmt_pending, list) {
- if (hci_sock_get_channel(cmd->sk) != channel)
+ list_for_each_entry_rcu(tmp, &hdev->mgmt_pending, list) {
+ if (atomic_read(&tmp->deleted))
continue;
- if (cmd->opcode == opcode)
- return cmd;
+ if (hci_sock_get_channel(tmp->sk) != channel)
+ continue;
+ if (tmp->opcode == opcode) {
+ cmd = tmp;
+ break;
+ }
}
- return NULL;
+ rcu_read_unlock();
+ return cmd;
}
void mgmt_pending_foreach(u16 opcode, struct hci_dev *hdev,
void (*cb)(struct mgmt_pending_cmd *cmd, void *data),
void *data)
{
- struct mgmt_pending_cmd *cmd, *tmp;
+ struct mgmt_pending_cmd *cmd;
- list_for_each_entry_safe(cmd, tmp, &hdev->mgmt_pending, list) {
+ rcu_read_lock();
+
+ list_for_each_entry_rcu(cmd, &hdev->mgmt_pending, list) {
+ if (atomic_read(&cmd->deleted))
+ continue;
if (opcode > 0 && cmd->opcode != opcode)
continue;
+ rcu_read_unlock();
cb(cmd, data);
+ rcu_read_lock();
}
+
+ rcu_read_unlock();
}
struct mgmt_pending_cmd *mgmt_pending_new(struct sock *sk, u16 opcode,
@@ -270,17 +285,34 @@ struct mgmt_pending_cmd *mgmt_pending_new(struct sock *sk, u16 opcode,
return cmd;
}
+static void mgmt_pending_delayed_free(struct rcu_head *rcu)
+{
+ struct mgmt_pending_cmd *cmd =
+ container_of(rcu, struct mgmt_pending_cmd, head);
+ kfree(cmd->param);
+ kfree(cmd);
+}
+
struct mgmt_pending_cmd *mgmt_pending_add(struct sock *sk, u16 opcode,
struct hci_dev *hdev,
void *data, u16 len)
{
- struct mgmt_pending_cmd *cmd;
+ struct mgmt_pending_cmd *cmd, *old, *tmp;
cmd = mgmt_pending_new(sk, opcode, hdev, data, len);
if (!cmd)
return NULL;
- list_add_tail(&cmd->list, &hdev->mgmt_pending);
+ mutex_lock(&hdev->mgmt_lock);
+ list_for_each_entry_safe(old, tmp, &hdev->mgmt_pending, list)
+ if (atomic_read(&old->deleted)) {
+ list_del_rcu(&old->list);
+ sock_put(old->sk);
+ call_rcu(&old->head, mgmt_pending_delayed_free);
+ }
+
+ list_add_tail_rcu(&cmd->list, &hdev->mgmt_pending);
+ mutex_unlock(&hdev->mgmt_lock);
return cmd;
}
@@ -294,8 +326,7 @@ void mgmt_pending_free(struct mgmt_pending_cmd *cmd)
void mgmt_pending_remove(struct mgmt_pending_cmd *cmd)
{
- list_del(&cmd->list);
- mgmt_pending_free(cmd);
+ atomic_set(&cmd->deleted, 1);
}
void mgmt_mesh_foreach(struct hci_dev *hdev,
@@ -32,6 +32,8 @@ struct mgmt_mesh_tx {
struct mgmt_pending_cmd {
struct list_head list;
+ struct rcu_head head;
+ atomic_t deleted;
u16 opcode;
int index;
void *param;
@@ -65,6 +67,7 @@ struct mgmt_pending_cmd *mgmt_pending_new(struct sock *sk, u16 opcode,
void *data, u16 len);
void mgmt_pending_free(struct mgmt_pending_cmd *cmd);
void mgmt_pending_remove(struct mgmt_pending_cmd *cmd);
+void mgmt_pending_cleanup(struct hci_dev *hdev);
void mgmt_mesh_foreach(struct hci_dev *hdev,
void (*cb)(struct mgmt_mesh_tx *mesh_tx, void *data),
void *data, struct sock *sk);
An overall idea is that 'mgmt_pending' of 'struct hci_dev' may be altered only in 'mgmt_pending_add()' under 'mgmt_lock' protection, where processed commands are removed each time when the new command is added. All other users of 'mgmt_pending' (except 'mgmt_pending_cleanup()' where no concurrent accesses are expected) are read-side critical sections running under 'rcu_read_lock()'. (I'm also trying to fix socket UAFs observed when running this code, and most likely these fixes should go to the separate patch). Suggested-by: Luiz Augusto von Dentz <luiz.dentz@gmail.com> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru> --- include/net/bluetooth/bluetooth.h | 1 + include/net/bluetooth/hci_core.h | 1 + net/bluetooth/hci_core.c | 6 +-- net/bluetooth/mgmt.c | 65 ++++++++++++++++++++++--------- net/bluetooth/mgmt_util.c | 55 ++++++++++++++++++++------ net/bluetooth/mgmt_util.h | 3 ++ 6 files changed, 96 insertions(+), 35 deletions(-)