From 8c462b6047da80491b8cb6be878e8bf9313ac3e1 Mon Sep 17 00:00:00 2001 From: Mat Martineau Date: Tue, 24 Aug 2010 15:35:42 -0700 Subject: Bluetooth: Only enable L2CAP FCS for ERTM or streaming This fixes a bug which caused the FCS setting to show L2CAP_FCS_CRC16 with L2CAP modes other than ERTM or streaming. At present, this only affects the FCS value shown with getsockopt() for basic mode. Signed-off-by: Mat Martineau Signed-off-by: Gustavo F. Padovan --- net/bluetooth/l2cap.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c index fadf26b..c784703 100644 --- a/net/bluetooth/l2cap.c +++ b/net/bluetooth/l2cap.c @@ -3071,6 +3071,17 @@ static inline int l2cap_connect_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hd return 0; } +static inline void set_default_fcs(struct l2cap_pinfo *pi) +{ + /* FCS is enabled only in ERTM or streaming mode, if one or both + * sides request it. + */ + if (pi->mode != L2CAP_MODE_ERTM && pi->mode != L2CAP_MODE_STREAMING) + pi->fcs = L2CAP_FCS_NONE; + else if (!(pi->conf_state & L2CAP_CONF_NO_FCS_RECV)) + pi->fcs = L2CAP_FCS_CRC16; +} + static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr *cmd, u16 cmd_len, u8 *data) { struct l2cap_conf_req *req = (struct l2cap_conf_req *) data; @@ -3135,9 +3146,7 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr goto unlock; if (l2cap_pi(sk)->conf_state & L2CAP_CONF_INPUT_DONE) { - if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) || - l2cap_pi(sk)->fcs != L2CAP_FCS_NONE) - l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16; + set_default_fcs(l2cap_pi(sk)); sk->sk_state = BT_CONNECTED; @@ -3225,9 +3234,7 @@ static inline int l2cap_config_rsp(struct l2cap_conn *conn, struct l2cap_cmd_hdr l2cap_pi(sk)->conf_state |= L2CAP_CONF_INPUT_DONE; if (l2cap_pi(sk)->conf_state & L2CAP_CONF_OUTPUT_DONE) { - if (!(l2cap_pi(sk)->conf_state & L2CAP_CONF_NO_FCS_RECV) || - l2cap_pi(sk)->fcs != L2CAP_FCS_NONE) - l2cap_pi(sk)->fcs = L2CAP_FCS_CRC16; + set_default_fcs(l2cap_pi(sk)); sk->sk_state = BT_CONNECTED; l2cap_pi(sk)->next_tx_seq = 0; -- cgit v1.1 From 8183b775bc5b79b6b1e250019c9dd930554dfa94 Mon Sep 17 00:00:00 2001 From: Andrei Emeltchenko Date: Wed, 1 Sep 2010 15:17:25 +0300 Subject: Bluetooth: fix MTU L2CAP configuration parameter When receiving L2CAP negative configuration response with respect to MTU parameter we modify wrong field. MTU here means proposed value of MTU that the remote device intends to transmit. So for local L2CAP socket it is pi->imtu. Signed-off-by: Andrei Emeltchenko Acked-by: Ville Tervo Signed-off-by: Gustavo F. Padovan --- net/bluetooth/l2cap.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c index c784703..9fad312 100644 --- a/net/bluetooth/l2cap.c +++ b/net/bluetooth/l2cap.c @@ -2771,10 +2771,10 @@ static int l2cap_parse_conf_rsp(struct sock *sk, void *rsp, int len, void *data, case L2CAP_CONF_MTU: if (val < L2CAP_DEFAULT_MIN_MTU) { *result = L2CAP_CONF_UNACCEPT; - pi->omtu = L2CAP_DEFAULT_MIN_MTU; + pi->imtu = L2CAP_DEFAULT_MIN_MTU; } else - pi->omtu = val; - l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, pi->omtu); + pi->imtu = val; + l2cap_add_conf_opt(&ptr, L2CAP_CONF_MTU, 2, pi->imtu); break; case L2CAP_CONF_FLUSH_TO: -- cgit v1.1 From ccbb84af28594e19fd4bf27ff2828c80d03b6081 Mon Sep 17 00:00:00 2001 From: "Gustavo F. Padovan" Date: Mon, 30 Aug 2010 18:44:44 -0300 Subject: Bluetooth: Simplify L2CAP Streaming mode sending As we don't have any error control on the Streaming mode, i.e., we don't need to keep a copy of the skb for later resending we don't need to call skb_clone() on it. Then we can go one further here, and dequeue the skb before sending it, that also means we don't need to look to sk->sk_send_head anymore. The patch saves memory and time when sending Streaming mode data, so it is good to mainline. Signed-off-by: Gustavo F. Padovan --- net/bluetooth/l2cap.c | 24 +++++++----------------- 1 file changed, 7 insertions(+), 17 deletions(-) diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c index 9fad312..f2062aa 100644 --- a/net/bluetooth/l2cap.c +++ b/net/bluetooth/l2cap.c @@ -1441,33 +1441,23 @@ static inline void l2cap_do_send(struct sock *sk, struct sk_buff *skb) static void l2cap_streaming_send(struct sock *sk) { - struct sk_buff *skb, *tx_skb; + struct sk_buff *skb; struct l2cap_pinfo *pi = l2cap_pi(sk); u16 control, fcs; - while ((skb = sk->sk_send_head)) { - tx_skb = skb_clone(skb, GFP_ATOMIC); - - control = get_unaligned_le16(tx_skb->data + L2CAP_HDR_SIZE); + while ((skb = skb_dequeue(TX_QUEUE(sk)))) { + control = get_unaligned_le16(skb->data + L2CAP_HDR_SIZE); control |= pi->next_tx_seq << L2CAP_CTRL_TXSEQ_SHIFT; - put_unaligned_le16(control, tx_skb->data + L2CAP_HDR_SIZE); + put_unaligned_le16(control, skb->data + L2CAP_HDR_SIZE); if (pi->fcs == L2CAP_FCS_CRC16) { - fcs = crc16(0, (u8 *)tx_skb->data, tx_skb->len - 2); - put_unaligned_le16(fcs, tx_skb->data + tx_skb->len - 2); + fcs = crc16(0, (u8 *)skb->data, skb->len - 2); + put_unaligned_le16(fcs, skb->data + skb->len - 2); } - l2cap_do_send(sk, tx_skb); + l2cap_do_send(sk, skb); pi->next_tx_seq = (pi->next_tx_seq + 1) % 64; - - if (skb_queue_is_last(TX_QUEUE(sk), skb)) - sk->sk_send_head = NULL; - else - sk->sk_send_head = skb_queue_next(TX_QUEUE(sk), skb); - - skb = skb_dequeue(TX_QUEUE(sk)); - kfree_skb(skb); } } -- cgit v1.1 From fad003b6c8e3d944d4453fd569b0702ef1af82b3 Mon Sep 17 00:00:00 2001 From: "Gustavo F. Padovan" Date: Sat, 14 Aug 2010 00:48:07 -0300 Subject: Bluetooth: Fix inconsistent lock state with RFCOMM When receiving a rfcomm connection with the old dund deamon a inconsistent lock state happens. That's because interrupts were already disabled by l2cap_conn_start() when rfcomm_sk_state_change() try to lock the spin_lock. As result we may have a inconsistent lock state for l2cap_conn_start() after rfcomm_sk_state_change() calls bh_lock_sock() and disable interrupts as well. [ 2833.151999] [ 2833.151999] ================================= [ 2833.151999] [ INFO: inconsistent lock state ] [ 2833.151999] 2.6.36-rc3 #2 [ 2833.151999] --------------------------------- [ 2833.151999] inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage. [ 2833.151999] krfcommd/2306 [HC0[0]:SC0[0]:HE1:SE1] takes: [ 2833.151999] (slock-AF_BLUETOOTH){+.?...}, at: [] rfcomm_sk_state_change+0x46/0x170 [rfcomm] [ 2833.151999] {IN-SOFTIRQ-W} state was registered at: [ 2833.151999] [] __lock_acquire+0x5b6/0x1560 [ 2833.151999] [] lock_acquire+0x5a/0x70 [ 2833.151999] [] _raw_spin_lock+0x2c/0x40 [ 2833.151999] [] l2cap_conn_start+0x92/0x640 [l2cap] [ 2833.151999] [] l2cap_sig_channel+0x6bf/0x1320 [l2cap] [ 2833.151999] [] l2cap_recv_frame+0x133/0x770 [l2cap] [ 2833.151999] [] l2cap_recv_acldata+0x1cb/0x390 [l2cap] [ 2833.151999] [] hci_rx_task+0x2ab/0x450 [bluetooth] [ 2833.151999] [] tasklet_action+0xcb/0xe0 [ 2833.151999] [] __do_softirq+0xae/0x150 [ 2833.151999] [] call_softirq+0x1c/0x30 [ 2833.151999] [] do_softirq+0x75/0xb0 [ 2833.151999] [] irq_exit+0x8d/0xa0 [ 2833.151999] [] smp_apic_timer_interrupt+0x6b/0xa0 [ 2833.151999] [] apic_timer_interrupt+0x13/0x20 [ 2833.151999] [] cpu_idle+0x5a/0xb0 [ 2833.151999] [] rest_init+0xad/0xc0 [ 2833.151999] [] start_kernel+0x2dd/0x2e8 [ 2833.151999] [] x86_64_start_reservations+0xf6/0xfa [ 2833.151999] [] x86_64_start_kernel+0xe4/0xeb [ 2833.151999] irq event stamp: 731 [ 2833.151999] hardirqs last enabled at (731): [] local_bh_enable_ip+0x82/0xe0 [ 2833.151999] hardirqs last disabled at (729): [] __do_softirq+0xce/0x150 [ 2833.151999] softirqs last enabled at (730): [] __do_softirq+0xfe/0x150 [ 2833.151999] softirqs last disabled at (711): [] call_softirq+0x1c/0x30 [ 2833.151999] [ 2833.151999] other info that might help us debug this: [ 2833.151999] 2 locks held by krfcommd/2306: [ 2833.151999] #0: (rfcomm_mutex){+.+.+.}, at: [] rfcomm_run+0x174/0xb20 [rfcomm] [ 2833.151999] #1: (&(&d->lock)->rlock){+.+...}, at: [] rfcomm_dlc_accept+0x53/0x100 [rfcomm] [ 2833.151999] [ 2833.151999] stack backtrace: [ 2833.151999] Pid: 2306, comm: krfcommd Tainted: G W 2.6.36-rc3 #2 [ 2833.151999] Call Trace: [ 2833.151999] [] print_usage_bug+0x171/0x180 [ 2833.151999] [] mark_lock+0x333/0x400 [ 2833.151999] [] __lock_acquire+0x63a/0x1560 [ 2833.151999] [] ? __lock_acquire+0xb25/0x1560 [ 2833.151999] [] lock_acquire+0x5a/0x70 [ 2833.151999] [] ? rfcomm_sk_state_change+0x46/0x170 [rfcomm] [ 2833.151999] [] _raw_spin_lock+0x2c/0x40 [ 2833.151999] [] ? rfcomm_sk_state_change+0x46/0x170 [rfcomm] [ 2833.151999] [] rfcomm_sk_state_change+0x46/0x170 [rfcomm] [ 2833.151999] [] rfcomm_dlc_accept+0x69/0x100 [rfcomm] [ 2833.151999] [] rfcomm_check_accept+0x59/0xd0 [rfcomm] [ 2833.151999] [] rfcomm_recv_frame+0x9fb/0x1320 [rfcomm] [ 2833.151999] [] ? _raw_spin_unlock_irqrestore+0x3b/0x60 [ 2833.151999] [] ? trace_hardirqs_on_caller+0x13d/0x180 [ 2833.151999] [] ? trace_hardirqs_on+0xd/0x10 [ 2833.151999] [] rfcomm_run+0x221/0xb20 [rfcomm] [ 2833.151999] [] ? schedule+0x287/0x780 [ 2833.151999] [] ? rfcomm_run+0x0/0xb20 [rfcomm] [ 2833.151999] [] kthread+0x96/0xa0 [ 2833.151999] [] kernel_thread_helper+0x4/0x10 [ 2833.151999] [] ? restore_args+0x0/0x30 [ 2833.151999] [] ? kthread+0x0/0xa0 [ 2833.151999] [] ? kernel_thread_helper+0x0/0x10 Signed-off-by: Gustavo F. Padovan --- net/bluetooth/rfcomm/sock.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/net/bluetooth/rfcomm/sock.c b/net/bluetooth/rfcomm/sock.c index 44a6232..194b3a0 100644 --- a/net/bluetooth/rfcomm/sock.c +++ b/net/bluetooth/rfcomm/sock.c @@ -82,11 +82,14 @@ static void rfcomm_sk_data_ready(struct rfcomm_dlc *d, struct sk_buff *skb) static void rfcomm_sk_state_change(struct rfcomm_dlc *d, int err) { struct sock *sk = d->owner, *parent; + unsigned long flags; + if (!sk) return; BT_DBG("dlc %p state %ld err %d", d, d->state, err); + local_irq_save(flags); bh_lock_sock(sk); if (err) @@ -108,6 +111,7 @@ static void rfcomm_sk_state_change(struct rfcomm_dlc *d, int err) } bh_unlock_sock(sk); + local_irq_restore(flags); if (parent && sock_flag(sk, SOCK_ZAPPED)) { /* We have to drop DLC lock here, otherwise -- cgit v1.1 From b0239c80fe89d5832a68a0f3121a9d5ec9fb763e Mon Sep 17 00:00:00 2001 From: "Gustavo F. Padovan" Date: Wed, 8 Sep 2010 14:59:44 -0300 Subject: Revert "Bluetooth: Don't accept ConfigReq if we aren't in the BT_CONFIG state" This reverts commit 8cb8e6f1684be13b51f8429b15f39c140326b327. That commit introduced a regression with the Bluetooth Profile Tuning Suite(PTS), Reverting this make sure that L2CAP is in a qualificable state. Signed-off-by: Gustavo F. Padovan --- net/bluetooth/l2cap.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c index f2062aa..44a8fb0 100644 --- a/net/bluetooth/l2cap.c +++ b/net/bluetooth/l2cap.c @@ -3089,14 +3089,8 @@ static inline int l2cap_config_req(struct l2cap_conn *conn, struct l2cap_cmd_hdr if (!sk) return -ENOENT; - 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); + if (sk->sk_state == BT_DISCONN) goto unlock; - } /* Reject if config buffer is too small. */ len = cmd_len - sizeof(*req); -- cgit v1.1 From e454c844644683571617896ab2a4ce0109c1943e Mon Sep 17 00:00:00 2001 From: "Gustavo F. Padovan" Date: Tue, 21 Sep 2010 16:31:11 -0300 Subject: Bluetooth: Fix deadlock in the ERTM logic The Enhanced Retransmission Mode(ERTM) is a realiable mode of operation of the Bluetooth L2CAP layer. Think on it like a simplified version of TCP. The problem we were facing here was a deadlock. ERTM uses a backlog queue to queue incomimg packets while the user is helding the lock. At some moment the sk_sndbuf can be exceeded and we can't alloc new skbs then the code sleep with the lock to wait for memory, that stalls the ERTM connection once we can't read the acknowledgements packets in the backlog queue to free memory and make the allocation of outcoming skb successful. This patch actually affect all users of bt_skb_send_alloc(), i.e., all L2CAP modes and SCO. We are safe against socket states changes or channels deletion while the we are sleeping wait memory. Checking for the sk->sk_err and sk->sk_shutdown make the code safe, since any action that can leave the socket or the channel in a not usable state set one of the struct members at least. Then we can check both of them when getting the lock again and return with the proper error if something unexpected happens. Signed-off-by: Gustavo F. Padovan Signed-off-by: Ulisses Furquim --- include/net/bluetooth/bluetooth.h | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h index 27a902d..30fce01 100644 --- a/include/net/bluetooth/bluetooth.h +++ b/include/net/bluetooth/bluetooth.h @@ -161,12 +161,30 @@ static inline struct sk_buff *bt_skb_send_alloc(struct sock *sk, unsigned long l { struct sk_buff *skb; + release_sock(sk); if ((skb = sock_alloc_send_skb(sk, len + BT_SKB_RESERVE, nb, err))) { skb_reserve(skb, BT_SKB_RESERVE); bt_cb(skb)->incoming = 0; } + lock_sock(sk); + + if (!skb && *err) + return NULL; + + *err = sock_error(sk); + if (*err) + goto out; + + if (sk->sk_shutdown) { + *err = -ECONNRESET; + goto out; + } return skb; + +out: + kfree_skb(skb); + return NULL; } int bt_err(__u16 code); -- cgit v1.1 From eaa71b318c5ed0cd1ac3182a533471dc5edf372d Mon Sep 17 00:00:00 2001 From: "Gustavo F. Padovan" Date: Mon, 4 Oct 2010 19:28:52 -0300 Subject: Bluetooth: Disallow to change L2CAP_OPTIONS values when connected L2CAP doesn't permit change like MTU, FCS, TxWindow values while the connection is alive, we can only set that before the connection/configuration process. That can lead to bugs in the L2CAP operation. Signed-off-by: Gustavo F. Padovan --- net/bluetooth/l2cap.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c index 44a8fb0..0b54b7d 100644 --- a/net/bluetooth/l2cap.c +++ b/net/bluetooth/l2cap.c @@ -1950,6 +1950,11 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname, char __us switch (optname) { case L2CAP_OPTIONS: + if (sk->sk_state == BT_CONNECTED) { + err = -EINVAL; + break; + } + opts.imtu = l2cap_pi(sk)->imtu; opts.omtu = l2cap_pi(sk)->omtu; opts.flush_to = l2cap_pi(sk)->flush_to; -- cgit v1.1