fix race condition
Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/l2cap.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index ca4d3b4..355f375 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -175,12 +175,14 @@ static inline void __l2cap_chan_link(struct l2cap_chan_list *l, struct sock *sk)
{
sock_hold(sk);
+ write_lock_bh(&l->lock);
if (l->head)
l2cap_pi(l->head)->prev_c = sk;
l2cap_pi(sk)->next_c = l->head;
l2cap_pi(sk)->prev_c = NULL;
l->head = sk;
+ write_unlock_bh(&l->lock);
}
static inline void l2cap_chan_unlink(struct l2cap_chan_list *l, struct sock *sk)
--
1.6.0.6
On Sat, Feb 28, 2009 at 6:33 PM, Marcel Holtmann <[email protected]> wrot=
e:
> Hi Gustavo,
>
>> fix race condition
>>
>> Signed-off-by: Gustavo F. Padovan <[email protected]>
>> ---
>> =A0net/bluetooth/l2cap.c | =A0 =A02 ++
>> =A01 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>> index ca4d3b4..355f375 100644
>> --- a/net/bluetooth/l2cap.c
>> +++ b/net/bluetooth/l2cap.c
>> @@ -175,12 +175,14 @@ static inline void __l2cap_chan_link(struct l2cap_=
chan_list *l, struct sock *sk)
>> =A0{
>> =A0 =A0 =A0 sock_hold(sk);
>>
>> + =A0 =A0 write_lock_bh(&l->lock);
>> =A0 =A0 =A0 if (l->head)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 l2cap_pi(l->head)->prev_c =3D sk;
>>
>> =A0 =A0 =A0 l2cap_pi(sk)->next_c =3D l->head;
>> =A0 =A0 =A0 l2cap_pi(sk)->prev_c =3D NULL;
>> =A0 =A0 =A0 l->head =3D sk;
>> + =A0 =A0 write_unlock_bh(&l->lock);
>> =A0}
>
> a general rule of thumb is that function starting with __ are the ones
> that don't use any kind of locking.
Ok. I noted this after send the patch. :-(
>
> Regards
>
> Marcel
>
>
>
--=20
Gustavo F. Padovan
Computer Engineering Student
Institute of Computing - IC
University of Campinas - UNICAMP
email: [email protected]
gtalk: [email protected]
mobile: +55 19 81030803
Hi Gustavo,
> fix race condition
>
> Signed-off-by: Gustavo F. Padovan <[email protected]>
> ---
> net/bluetooth/l2cap.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index ca4d3b4..355f375 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -175,12 +175,14 @@ static inline void __l2cap_chan_link(struct l2cap_chan_list *l, struct sock *sk)
> {
> sock_hold(sk);
>
> + write_lock_bh(&l->lock);
> if (l->head)
> l2cap_pi(l->head)->prev_c = sk;
>
> l2cap_pi(sk)->next_c = l->head;
> l2cap_pi(sk)->prev_c = NULL;
> l->head = sk;
> + write_unlock_bh(&l->lock);
> }
a general rule of thumb is that function starting with __ are the ones
that don't use any kind of locking.
Regards
Marcel
Hi Gustavo,
> Use macros instead of hardcoded numbers
combine this with your previous patch and get me a proper commit
message.
Regards
Marcel
Hi Gustavo,
you can keep Bluetooth patches on linux-bluetooth only since they have
to go via bluetooth-testing.git first.
You are missing a commit message before the signed-off-by line. I am
serious about that every kernel patch has a proper commit message in
human readable text. No matter how simple the patch might look.
> Signed-off-by: Gustavo F. Padovan <[email protected]>
> ---
> include/net/bluetooth/l2cap.h | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
> index f566aa1..e7bf331 100644
> --- a/include/net/bluetooth/l2cap.h
> +++ b/include/net/bluetooth/l2cap.h
> @@ -118,6 +118,13 @@ struct l2cap_conn_rsp {
> #define L2CAP_CS_AUTHEN_PEND 0x0001
> #define L2CAP_CS_AUTHOR_PEND 0x0002
>
> +
> +/* cid channel type */
Use /* channel identifier */ here.
> +#define L2CAP_CID_SIGNALING 0x0001
> +#define L2CAP_CID_CONN_LESS 0x0002
> +#define L2CAP_CID_DYN_START 0x0040
> +#define L2CAP_CID_DYN_END 0xffff
> +
And put these above /* connect result */ defs.
Regards
Marcel
On Sat, Feb 28, 2009 at 2:02 AM, Gustavo F. Padovan
<[email protected]> wrote:
> fix race condition
>
> Signed-off-by: Gustavo F. Padovan <[email protected]>
> ---
> ?net/bluetooth/l2cap.c | ? ?2 ++
> ?1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
> index ca4d3b4..355f375 100644
> --- a/net/bluetooth/l2cap.c
> +++ b/net/bluetooth/l2cap.c
> @@ -175,12 +175,14 @@ static inline void __l2cap_chan_link(struct l2cap_chan_list *l, struct sock *sk)
> ?{
> ? ? ? ?sock_hold(sk);
>
> + ? ? ? write_lock_bh(&l->lock);
> ? ? ? ?if (l->head)
> ? ? ? ? ? ? ? ?l2cap_pi(l->head)->prev_c = sk;
>
> ? ? ? ?l2cap_pi(sk)->next_c = l->head;
> ? ? ? ?l2cap_pi(sk)->prev_c = NULL;
> ? ? ? ?l->head = sk;
> + ? ? ? write_unlock_bh(&l->lock);
> ?}
>
> ?static inline void l2cap_chan_unlink(struct l2cap_chan_list *l, struct sock *sk)
This patch is wrong, ignore it.
> --
> 1.6.0.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>
--
Gustavo F. Padovan
Computer Engineering Student
Institute of Computing - IC
University of Campinas - UNICAMP
email: [email protected]
gtalk: [email protected]
mobile: +55 19 81030803
Use macros instead of hardcoded numbers
Signed-off-by: Gustavo F. Padovan <[email protected]>
---
net/bluetooth/l2cap.c | 18 +++++++++---------
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
index 355f375..6181fc8 100644
--- a/net/bluetooth/l2cap.c
+++ b/net/bluetooth/l2cap.c
@@ -161,9 +161,9 @@ static inline struct sock *l2cap_get_chan_by_ident(struct l2cap_chan_list *l, u8
static u16 l2cap_alloc_cid(struct l2cap_chan_list *l)
{
- u16 cid = 0x0040;
+ u16 cid = L2CAP_CID_DYN_START;
- for (; cid < 0xffff; cid++) {
+ for (; cid < L2CAP_CID_DYN_END; cid++) {
if(!__l2cap_get_chan_by_scid(l, cid))
return cid;
}
@@ -217,13 +217,13 @@ static void __l2cap_chan_add(struct l2cap_conn *conn, struct sock *sk, struct so
l2cap_pi(sk)->scid = l2cap_alloc_cid(l);
} else if (sk->sk_type == SOCK_DGRAM) {
/* Connectionless socket */
- l2cap_pi(sk)->scid = 0x0002;
- l2cap_pi(sk)->dcid = 0x0002;
+ l2cap_pi(sk)->scid = L2CAP_CID_CONN_LESS;
+ l2cap_pi(sk)->dcid = L2CAP_CID_CONN_LESS;
l2cap_pi(sk)->omtu = L2CAP_DEFAULT_MTU;
} else {
/* Raw socket can send/recv signalling messages only */
- l2cap_pi(sk)->scid = 0x0001;
- l2cap_pi(sk)->dcid = 0x0001;
+ l2cap_pi(sk)->scid = L2CAP_CID_SIGNALING;
+ l2cap_pi(sk)->dcid = L2CAP_CID_SIGNALING;
l2cap_pi(sk)->omtu = L2CAP_DEFAULT_MTU;
}
@@ -1600,7 +1600,7 @@ static struct sk_buff *l2cap_build_cmd(struct l2cap_conn *conn,
lh = (struct l2cap_hdr *) skb_put(skb, L2CAP_HDR_SIZE);
lh->len = cpu_to_le16(L2CAP_CMD_HDR_SIZE + dlen);
- lh->cid = cpu_to_le16(0x0001);
+ lh->cid = cpu_to_le16(L2CAP_CID_SIGNALING);
cmd = (struct l2cap_cmd_hdr *) skb_put(skb, L2CAP_CMD_HDR_SIZE);
cmd->code = code;
@@ -2422,11 +2422,11 @@ static void l2cap_recv_frame(struct l2cap_conn *conn, struct sk_buff *skb)
BT_DBG("len %d, cid 0x%4.4x", len, cid);
switch (cid) {
- case 0x0001:
+ case L2CAP_CID_SIGNALING:
l2cap_sig_channel(conn, skb);
break;
- case 0x0002:
+ case L2CAP_CID_CONN_LESS:
psm = get_unaligned((__le16 *) skb->data);
skb_pull(skb, 2);
l2cap_conless_channel(conn, psm, skb);
--
1.6.0.6
Signed-off-by: Gustavo F. Padovan <[email protected]>
---
include/net/bluetooth/l2cap.h | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index f566aa1..e7bf331 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -118,6 +118,13 @@ struct l2cap_conn_rsp {
#define L2CAP_CS_AUTHEN_PEND 0x0001
#define L2CAP_CS_AUTHOR_PEND 0x0002
+
+/* cid channel type */
+#define L2CAP_CID_SIGNALING 0x0001
+#define L2CAP_CID_CONN_LESS 0x0002
+#define L2CAP_CID_DYN_START 0x0040
+#define L2CAP_CID_DYN_END 0xffff
+
struct l2cap_conf_req {
__le16 dcid;
__le16 flags;
--
1.6.0.6