From bfaaeb3ed5533a2dd38e3aa9ea43efd619690aed Mon Sep 17 00:00:00 2001 From: steven miao Date: Sat, 16 Oct 2010 18:29:47 -0400 Subject: Bluetooth: fix unaligned access to l2cap conf data In function l2cap_get_conf_opt() and l2cap_add_conf_opt() the address of opt->val sometimes is not at the edge of 2-bytes/4-bytes, so 2-bytes/4 bytes access will cause data misalignment exeception. Use get_unaligned_le16/32 and put_unaligned_le16/32 function to avoid data misalignment execption. Signed-off-by: steven miao Signed-off-by: Mike Frysinger Acked-by: Marcel Holtmann Signed-off-by: Gustavo F. Padovan --- net/bluetooth/l2cap.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'net/bluetooth/l2cap.c') diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c index daa7a988d9a..b3fb02ab229 100644 --- a/net/bluetooth/l2cap.c +++ b/net/bluetooth/l2cap.c @@ -2421,11 +2421,11 @@ static inline int l2cap_get_conf_opt(void **ptr, int *type, int *olen, unsigned break; case 2: - *val = __le16_to_cpu(*((__le16 *) opt->val)); + *val = get_unaligned_le16(opt->val); break; case 4: - *val = __le32_to_cpu(*((__le32 *) opt->val)); + *val = get_unaligned_le32(opt->val); break; default: @@ -2452,11 +2452,11 @@ static void l2cap_add_conf_opt(void **ptr, u8 type, u8 len, unsigned long val) break; case 2: - *((__le16 *) opt->val) = cpu_to_le16(val); + put_unaligned_le16(cpu_to_le16(val), opt->val); break; case 4: - *((__le32 *) opt->val) = cpu_to_le32(val); + put_unaligned_le32(cpu_to_le32(val), opt->val); break; default: -- cgit v1.2.3 From 4f8b691c9fb02e72359e71592098c1de3b8ec712 Mon Sep 17 00:00:00 2001 From: "Gustavo F. Padovan" Date: Mon, 18 Oct 2010 14:25:53 -0200 Subject: Bluetooth: fix endianness conversion in L2CAP Last commit added a wrong endianness conversion. Fixing that. Reported-by: Harvey Harrison Signed-off-by: Gustavo F. Padovan --- net/bluetooth/l2cap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'net/bluetooth/l2cap.c') diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c index b3fb02ab229..cd8f6ea0384 100644 --- a/net/bluetooth/l2cap.c +++ b/net/bluetooth/l2cap.c @@ -2452,11 +2452,11 @@ static void l2cap_add_conf_opt(void **ptr, u8 type, u8 len, unsigned long val) break; case 2: - put_unaligned_le16(cpu_to_le16(val), opt->val); + put_unaligned_le16(val, opt->val); break; case 4: - put_unaligned_le32(cpu_to_le32(val), opt->val); + put_unaligned_le32(val, opt->val); break; default: -- cgit v1.2.3 From a49184c229535ebedbb659214db2d4d1d77b7c07 Mon Sep 17 00:00:00 2001 From: Andrei Emeltchenko Date: Wed, 3 Nov 2010 12:32:44 +0200 Subject: Bluetooth: Check sk is not owned before freeing l2cap_conn Check that socket sk is not locked in user process before removing l2cap connection handler. lock_sock and release_sock do not hold a normal spinlock directly but instead hold the owner field. This means bh_lock_sock can still execute even if the socket is "locked". More info can be found here: http://www.linuxfoundation.org/collaborate/workgroups/networking/socketlocks krfcommd kernel thread may be preempted with l2cap tasklet which remove l2cap_conn structure. If krfcommd is in process of sending of RFCOMM reply (like "RFCOMM UA" reply to "RFCOMM DISC") then kernel crash happens. ... [ 694.175933] Unable to handle kernel NULL pointer dereference at virtual address 00000000 [ 694.184936] pgd = c0004000 [ 694.187683] [00000000] *pgd=00000000 [ 694.191711] Internal error: Oops: 5 [#1] PREEMPT [ 694.196350] last sysfs file: /sys/devices/platform/hci_h4p/firmware/hci_h4p/loading [ 694.260375] CPU: 0 Not tainted (2.6.32.10 #1) [ 694.265106] PC is at l2cap_sock_sendmsg+0x43c/0x73c [l2cap] [ 694.270721] LR is at 0xd7017303 ... [ 694.525085] Backtrace: [ 694.527587] [] (l2cap_sock_sendmsg+0x0/0x73c [l2cap]) from [] (sock_sendmsg+0xb8/0xd8) [ 694.537292] [] (sock_sendmsg+0x0/0xd8) from [] (kernel_sendmsg+0x48/0x80) Signed-off-by: Andrei Emeltchenko Acked-by: Marcel Holtmann Signed-off-by: Gustavo F. Padovan --- net/bluetooth/l2cap.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) (limited to 'net/bluetooth/l2cap.c') diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c index cd8f6ea0384..4ed38272df7 100644 --- a/net/bluetooth/l2cap.c +++ b/net/bluetooth/l2cap.c @@ -3078,6 +3078,14 @@ static inline int l2cap_connect_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hd break; default: + /* don't delete l2cap channel if sk is owned by user */ + if (sock_owned_by_user(sk)) { + sk->sk_state = BT_DISCONN; + l2cap_sock_clear_timer(sk); + l2cap_sock_set_timer(sk, HZ / 5); + break; + } + l2cap_chan_del(sk, ECONNREFUSED); break; } @@ -3283,6 +3291,15 @@ static inline int l2cap_disconnect_req(struct l2cap_conn *conn, struct l2cap_cmd sk->sk_shutdown = SHUTDOWN_MASK; + /* don't delete l2cap channel if sk is owned by user */ + if (sock_owned_by_user(sk)) { + sk->sk_state = BT_DISCONN; + l2cap_sock_clear_timer(sk); + l2cap_sock_set_timer(sk, HZ / 5); + bh_unlock_sock(sk); + return 0; + } + l2cap_chan_del(sk, ECONNRESET); bh_unlock_sock(sk); @@ -3305,6 +3322,15 @@ static inline int l2cap_disconnect_rsp(struct l2cap_conn *conn, struct l2cap_cmd if (!sk) return 0; + /* don't delete l2cap channel if sk is owned by user */ + if (sock_owned_by_user(sk)) { + sk->sk_state = BT_DISCONN; + l2cap_sock_clear_timer(sk); + l2cap_sock_set_timer(sk, HZ / 5); + bh_unlock_sock(sk); + return 0; + } + l2cap_chan_del(sk, 0); bh_unlock_sock(sk); -- cgit v1.2.3 From 940a9eea80946b64b96bd8af1fc71b30c602d057 Mon Sep 17 00:00:00 2001 From: Andrei Emeltchenko Date: Wed, 3 Nov 2010 12:32:45 +0200 Subject: Bluetooth: timer check sk is not owned before freeing In timer context we might delete l2cap channel used by krfcommd. The check makes sure that sk is not owned. If sk is owned we restart timer for HZ/5. Signed-off-by: Andrei Emeltchenko Acked-by: Marcel Holtmann Signed-off-by: Gustavo F. Padovan --- net/bluetooth/l2cap.c | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) (limited to 'net/bluetooth/l2cap.c') diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c index 4ed38272df7..18a802cc046 100644 --- a/net/bluetooth/l2cap.c +++ b/net/bluetooth/l2cap.c @@ -83,6 +83,18 @@ static struct sk_buff *l2cap_build_cmd(struct l2cap_conn *conn, static int l2cap_ertm_data_rcv(struct sock *sk, struct sk_buff *skb); /* ---- L2CAP timers ---- */ +static void l2cap_sock_set_timer(struct sock *sk, long timeout) +{ + BT_DBG("sk %p state %d timeout %ld", sk, sk->sk_state, timeout); + sk_reset_timer(sk, &sk->sk_timer, jiffies + timeout); +} + +static void l2cap_sock_clear_timer(struct sock *sk) +{ + BT_DBG("sock %p state %d", sk, sk->sk_state); + sk_stop_timer(sk, &sk->sk_timer); +} + static void l2cap_sock_timeout(unsigned long arg) { struct sock *sk = (struct sock *) arg; @@ -92,6 +104,14 @@ static void l2cap_sock_timeout(unsigned long arg) bh_lock_sock(sk); + if (sock_owned_by_user(sk)) { + /* sk is owned by user. Try again later */ + l2cap_sock_set_timer(sk, HZ / 5); + bh_unlock_sock(sk); + sock_put(sk); + return; + } + if (sk->sk_state == BT_CONNECTED || sk->sk_state == BT_CONFIG) reason = ECONNREFUSED; else if (sk->sk_state == BT_CONNECT && @@ -108,18 +128,6 @@ static void l2cap_sock_timeout(unsigned long arg) sock_put(sk); } -static void l2cap_sock_set_timer(struct sock *sk, long timeout) -{ - BT_DBG("sk %p state %d timeout %ld", sk, sk->sk_state, timeout); - sk_reset_timer(sk, &sk->sk_timer, jiffies + timeout); -} - -static void l2cap_sock_clear_timer(struct sock *sk) -{ - BT_DBG("sock %p state %d", sk, sk->sk_state); - sk_stop_timer(sk, &sk->sk_timer); -} - /* ---- L2CAP channels ---- */ static struct sock *__l2cap_get_chan_by_dcid(struct l2cap_chan_list *l, u16 cid) { -- cgit v1.2.3 From e0f0cb56364958223f0cb1f1b0b0eecf1b8dcb95 Mon Sep 17 00:00:00 2001 From: "Gustavo F. Padovan" Date: Mon, 1 Nov 2010 18:43:53 +0000 Subject: Bluetooth: Get rid of __l2cap_get_sock_by_psm() l2cap_get_sock_by_psm() was the only user of this function, so I merged both into l2cap_get_sock_by_psm(). The socket lock now should be hold outside of l2cap_get_sock_by_psm() once we hold and release it inside the same function now. Signed-off-by: Gustavo F. Padovan --- net/bluetooth/l2cap.c | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) (limited to 'net/bluetooth/l2cap.c') diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c index 18a802cc046..12b4aa2f8fc 100644 --- a/net/bluetooth/l2cap.c +++ b/net/bluetooth/l2cap.c @@ -751,11 +751,13 @@ found: /* Find socket with psm and source bdaddr. * Returns closest match. */ -static struct sock *__l2cap_get_sock_by_psm(int state, __le16 psm, bdaddr_t *src) +static struct sock *l2cap_get_sock_by_psm(int state, __le16 psm, bdaddr_t *src) { struct sock *sk = NULL, *sk1 = NULL; struct hlist_node *node; + read_lock(&l2cap_sk_list.lock); + sk_for_each(sk, node, &l2cap_sk_list.head) { if (state && sk->sk_state != state) continue; @@ -770,20 +772,10 @@ static struct sock *__l2cap_get_sock_by_psm(int state, __le16 psm, bdaddr_t *src sk1 = sk; } } - return node ? sk : sk1; -} -/* Find socket with given address (psm, src). - * Returns locked socket */ -static inline struct sock *l2cap_get_sock_by_psm(int state, __le16 psm, bdaddr_t *src) -{ - struct sock *s; - read_lock(&l2cap_sk_list.lock); - s = __l2cap_get_sock_by_psm(state, psm, src); - if (s) - bh_lock_sock(s); read_unlock(&l2cap_sk_list.lock); - return s; + + return node ? sk : sk1; } static void l2cap_sock_destruct(struct sock *sk) @@ -2934,6 +2926,8 @@ static inline int l2cap_connect_req(struct l2cap_conn *conn, struct l2cap_cmd_hd goto sendresp; } + bh_lock_sock(parent); + /* Check if the ACL is secure enough (if not SDP) */ if (psm != cpu_to_le16(0x0001) && !hci_conn_check_link_mode(conn->hcon)) { @@ -4464,6 +4458,8 @@ static inline int l2cap_conless_channel(struct l2cap_conn *conn, __le16 psm, str if (!sk) goto drop; + bh_lock_sock(sk); + BT_DBG("sk %p, len %d", sk, skb->len); if (sk->sk_state != BT_BOUND && sk->sk_state != BT_CONNECTED) -- cgit v1.2.3 From b78d7b4f204a6ba1901af36c95e10fded9816054 Mon Sep 17 00:00:00 2001 From: Anderson Lizardo Date: Mon, 29 Nov 2010 12:15:50 -0400 Subject: Bluetooth: Fix error handling for l2cap_init() create_singlethread_workqueue() may fail with errors such as -ENOMEM. If this happens, the return value is not set to a negative value and the module load will succeed. It will then crash on module unload because of a destroy_workqueue() call on a NULL pointer. Additionally, the _busy_wq workqueue is not being destroyed if any errors happen on l2cap_init(). Signed-off-by: Anderson Lizardo Signed-off-by: Gustavo F. Padovan --- net/bluetooth/l2cap.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'net/bluetooth/l2cap.c') diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c index 12b4aa2f8fc..a1c7ae88dd1 100644 --- a/net/bluetooth/l2cap.c +++ b/net/bluetooth/l2cap.c @@ -4871,8 +4871,10 @@ static int __init l2cap_init(void) return err; _busy_wq = create_singlethread_workqueue("l2cap"); - if (!_busy_wq) - goto error; + if (!_busy_wq) { + proto_unregister(&l2cap_proto); + return -ENOMEM; + } err = bt_sock_register(BTPROTO_L2CAP, &l2cap_sock_family_ops); if (err < 0) { @@ -4900,6 +4902,7 @@ static int __init l2cap_init(void) return 0; error: + destroy_workqueue(_busy_wq); proto_unregister(&l2cap_proto); return err; } -- cgit v1.2.3 From 894718a6be69d8cfd191dc291b42be32a1e4851b Mon Sep 17 00:00:00 2001 From: Andrei Emeltchenko Date: Wed, 1 Dec 2010 16:58:24 +0200 Subject: Bluetooth: clean up l2cap code Do not initialize static vars to zero, macros with complex values shall be enclosed with (), remove unneeded braces. Signed-off-by: Andrei Emeltchenko Signed-off-by: Gustavo F. Padovan --- include/net/bluetooth/l2cap.h | 10 +++++----- net/bluetooth/l2cap.c | 7 +++---- 2 files changed, 8 insertions(+), 9 deletions(-) (limited to 'net/bluetooth/l2cap.c') diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h index c819c8bf9b6..217bb91a734 100644 --- a/include/net/bluetooth/l2cap.h +++ b/include/net/bluetooth/l2cap.h @@ -417,11 +417,11 @@ static inline int l2cap_tx_window_full(struct sock *sk) return sub == pi->remote_tx_win; } -#define __get_txseq(ctrl) ((ctrl) & L2CAP_CTRL_TXSEQ) >> 1 -#define __get_reqseq(ctrl) ((ctrl) & L2CAP_CTRL_REQSEQ) >> 8 -#define __is_iframe(ctrl) !((ctrl) & L2CAP_CTRL_FRAME_TYPE) -#define __is_sframe(ctrl) (ctrl) & L2CAP_CTRL_FRAME_TYPE -#define __is_sar_start(ctrl) ((ctrl) & L2CAP_CTRL_SAR) == L2CAP_SDU_START +#define __get_txseq(ctrl) (((ctrl) & L2CAP_CTRL_TXSEQ) >> 1) +#define __get_reqseq(ctrl) (((ctrl) & L2CAP_CTRL_REQSEQ) >> 8) +#define __is_iframe(ctrl) (!((ctrl) & L2CAP_CTRL_FRAME_TYPE)) +#define __is_sframe(ctrl) ((ctrl) & L2CAP_CTRL_FRAME_TYPE) +#define __is_sar_start(ctrl) (((ctrl) & L2CAP_CTRL_SAR) == L2CAP_SDU_START) void l2cap_load(void); diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c index a1c7ae88dd1..c12eccfdfe0 100644 --- a/net/bluetooth/l2cap.c +++ b/net/bluetooth/l2cap.c @@ -57,7 +57,7 @@ #define VERSION "2.15" -static int disable_ertm = 0; +static int disable_ertm; static u32 l2cap_feat_mask = L2CAP_FEAT_FIXED_CHAN; static u8 l2cap_fixed_chan[8] = { 0x02, }; @@ -4162,11 +4162,10 @@ static inline void l2cap_data_channel_rrframe(struct sock *sk, u16 rx_control) __mod_retrans_timer(); pi->conn_state &= ~L2CAP_CONN_REMOTE_BUSY; - if (pi->conn_state & L2CAP_CONN_SREJ_SENT) { + if (pi->conn_state & L2CAP_CONN_SREJ_SENT) l2cap_send_ack(pi); - } else { + else l2cap_ertm_send(sk); - } } } -- cgit v1.2.3 From df6bd743b6f06b066c1c3ba7f2853a6e8d61468c Mon Sep 17 00:00:00 2001 From: "Gustavo F. Padovan" Date: Mon, 14 Jun 2010 02:26:15 -0300 Subject: Bluetooth: Don't accept ConfigReq if we aren't in the BT_CONFIG state If such event happens we shall reply with a Command Reject, because we are not expecting any configure request. Signed-off-by: Gustavo F. Padovan Signed-off-by: Marcel Holtmann --- net/bluetooth/l2cap.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) (limited to 'net/bluetooth/l2cap.c') diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c index c12eccfdfe0..c791fcda7b2 100644 --- a/net/bluetooth/l2cap.c +++ b/net/bluetooth/l2cap.c @@ -3124,8 +3124,14 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr if (!sk) return -ENOENT; - if (sk->sk_state == BT_DISCONN) + if (sk->sk_state != BT_CONFIG) { + struct l2cap_cmd_rej rej; + + rej.reason = cpu_to_le16(0x0002); + l2cap_send_cmd(conn, cmd->ident, L2CAP_COMMAND_REJ, + sizeof(rej), &rej); goto unlock; + } /* Reject if config buffer is too small. */ len = cmd_len - sizeof(*req); -- cgit v1.2.3