2010-05-10 17:39:40

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 1/2] Bluetooth: Remove max_tx and tx_window modules paramenter from L2CAP

From: Gustavo F. Padovan <[email protected]>

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/l2cap.c | 12 ++----------
1 files changed, 2 insertions(+), 10 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 673a368..a1b826f 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -60,8 +60,6 @@ static int enable_ertm = 1;
#else
static int enable_ertm = 0;
#endif
-static int max_transmit = L2CAP_DEFAULT_MAX_TX;
-static int tx_window = L2CAP_DEFAULT_TX_WINDOW;

static u32 l2cap_feat_mask = L2CAP_FEAT_FIXED_CHAN;
static u8 l2cap_fixed_chan[8] = { 0x02, };
@@ -808,9 +806,9 @@ static void l2cap_sock_init(struct sock *sk, struct sock *parent)
pi->mode = L2CAP_MODE_ERTM;
else
pi->mode = L2CAP_MODE_BASIC;
- pi->max_tx = max_transmit;
+ pi->max_tx = L2CAP_DEFAULT_MAX_TX;
pi->fcs = L2CAP_FCS_CRC16;
- pi->tx_win = tx_window;
+ pi->tx_win = L2CAP_DEFAULT_TX_WINDOW;
pi->sec_level = BT_SECURITY_LOW;
pi->role_switch = 0;
pi->force_reliable = 0;
@@ -4677,12 +4675,6 @@ module_exit(l2cap_exit);
module_param(enable_ertm, bool, 0644);
MODULE_PARM_DESC(enable_ertm, "Enable enhanced retransmission mode");

-module_param(max_transmit, uint, 0644);
-MODULE_PARM_DESC(max_transmit, "Max transmit value (default = 3)");
-
-module_param(tx_window, uint, 0644);
-MODULE_PARM_DESC(tx_window, "Transmission window size value (default = 63)");
-
MODULE_AUTHOR("Marcel Holtmann <[email protected]>");
MODULE_DESCRIPTION("Bluetooth L2CAP ver " VERSION);
MODULE_VERSION(VERSION);
--
1.7.1


2010-05-12 21:59:36

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH] Bluetooth: Stop ack_timer if ERTM enters in Local Busy or SREJ_SENT

The ack_timer is implemation specific, disabling it in such situation
avoids some potencial errors in the ERTM protocol.

Signed-off-by: Gustavo F. Padovan <[email protected]>
Reviewed-by: João Paulo Rechi Vita <[email protected]>
---
net/bluetooth/l2cap.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 2192b9c..b061108 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3633,6 +3633,8 @@ static int l2cap_push_rx_skb(struct sock *sk, struct sk_buff *skb, u16 control)

pi->conn_state |= L2CAP_CONN_RNR_SENT;

+ del_timer(&pi->ack_timer);
+
queue_work(_busy_wq, &pi->busy_work);

return err;
@@ -3882,6 +3884,8 @@ static inline int l2cap_data_channel_iframe(struct sock *sk, u16 rx_control, str
pi->conn_state |= L2CAP_CONN_SEND_PBIT;

l2cap_send_srejframe(sk, tx_seq);
+
+ del_timer(&pi->ack_timer);
}
return 0;

--
1.6.4.4

2010-05-12 18:36:48

by João Paulo Rechi Vita

[permalink] [raw]
Subject: Re: [l2cap 6/6] Bluetooth: Only check SAR bits if frame is I-frame

On Wed, May 12, 2010 at 00:05, Gustavo F. Padovan
<[email protected]> wrote:
> Signed-off-by: Gustavo F. Padovan <[email protected]>
> ---
> =C2=A0net/bluetooth/l2cap.c | =C2=A0 =C2=A02 +-
> =C2=A01 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index dbd43d1..2192b9c 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -4123,7 +4123,7 @@ static inline int l2cap_data_channel(struct l2cap_c=
onn *conn, u16 cid, struct sk
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (l2cap_check_fc=
s(pi, skb))
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0goto drop;
>
> - =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (__is_sar_start(con=
trol))
> + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 if (__is_sar_start(con=
trol) && __is_iframe(control))
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0len -=3D 2;
>
> =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (pi->fcs =3D=3D=
L2CAP_FCS_CRC16)
> --
> 1.6.4.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth=
" in
> the body of a message to [email protected]
> More majordomo info at =C2=A0http://vger.kernel.org/majordomo-info.html
>

The whole series looks good to me.

--=20
Jo=C3=A3o Paulo Rechi Vita
http://jprvita.wordpress.com/

2010-05-12 03:05:58

by Gustavo Padovan

[permalink] [raw]
Subject: [l2cap 6/6] Bluetooth: Only check SAR bits if frame is I-frame

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/l2cap.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index dbd43d1..2192b9c 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -4123,7 +4123,7 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
if (l2cap_check_fcs(pi, skb))
goto drop;

- if (__is_sar_start(control))
+ if (__is_sar_start(control) && __is_iframe(control))
len -= 2;

if (pi->fcs == L2CAP_FCS_CRC16)
--
1.6.4.4

2010-05-12 03:05:57

by Gustavo Padovan

[permalink] [raw]
Subject: [l2cap 5/6] Bluetooth: Check packet FCS earlier

This way, if FCS is enabled and the packet is corrupted, we just drop it
without read it len, which could be corrupted.

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/l2cap.c | 22 +++++++++++-----------
1 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 6b5226d..dbd43d1 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -4115,25 +4115,25 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
skb_pull(skb, 2);
len = skb->len;

+ /*
+ * We can just drop the corrupted I-frame here.
+ * Receiver will miss it and start proper recovery
+ * procedures and ask retransmission.
+ */
+ if (l2cap_check_fcs(pi, skb))
+ goto drop;
+
if (__is_sar_start(control))
len -= 2;

if (pi->fcs == L2CAP_FCS_CRC16)
len -= 2;

- /*
- * We can just drop the corrupted I-frame here.
- * Receiver will miss it and start proper recovery
- * procedures and ask retransmission.
- */
if (len > pi->mps) {
l2cap_send_disconn_req(pi->conn, sk);
goto drop;
}

- if (l2cap_check_fcs(pi, skb))
- goto drop;
-
req_seq = __get_reqseq(control);
req_seq_offset = (req_seq - pi->expected_ack_seq) % 64;
if (req_seq_offset < 0)
@@ -4173,6 +4173,9 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
skb_pull(skb, 2);
len = skb->len;

+ if (l2cap_check_fcs(pi, skb))
+ goto drop;
+
if (__is_sar_start(control))
len -= 2;

@@ -4182,9 +4185,6 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
if (len > pi->mps || len < 4 || __is_sframe(control))
goto drop;

- if (l2cap_check_fcs(pi, skb))
- goto drop;
-
tx_seq = __get_txseq(control);

if (pi->expected_tx_seq == tx_seq)
--
1.6.4.4

2010-05-12 03:05:56

by Gustavo Padovan

[permalink] [raw]
Subject: [l2cap 4/6] Bluetooth: Fix ERTM vars increment

They should be modulo 64 ;)

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/l2cap.c | 7 ++++---
1 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 784bbed..6b5226d 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3746,7 +3746,7 @@ static void l2cap_check_srej_gap(struct sock *sk, u8 tx_seq)
l2cap_ertm_reassembly_sdu(sk, skb, control);
l2cap_pi(sk)->buffer_seq_srej =
(l2cap_pi(sk)->buffer_seq_srej + 1) % 64;
- tx_seq++;
+ tx_seq = (tx_seq + 1) % 64;
}
}

@@ -3782,10 +3782,11 @@ static void l2cap_send_srejframe(struct sock *sk, u8 tx_seq)
l2cap_send_sframe(pi, control);

new = kzalloc(sizeof(struct srej_list), GFP_ATOMIC);
- new->tx_seq = pi->expected_tx_seq++;
+ new->tx_seq = pi->expected_tx_seq;
+ pi->expected_tx_seq = (pi->expected_tx_seq + 1) % 64;
list_add_tail(&new->list, SREJ_LIST(sk));
}
- pi->expected_tx_seq++;
+ pi->expected_tx_seq = (pi->expected_tx_seq + 1) % 64;
}

static inline int l2cap_data_channel_iframe(struct sock *sk, u16 rx_control, struct sk_buff *skb)
--
1.6.4.4

2010-05-12 03:05:55

by Gustavo Padovan

[permalink] [raw]
Subject: [l2cap 3/6] Bluetooth: Check skb_clone return to avoid NULL dereference

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/l2cap.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 93667ac..784bbed 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -1337,6 +1337,8 @@ static int l2cap_streaming_send(struct sock *sk)

while ((skb = sk->sk_send_head)) {
tx_skb = skb_clone(skb, GFP_ATOMIC);
+ if (!tx_skb)
+ break;

control = get_unaligned_le16(tx_skb->data + L2CAP_HDR_SIZE);
control |= pi->next_tx_seq << L2CAP_CTRL_TXSEQ_SHIFT;
@@ -1422,6 +1424,8 @@ static int l2cap_ertm_send(struct sock *sk)
}

tx_skb = skb_clone(skb, GFP_ATOMIC);
+ if (!tx_skb)
+ break;

bt_cb(skb)->retries++;

--
1.6.4.4

2010-05-12 03:05:54

by Gustavo Padovan

[permalink] [raw]
Subject: [l2cap 2/6] Bluetooth: Fix drop of packets with invalid req_seq/tx_seq

We can't use an unsigned var since we are expecting negatives value
there too.

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/l2cap.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index a7e14c0..93667ac 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3790,7 +3790,7 @@ static inline int l2cap_data_channel_iframe(struct sock *sk, u16 rx_control, str
u8 tx_seq = __get_txseq(rx_control);
u8 req_seq = __get_reqseq(rx_control);
u8 sar = rx_control >> L2CAP_CTRL_SAR_SHIFT;
- u8 tx_seq_offset, expected_tx_seq_offset;
+ int tx_seq_offset, expected_tx_seq_offset;
int num_to_ack = (pi->tx_win/6) + 1;
int err = 0;

@@ -4075,7 +4075,8 @@ static inline int l2cap_data_channel(struct l2cap_conn *conn, u16 cid, struct sk
struct sock *sk;
struct l2cap_pinfo *pi;
u16 control, len;
- u8 tx_seq, req_seq, next_tx_seq_offset, req_seq_offset;
+ u8 tx_seq, req_seq;
+ int next_tx_seq_offset, req_seq_offset;

sk = l2cap_get_chan_by_scid(&conn->chan_list, cid);
if (!sk) {
--
1.6.4.4

2010-05-12 03:05:53

by Gustavo Padovan

[permalink] [raw]
Subject: [l2cap 1/6] Bluetooth: Tweaks to l2cap_send_i_or_rr_or_rnr() flow

l2cap_send_sframe() already set the F-bit if we set L2CAP_CONN_SEND_FBIT
and unset L2CAP_CONN_SEND_FBIT after send the F-bit.

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/l2cap.c | 3 +--
1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index e8c8f8b..a7e14c0 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -3364,10 +3364,9 @@ static inline void l2cap_send_i_or_rr_or_rnr(struct sock *sk)
control |= pi->buffer_seq << L2CAP_CTRL_REQSEQ_SHIFT;

if (pi->conn_state & L2CAP_CONN_LOCAL_BUSY) {
- control |= L2CAP_SUPER_RCV_NOT_READY | L2CAP_CTRL_FINAL;
+ control |= L2CAP_SUPER_RCV_NOT_READY;
l2cap_send_sframe(pi, control);
pi->conn_state |= L2CAP_CONN_RNR_SENT;
- pi->conn_state &= ~L2CAP_CONN_SEND_FBIT;
}

if (pi->conn_state & L2CAP_CONN_REMOTE_BUSY && pi->unacked_frames > 0)
--
1.6.4.4

2010-05-10 17:39:41

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 2/2] Bluetooth: Remove L2CAP Extended Features from Kconfig

From: Gustavo F. Padovan <[email protected]>

This reverts commit 84fb0a6334af0ccad3544f6972c055d90fbb9fbe
One can use other mechanisms to enable L2CAP Extended Features.

Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/Kconfig | 13 -------------
net/bluetooth/l2cap.c | 4 ----
2 files changed, 0 insertions(+), 17 deletions(-)

diff --git a/net/bluetooth/Kconfig b/net/bluetooth/Kconfig
index ee3b304..ed37168 100644
--- a/net/bluetooth/Kconfig
+++ b/net/bluetooth/Kconfig
@@ -43,19 +43,6 @@ config BT_L2CAP
Say Y here to compile L2CAP support into the kernel or say M to
compile it as module (l2cap).

-config BT_L2CAP_EXT_FEATURES
- bool "L2CAP Extended Features support (EXPERIMENTAL)"
- depends on BT_L2CAP && EXPERIMENTAL
- help
- This option enables the L2CAP Extended Features support. These
- new features include the Enhanced Retransmission and Streaming
- Modes, the Frame Check Sequence (FCS), and Segmentation and
- Reassembly (SAR) for L2CAP packets. They are a required for the
- new Alternate MAC/PHY and the Bluetooth Medical Profile.
-
- You should say N unless you know what you are doing. Note that
- this is in an experimental state yet.
-
config BT_SCO
tristate "SCO links support"
depends on BT
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index a1b826f..e8c8f8b 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -55,11 +55,7 @@

#define VERSION "2.14"

-#ifdef CONFIG_BT_L2CAP_EXT_FEATURES
-static int enable_ertm = 1;
-#else
static int enable_ertm = 0;
-#endif

static u32 l2cap_feat_mask = L2CAP_FEAT_FIXED_CHAN;
static u8 l2cap_fixed_chan[8] = { 0x02, };
--
1.7.1