From bed71748346ae0807c7f7a2913965508dbd61403 Mon Sep 17 00:00:00 2001 From: Andre Guedes Date: Wed, 30 Jan 2013 11:50:56 -0300 Subject: Bluetooth: Rename hci_acl_disconn As hci_acl_disconn function basically sends the HCI Disconnect Command and it is used to disconnect ACL, SCO and LE links, renaming it to hci_disconnect is more suitable. Signed-off-by: Andre Guedes Signed-off-by: Gustavo Padovan --- net/bluetooth/hci_conn.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'net/bluetooth/hci_conn.c') diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 4925a02ae7e4..b9f90169940b 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -117,7 +117,7 @@ static void hci_acl_create_connection_cancel(struct hci_conn *conn) hci_send_cmd(conn->hdev, HCI_OP_CREATE_CONN_CANCEL, sizeof(cp), &cp); } -void hci_acl_disconn(struct hci_conn *conn, __u8 reason) +void hci_disconnect(struct hci_conn *conn, __u8 reason) { struct hci_cp_disconnect cp; @@ -253,7 +253,7 @@ static void hci_conn_disconnect(struct hci_conn *conn) hci_amp_disconn(conn, reason); break; default: - hci_acl_disconn(conn, reason); + hci_disconnect(conn, reason); break; } } -- cgit v1.2.3 From 76a68ba0ae097be72dfa8f918b3139130da769a4 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Sat, 6 Apr 2013 20:28:37 +0200 Subject: Bluetooth: rename hci_conn_put to hci_conn_drop We use _get() and _put() for device ref-counting in the kernel. However, hci_conn_put() is _not_ used for ref-counting, hence, rename it to hci_conn_drop() so we can later fix ref-counting and introduce hci_conn_put(). hci_conn_hold() and hci_conn_put() are currently used to manage how long a connection should be held alive. When the last user drops the connection, we spawn a delayed work that performs the disconnect. Obviously, this has nothing to do with ref-counting for the _object_ but rather for the keep-alive of the connection. But we really _need_ proper ref-counting for the _object_ to allow connection-users like rfcomm-tty, HIDP or others. Signed-off-by: David Herrmann Acked-by: Marcel Holtmann Signed-off-by: Gustavo Padovan --- include/net/bluetooth/hci_core.h | 2 +- net/bluetooth/hci_conn.c | 6 +++--- net/bluetooth/hci_event.c | 36 ++++++++++++++++++------------------ net/bluetooth/l2cap_core.c | 6 +++--- net/bluetooth/mgmt.c | 6 +++--- net/bluetooth/sco.c | 6 +++--- net/bluetooth/smp.c | 2 +- 7 files changed, 32 insertions(+), 32 deletions(-) (limited to 'net/bluetooth/hci_conn.c') diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index d4e13bf5ae59..78ea9c7c202c 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -612,7 +612,7 @@ static inline void hci_conn_hold(struct hci_conn *conn) cancel_delayed_work(&conn->disc_work); } -static inline void hci_conn_put(struct hci_conn *conn) +static inline void hci_conn_drop(struct hci_conn *conn) { BT_DBG("hcon %p orig refcnt %d", conn, atomic_read(&conn->refcnt)); diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index b9f90169940b..30d7dfc23002 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -433,7 +433,7 @@ int hci_conn_del(struct hci_conn *conn) struct hci_conn *acl = conn->link; if (acl) { acl->link = NULL; - hci_conn_put(acl); + hci_conn_drop(acl); } } @@ -565,7 +565,7 @@ static struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, if (!sco) { sco = hci_conn_add(hdev, type, dst); if (!sco) { - hci_conn_put(acl); + hci_conn_drop(acl); return ERR_PTR(-ENOMEM); } } @@ -980,7 +980,7 @@ void hci_chan_del(struct hci_chan *chan) synchronize_rcu(); - hci_conn_put(conn); + hci_conn_drop(conn); skb_queue_purge(&chan->data_q); kfree(chan); diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index 0a2b128d2cc9..2cf28b198b31 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -1190,7 +1190,7 @@ static void hci_cs_auth_requested(struct hci_dev *hdev, __u8 status) if (conn) { if (conn->state == BT_CONFIG) { hci_proto_connect_cfm(conn, status); - hci_conn_put(conn); + hci_conn_drop(conn); } } @@ -1217,7 +1217,7 @@ static void hci_cs_set_conn_encrypt(struct hci_dev *hdev, __u8 status) if (conn) { if (conn->state == BT_CONFIG) { hci_proto_connect_cfm(conn, status); - hci_conn_put(conn); + hci_conn_drop(conn); } } @@ -1379,7 +1379,7 @@ static void hci_cs_read_remote_features(struct hci_dev *hdev, __u8 status) if (conn) { if (conn->state == BT_CONFIG) { hci_proto_connect_cfm(conn, status); - hci_conn_put(conn); + hci_conn_drop(conn); } } @@ -1406,7 +1406,7 @@ static void hci_cs_read_remote_ext_features(struct hci_dev *hdev, __u8 status) if (conn) { if (conn->state == BT_CONFIG) { hci_proto_connect_cfm(conn, status); - hci_conn_put(conn); + hci_conn_drop(conn); } } @@ -1860,7 +1860,7 @@ static void hci_conn_request_evt(struct hci_dev *hdev, struct sk_buff *skb) } else { conn->state = BT_CONNECT2; hci_proto_connect_cfm(conn, 0); - hci_conn_put(conn); + hci_conn_drop(conn); } } else { /* Connection rejected */ @@ -1967,14 +1967,14 @@ static void hci_auth_complete_evt(struct hci_dev *hdev, struct sk_buff *skb) } else { conn->state = BT_CONNECTED; hci_proto_connect_cfm(conn, ev->status); - hci_conn_put(conn); + hci_conn_drop(conn); } } else { hci_auth_cfm(conn, ev->status); hci_conn_hold(conn); conn->disc_timeout = HCI_DISCONN_TIMEOUT; - hci_conn_put(conn); + hci_conn_drop(conn); } if (test_bit(HCI_CONN_ENCRYPT_PEND, &conn->flags)) { @@ -2058,7 +2058,7 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb) if (ev->status && conn->state == BT_CONNECTED) { hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE); - hci_conn_put(conn); + hci_conn_drop(conn); goto unlock; } @@ -2067,7 +2067,7 @@ static void hci_encrypt_change_evt(struct hci_dev *hdev, struct sk_buff *skb) conn->state = BT_CONNECTED; hci_proto_connect_cfm(conn, ev->status); - hci_conn_put(conn); + hci_conn_drop(conn); } else hci_encrypt_cfm(conn, ev->status, ev->encrypt); } @@ -2142,7 +2142,7 @@ static void hci_remote_features_evt(struct hci_dev *hdev, if (!hci_outgoing_auth_needed(hdev, conn)) { conn->state = BT_CONNECTED; hci_proto_connect_cfm(conn, ev->status); - hci_conn_put(conn); + hci_conn_drop(conn); } unlock: @@ -2682,7 +2682,7 @@ static void hci_pin_code_request_evt(struct hci_dev *hdev, struct sk_buff *skb) if (conn->state == BT_CONNECTED) { hci_conn_hold(conn); conn->disc_timeout = HCI_PAIRING_TIMEOUT; - hci_conn_put(conn); + hci_conn_drop(conn); } if (!test_bit(HCI_PAIRABLE, &hdev->dev_flags)) @@ -2785,7 +2785,7 @@ static void hci_link_key_notify_evt(struct hci_dev *hdev, struct sk_buff *skb) if (ev->key_type != HCI_LK_CHANGED_COMBINATION) conn->key_type = ev->key_type; - hci_conn_put(conn); + hci_conn_drop(conn); } if (test_bit(HCI_LINK_KEYS, &hdev->dev_flags)) @@ -2954,7 +2954,7 @@ static void hci_remote_ext_features_evt(struct hci_dev *hdev, if (!hci_outgoing_auth_needed(hdev, conn)) { conn->state = BT_CONNECTED; hci_proto_connect_cfm(conn, ev->status); - hci_conn_put(conn); + hci_conn_drop(conn); } unlock: @@ -3087,7 +3087,7 @@ static void hci_key_refresh_complete_evt(struct hci_dev *hdev, if (ev->status && conn->state == BT_CONNECTED) { hci_disconnect(conn, HCI_ERROR_AUTH_FAILURE); - hci_conn_put(conn); + hci_conn_drop(conn); goto unlock; } @@ -3096,13 +3096,13 @@ static void hci_key_refresh_complete_evt(struct hci_dev *hdev, conn->state = BT_CONNECTED; hci_proto_connect_cfm(conn, ev->status); - hci_conn_put(conn); + hci_conn_drop(conn); } else { hci_auth_cfm(conn, ev->status); hci_conn_hold(conn); conn->disc_timeout = HCI_DISCONN_TIMEOUT; - hci_conn_put(conn); + hci_conn_drop(conn); } unlock: @@ -3363,7 +3363,7 @@ static void hci_simple_pair_complete_evt(struct hci_dev *hdev, mgmt_auth_failed(hdev, &conn->dst, conn->type, conn->dst_type, ev->status); - hci_conn_put(conn); + hci_conn_drop(conn); unlock: hci_dev_unlock(hdev); @@ -3451,7 +3451,7 @@ static void hci_phy_link_complete_evt(struct hci_dev *hdev, hci_conn_hold(hcon); hcon->disc_timeout = HCI_DISCONN_TIMEOUT; - hci_conn_put(hcon); + hci_conn_drop(hcon); hci_conn_hold_device(hcon); hci_conn_add_sysfs(hcon); diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 7c7e9321f1ea..7cdb93c21b32 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -571,7 +571,7 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err) chan->conn = NULL; if (chan->chan_type != L2CAP_CHAN_CONN_FIX_A2MP) - hci_conn_put(conn->hcon); + hci_conn_drop(conn->hcon); if (mgr && mgr->bredr_chan == chan) mgr->bredr_chan = NULL; @@ -1697,7 +1697,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, conn = l2cap_conn_add(hcon, 0); if (!conn) { - hci_conn_put(hcon); + hci_conn_drop(hcon); err = -ENOMEM; goto done; } @@ -1707,7 +1707,7 @@ int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid, if (!list_empty(&conn->chan_l)) { err = -EBUSY; - hci_conn_put(hcon); + hci_conn_drop(hcon); } if (err) diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c index 03e7e732215f..34ba1647e6e8 100644 --- a/net/bluetooth/mgmt.c +++ b/net/bluetooth/mgmt.c @@ -2131,7 +2131,7 @@ static void pairing_complete(struct pending_cmd *cmd, u8 status) conn->security_cfm_cb = NULL; conn->disconn_cfm_cb = NULL; - hci_conn_put(conn); + hci_conn_drop(conn); mgmt_pending_remove(cmd); } @@ -2222,7 +2222,7 @@ static int pair_device(struct sock *sk, struct hci_dev *hdev, void *data, } if (conn->connect_cfm_cb) { - hci_conn_put(conn); + hci_conn_drop(conn); err = cmd_complete(sk, hdev->id, MGMT_OP_PAIR_DEVICE, MGMT_STATUS_BUSY, &rp, sizeof(rp)); goto unlock; @@ -2231,7 +2231,7 @@ static int pair_device(struct sock *sk, struct hci_dev *hdev, void *data, cmd = mgmt_pending_add(sk, MGMT_OP_PAIR_DEVICE, hdev, data, len); if (!cmd) { err = -ENOMEM; - hci_conn_put(conn); + hci_conn_drop(conn); goto unlock; } diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c index d919d1161ab4..9909eec6afe3 100644 --- a/net/bluetooth/sco.c +++ b/net/bluetooth/sco.c @@ -185,7 +185,7 @@ static int sco_connect(struct sock *sk) conn = sco_conn_add(hcon); if (!conn) { - hci_conn_put(hcon); + hci_conn_drop(hcon); err = -ENOMEM; goto done; } @@ -353,7 +353,7 @@ static void __sco_sock_close(struct sock *sk) if (sco_pi(sk)->conn->hcon) { sk->sk_state = BT_DISCONN; sco_sock_set_timer(sk, SCO_DISCONN_TIMEOUT); - hci_conn_put(sco_pi(sk)->conn->hcon); + hci_conn_drop(sco_pi(sk)->conn->hcon); sco_pi(sk)->conn->hcon = NULL; } else sco_chan_del(sk, ECONNRESET); @@ -882,7 +882,7 @@ static void sco_chan_del(struct sock *sk, int err) sco_conn_unlock(conn); if (conn->hcon) - hci_conn_put(conn->hcon); + hci_conn_drop(conn->hcon); } sk->sk_state = BT_CLOSED; diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c index 5abefb12891d..b2296d3857a0 100644 --- a/net/bluetooth/smp.c +++ b/net/bluetooth/smp.c @@ -522,7 +522,7 @@ void smp_chan_destroy(struct l2cap_conn *conn) kfree(smp); conn->smp_chan = NULL; conn->hcon->smp_conn = NULL; - hci_conn_put(conn->hcon); + hci_conn_drop(conn->hcon); } int smp_user_confirm_reply(struct hci_conn *hcon, u16 mgmt_op, __le32 passkey) -- cgit v1.2.3 From 93796fa6f21411dab2ce7ba4fd7fd4d4ed4aca2e Mon Sep 17 00:00:00 2001 From: Claudio Takahasi Date: Thu, 11 Apr 2013 13:54:56 -0300 Subject: Bluetooth: Reject SCO when hci connection timeouts This patch sends Reject Synchronous Connection Request Command when hci_conn_timeout is triggered, and the SCO connection is in BT_CONNECT2 state. It prevents inconsistency if the remote host doesn't implement properly the timeout for the connection request, and it removes the connection reference left when the socket is closed for incoming SCO connections. [ 2650.129080] sco_sock_release: sock ffff8801ca417400, sk ffff88020c408800 [ 2650.129092] sco_sock_clear_timer: sock ffff88020c408800 state 6 [ 2650.129101] __sco_sock_close: sk ffff88020c408800 state 6 socket ffff8801ca417400 [ 2650.129108] sco_chan_del: sk ffff88020c408800, conn ffff8801c650ea20, err 104 [ 2650.129114] hci_conn_put: hcon ffff88020c40a800 orig refcnt 1 [ 2650.129128] sco_sock_kill: sk ffff88020c408800 state 9 [ 2650.129135] sco_sock_destruct: sk ffff88020c408800 [ 2650.138468] hci_conn_timeout: hcon ffff88020c40a800 state BT_CONNECT2 Signed-off-by: Claudio Takahasi Signed-off-by: Vinicius Costa Gomes Signed-off-by: Gustavo Padovan --- net/bluetooth/hci_conn.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'net/bluetooth/hci_conn.c') diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 30d7dfc23002..b1a02ce39a20 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -117,6 +117,16 @@ static void hci_acl_create_connection_cancel(struct hci_conn *conn) hci_send_cmd(conn->hdev, HCI_OP_CREATE_CONN_CANCEL, sizeof(cp), &cp); } +static void hci_reject_sco(struct hci_conn *conn) +{ + struct hci_cp_reject_sync_conn_req cp; + + cp.reason = HCI_ERROR_REMOTE_USER_TERM; + bacpy(&cp.bdaddr, &conn->dst); + + hci_send_cmd(conn->hdev, HCI_OP_REJECT_SYNC_CONN_REQ, sizeof(cp), &cp); +} + void hci_disconnect(struct hci_conn *conn, __u8 reason) { struct hci_cp_disconnect cp; @@ -276,6 +286,8 @@ static void hci_conn_timeout(struct work_struct *work) hci_acl_create_connection_cancel(conn); else if (conn->type == LE_LINK) hci_le_create_connection_cancel(conn); + } else if (conn->type == SCO_LINK || conn->type == ESCO_LINK) { + hci_reject_sco(conn); } break; case BT_CONFIG: -- cgit v1.2.3 From fc225c3f5d1b6aa6f99c5c300af4605e4923ce79 Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Sat, 6 Apr 2013 20:28:38 +0200 Subject: Bluetooth: remove unneeded hci_conn_hold/put_device() hci_conn_hold/put_device() is used to control when hci_conn->dev is no longer needed and can be deleted from the system. Lets first look how they are currently used throughout the code (excluding HIDP!). All code that uses hci_conn_hold_device() looks like this: ... hci_conn_hold_device(); hci_conn_add_sysfs(); ... On the other side, hci_conn_put_device() is exclusively used in hci_conn_del(). So, considering that hci_conn_del() must not be called twice (which would fail horribly), we know that hci_conn_put_device() is only called _once_ (which is in hci_conn_del()). On the other hand, hci_conn_add_sysfs() must not be called twice, either (it would call device_add twice, which breaks the device, see drivers/base/core.c). So we know that hci_conn_hold_device() is also called only once (it's only called directly before hci_conn_add_sysfs()). So hold and put are known to be called only once. That means we can safely remove them and directly call hci_conn_del_sysfs() in hci_conn_del(). But there is one issue left: HIDP also uses hci_conn_hold/put_device(). However, this case can be ignored and simply removed as it is totally broken. The issue is, the only thing HIDP delays with hci_conn_hold_device() is the removal of the hci_conn->dev from sysfs. But, the hci_conn device has no mechanism to get notified when its own parent (hci_dev) gets removed from sysfs. hci_dev_hold/put() does _not_ control when it is removed but only when the device object is created and destroyed. And hci_dev calls hci_conn_flush_*() when it removes itself from sysfs, which itself causes hci_conn_del() to be called, but it does _not_ cause hci_conn_del_sysfs() to be called, which is wrong. Hence, we fix it to call hci_conn_del_sysfs() in hci_conn_del(). This guarantees that a hci_conn object is removed from sysfs _before_ its parent hci_dev is removed. The changes to HIDP look scary, wrong and broken. However, if you look at the HIDP session management, you will notice they're already broken in the exact _same_ way (ever tried "unplugging" HIDP devices? Breaks _all_ the time). So this patch only makes HIDP look _scary_ and _obviously broken_. It does not break HIDP itself, it already is! See later patches in this series which fix HIDP to use proper session-management. Signed-off-by: David Herrmann Acked-by: Marcel Holtmann Signed-off-by: Gustavo Padovan --- include/net/bluetooth/hci_core.h | 4 ---- net/bluetooth/hci_conn.c | 17 +---------------- net/bluetooth/hci_event.c | 4 ---- net/bluetooth/hidp/core.c | 20 +++----------------- 4 files changed, 4 insertions(+), 41 deletions(-) (limited to 'net/bluetooth/hci_conn.c') diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 78ea9c7c202c..5590cc4412c6 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -345,7 +345,6 @@ struct hci_conn { struct timer_list auto_accept_timer; struct device dev; - atomic_t devref; struct hci_dev *hdev; void *l2cap_data; @@ -601,9 +600,6 @@ 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_conn_hold_device(struct hci_conn *conn); -void hci_conn_put_device(struct hci_conn *conn); - static inline void hci_conn_hold(struct hci_conn *conn) { BT_DBG("hcon %p orig refcnt %d", conn, atomic_read(&conn->refcnt)); diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index b1a02ce39a20..6b5b8e77cf0b 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -410,8 +410,6 @@ struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst) if (hdev->notify) hdev->notify(hdev, HCI_NOTIFY_CONN_ADD); - atomic_set(&conn->devref, 0); - hci_conn_init_sysfs(conn); return conn; @@ -460,7 +458,7 @@ int hci_conn_del(struct hci_conn *conn) skb_queue_purge(&conn->data_q); - hci_conn_put_device(conn); + hci_conn_del_sysfs(conn); hci_dev_put(hdev); @@ -847,19 +845,6 @@ void hci_conn_check_pending(struct hci_dev *hdev) hci_dev_unlock(hdev); } -void hci_conn_hold_device(struct hci_conn *conn) -{ - atomic_inc(&conn->devref); -} -EXPORT_SYMBOL(hci_conn_hold_device); - -void hci_conn_put_device(struct hci_conn *conn) -{ - if (atomic_dec_and_test(&conn->devref)) - hci_conn_del_sysfs(conn); -} -EXPORT_SYMBOL(hci_conn_put_device); - int hci_get_conn_list(void __user *arg) { struct hci_conn *c; diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c index f6ea3c734269..688c1a9949cc 100644 --- a/net/bluetooth/hci_event.c +++ b/net/bluetooth/hci_event.c @@ -1706,7 +1706,6 @@ static void hci_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb) } else conn->state = BT_CONNECTED; - hci_conn_hold_device(conn); hci_conn_add_sysfs(conn); if (test_bit(HCI_AUTH, &hdev->flags)) @@ -2987,7 +2986,6 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev, conn->handle = __le16_to_cpu(ev->handle); conn->state = BT_CONNECTED; - hci_conn_hold_device(conn); hci_conn_add_sysfs(conn); break; @@ -3452,7 +3450,6 @@ static void hci_phy_link_complete_evt(struct hci_dev *hdev, hcon->disc_timeout = HCI_DISCONN_TIMEOUT; hci_conn_drop(hcon); - hci_conn_hold_device(hcon); hci_conn_add_sysfs(hcon); amp_physical_cfm(bredr_hcon, hcon); @@ -3586,7 +3583,6 @@ static void hci_le_conn_complete_evt(struct hci_dev *hdev, struct sk_buff *skb) conn->handle = __le16_to_cpu(ev->handle); conn->state = BT_CONNECTED; - hci_conn_hold_device(conn); hci_conn_add_sysfs(conn); hci_proto_connect_cfm(conn, ev->status); diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c index 4ab82cb3eac3..9734136d6431 100644 --- a/net/bluetooth/hidp/core.c +++ b/net/bluetooth/hidp/core.c @@ -73,18 +73,6 @@ static struct hidp_session *__hidp_get_session(bdaddr_t *bdaddr) return NULL; } -static void __hidp_link_session(struct hidp_session *session) -{ - list_add(&session->list, &hidp_session_list); -} - -static void __hidp_unlink_session(struct hidp_session *session) -{ - hci_conn_put_device(session->conn); - - list_del(&session->list); -} - static void __hidp_copy_session(struct hidp_session *session, struct hidp_conninfo *ci) { memset(ci, 0, sizeof(*ci)); @@ -760,7 +748,7 @@ static int hidp_session(void *arg) fput(session->ctrl_sock->file); - __hidp_unlink_session(session); + list_del(&session->list); up_write(&hidp_session_sem); @@ -783,8 +771,6 @@ static struct hci_conn *hidp_get_connection(struct hidp_session *session) hci_dev_lock(hdev); conn = hci_conn_hash_lookup_ba(hdev, ACL_LINK, dst); - if (conn) - hci_conn_hold_device(conn); hci_dev_unlock(hdev); hci_dev_put(hdev); @@ -1026,7 +1012,7 @@ int hidp_add_connection(struct hidp_connadd_req *req, struct socket *ctrl_sock, session->flags = req->flags & (1 << HIDP_BLUETOOTH_VENDOR_ID); session->idle_to = req->idle_to; - __hidp_link_session(session); + list_add(&session->list, &hidp_session_list); if (req->rd_size > 0) { err = hidp_setup_hid(session, req); @@ -1106,7 +1092,7 @@ unlink: session->rd_data = NULL; purge: - __hidp_unlink_session(session); + list_del(&session->list); skb_queue_purge(&session->ctrl_transmit); skb_queue_purge(&session->intr_transmit); -- cgit v1.2.3 From 8d12356f33f819ec0d064e233f7ca8e59eaa38ef Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Sat, 6 Apr 2013 20:28:39 +0200 Subject: Bluetooth: introduce hci_conn ref-counting We currently do not allow using hci_conn from outside of HCI-core. However, several other users could make great use of it. This includes HIDP, rfcomm and all other sub-protocols that rely on an active connection. Hence, we now introduce hci_conn ref-counting. We currently never call get_device(). put_device() is exclusively used in hci_conn_del_sysfs(). Hence, we currently never have a greater device-refcnt than 1. Therefore, it is safe to move the put_device() call from hci_conn_del_sysfs() to hci_conn_del() (it's the only caller). In fact, this even fixes a "use-after-free" bug as we access hci_conn after calling hci_conn_del_sysfs() in hci_conn_del(). From now on we can add references to hci_conn objects in other layers (like l2cap_sock, HIDP, rfcomm, ...) and grab a reference via hci_conn_get(). This does _not_ guarantee, that the connection is still alive. But, this isn't what we want. We can simply lock the hci_conn device and use "device_is_registered(hci_conn->dev)" to test that. However, this is hardly necessary as outside users should never rely on the HCI connection to be alive, anyway. Instead, they should solely rely on the device-object to be available. But if sub-devices want the hci_conn object as sysfs parent, they need to be notified when the connection drops. This will be introduced in later patches with l2cap_users. Signed-off-by: David Herrmann Acked-by: Marcel Holtmann Signed-off-by: Gustavo Padovan --- include/net/bluetooth/hci_core.h | 31 +++++++++++++++++++++++++++++++ net/bluetooth/hci_conn.c | 3 +-- net/bluetooth/hci_sysfs.c | 1 - 3 files changed, 32 insertions(+), 3 deletions(-) (limited to 'net/bluetooth/hci_conn.c') diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h index 5590cc4412c6..d324b11a0c8f 100644 --- a/include/net/bluetooth/hci_core.h +++ b/include/net/bluetooth/hci_core.h @@ -600,6 +600,37 @@ int hci_conn_switch_role(struct hci_conn *conn, __u8 role); void hci_conn_enter_active_mode(struct hci_conn *conn, __u8 force_active); +/* + * hci_conn_get() and hci_conn_put() are used to control the life-time of an + * "hci_conn" object. They do not guarantee that the hci_conn object is running, + * working or anything else. They just guarantee that the object is available + * and can be dereferenced. So you can use its locks, local variables and any + * other constant data. + * Before accessing runtime data, you _must_ lock the object and then check that + * it is still running. As soon as you release the locks, the connection might + * get dropped, though. + * + * On the other hand, hci_conn_hold() and hci_conn_drop() are used to control + * how long the underlying connection is held. So every channel that runs on the + * hci_conn object calls this to prevent the connection from disappearing. As + * long as you hold a device, you must also guarantee that you have a valid + * reference to the device via hci_conn_get() (or the initial reference from + * hci_conn_add()). + * The hold()/drop() ref-count is known to drop below 0 sometimes, which doesn't + * break because nobody cares for that. But this means, we cannot use + * _get()/_drop() in it, but require the caller to have a valid ref (FIXME). + */ + +static inline void hci_conn_get(struct hci_conn *conn) +{ + get_device(&conn->dev); +} + +static inline void hci_conn_put(struct hci_conn *conn) +{ + put_device(&conn->dev); +} + static inline void hci_conn_hold(struct hci_conn *conn) { BT_DBG("hcon %p orig refcnt %d", conn, atomic_read(&conn->refcnt)); diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index 6b5b8e77cf0b..6c7f36379722 100644 --- a/net/bluetooth/hci_conn.c +++ b/net/bluetooth/hci_conn.c @@ -462,8 +462,7 @@ int hci_conn_del(struct hci_conn *conn) hci_dev_put(hdev); - if (conn->handle == 0) - kfree(conn); + hci_conn_put(conn); return 0; } diff --git a/net/bluetooth/hci_sysfs.c b/net/bluetooth/hci_sysfs.c index ff38561385de..6fe15c822847 100644 --- a/net/bluetooth/hci_sysfs.c +++ b/net/bluetooth/hci_sysfs.c @@ -146,7 +146,6 @@ void hci_conn_del_sysfs(struct hci_conn *conn) } device_del(&conn->dev); - put_device(&conn->dev); hci_dev_put(hdev); } -- cgit v1.2.3