RE: [Bluez-devel] Rfcomm Use Count

Marcel Holtmann
2004-09-22T13:53:49Z

[-- Attachment #1: Type: text/plain, Size: 645 bytes --]

Hi Daryl,

> the correct approach seems to be:
>
> sk->sk_zapped = 1;
> rfcomm_sock_kill(sk);
>
> The problem is that rfcomm_sock_kill() must be called on an unlocked
> socket and I think that we will deadlock on a SMP machine or get some
> NULL pointer dereferences.

after more and more thinking about that problem I am almost sure that it
is right to call rfcomm_sock_kill() in the state change function. Anyway
we must do this on an unlocked socket. Here is my proposal for the final
patch, but we need real testing on this so that I can be sure that there
are no side effects. Can anyone test it on SMP or HT systems?

Regards

Marcel

[-- Attachment #2: patch --]
[-- Type: text/plain, Size: 2199 bytes --]

===== include/net/bluetooth/bluetooth.h 1.18 vs edited =====
--- 1.18/include/net/bluetooth/bluetooth.h 2004-07-04 17:27:14 +02:00
+++ edited/include/net/bluetooth/bluetooth.h 2004-09-21 01:26:06 +02:00
@@ -133,6 +133,7 @@
int bt_sock_wait_state(struct sock *sk, int state, unsigned long timeo);

void bt_accept_enqueue(struct sock *parent, struct sock *sk);
+void bt_accept_unlink(struct sock *sk);
struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock);

/* Skb helpers */
===== net/bluetooth/af_bluetooth.c 1.36 vs edited =====
--- 1.36/net/bluetooth/af_bluetooth.c 2004-07-13 15:39:15 +02:00
+++ edited/net/bluetooth/af_bluetooth.c 2004-09-21 01:26:17 +02:00
@@ -165,7 +165,7 @@
}
EXPORT_SYMBOL(bt_accept_enqueue);

-static void bt_accept_unlink(struct sock *sk)
+void bt_accept_unlink(struct sock *sk)
{
BT_DBG("sk %p state %d", sk, sk->sk_state);

@@ -174,6 +174,7 @@
bt_sk(sk)->parent = NULL;
sock_put(sk);
}
+EXPORT_SYMBOL(bt_accept_unlink);

struct sock *bt_accept_dequeue(struct sock *parent, struct socket *newsock)
{
===== net/bluetooth/l2cap.c 1.44 vs edited =====
--- 1.44/net/bluetooth/l2cap.c 2004-09-04 12:51:17 +02:00
+++ edited/net/bluetooth/l2cap.c 2004-09-21 01:23:41 +02:00
@@ -1005,9 +1005,10 @@
if (err)
sk->sk_err = err;

- if (parent)
+ if (parent) {
+ bt_accept_unlink(sk);
parent->sk_data_ready(parent, 0);
- else
+ } else
sk->sk_state_change(sk);
}

===== net/bluetooth/rfcomm/sock.c 1.29 vs edited =====
--- 1.29/net/bluetooth/rfcomm/sock.c 2004-06-04 02:41:47 +02:00
+++ edited/net/bluetooth/rfcomm/sock.c 2004-09-22 13:26:26 +02:00
@@ -97,17 +97,26 @@

if (err)
sk->sk_err = err;
+
sk->sk_state = d->state;

parent = bt_sk(sk)->parent;
- if (!parent) {
+ if (parent) {
+ if (d->state == BT_CLOSED) {
+ sk->sk_zapped = 1;
+ bt_accept_unlink(sk);
+ }
+ parent->sk_data_ready(parent, 0);
+ } else {
if (d->state == BT_CONNECTED)
rfcomm_session_getaddr(d->session, &bt_sk(sk)->src, NULL);
sk->sk_state_change(sk);
- } else
- parent->sk_data_ready(parent, 0);
+ }

bh_unlock_sock(sk);
+
+ if (parent && sk->sk_zapped)
+ rfcomm_sock_kill(sk);
}

/* ---- Socket functions ---- */