[Patch] Keep rfcomm device in list until it's freed

[Patch] Keep rfcomm device in list until it's freed

[Ville Tervo]

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

Hi,

Here is patch for rfcomm to keep rfcomm device in list until it's really
unused.

-- 
Ville

[-- Attachment #2: 20070601_0001-Bluetooth-Keep-rfcomm_dev-in-list-until-it-is-free.patch --]
[-- Type: text/x-diff, Size: 3341 bytes --]

>From 9262ef03523c5d80418ad084d82b839127fc1846 Mon Sep 17 00:00:00 2001
From: Ville Tervo <ville.tervo@nokia.com>
Date: Fri, 1 Jun 2007 13:56:42 +0300
Subject: [PATCH 1/1] [Bluetooth] Keep rfcomm_dev in list until it is freed

This patch changes rfcomm tty release process so that tty is kept in list until
it is really freed. New device flag is introdused to keep track of released
ttys.

Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
---
 include/net/bluetooth/rfcomm.h |    1 +
 net/bluetooth/rfcomm/tty.c     |   25 +++++++++++++++++++------
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h
index 3c563f0..25aa575 100644
--- a/include/net/bluetooth/rfcomm.h
+++ b/include/net/bluetooth/rfcomm.h
@@ -323,6 +323,7 @@ int  rfcomm_connect_ind(struct rfcomm_session *s, u8 channel, struct rfcomm_dlc
 #define RFCOMM_RELEASE_ONHUP  1
 #define RFCOMM_HANGUP_NOW     2
 #define RFCOMM_TTY_ATTACHED   3
+#define RFCOMM_TTY_RELEASED   4
 
 struct rfcomm_dev_req {
 	s16      dev_id;
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index b2b1cce..86d630e 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -95,6 +95,10 @@ static void rfcomm_dev_destruct(struct rfcomm_dev *dev)
 
 	BT_DBG("dev %p dlc %p", dev, dlc);
 
+	write_lock_bh(&rfcomm_dev_lock);
+	list_del_init(&dev->list);
+	write_unlock_bh(&rfcomm_dev_lock);
+
 	rfcomm_dlc_lock(dlc);
 	/* Detach DLC if it's owned by this dev */
 	if (dlc->owner == dev)
@@ -156,6 +160,10 @@ static inline struct rfcomm_dev *rfcomm_dev_get(int id)
 	read_lock(&rfcomm_dev_lock);
 
 	dev = __rfcomm_dev_get(id);
+
+	if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
+		dev = NULL;
+
 	if (dev)
 		rfcomm_dev_hold(dev);
 
@@ -265,6 +273,12 @@ out:
 
 	dev->tty_dev = tty_register_device(rfcomm_tty_driver, dev->id, NULL);
 
+	if (IS_ERR(dev->tty_dev)) {
+		list_del(&dev->list);
+		kfree(dev);
+		return PTR_ERR(dev->tty_dev);
+	}
+
 	return dev->id;
 }
 
@@ -272,10 +286,7 @@ static void rfcomm_dev_del(struct rfcomm_dev *dev)
 {
 	BT_DBG("dev %p", dev);
 
-	write_lock_bh(&rfcomm_dev_lock);
-	list_del_init(&dev->list);
-	write_unlock_bh(&rfcomm_dev_lock);
-
+	set_bit(RFCOMM_TTY_RELEASED, &dev->flags);
 	rfcomm_dev_put(dev);
 }
 
@@ -329,7 +340,7 @@ static int rfcomm_create_dev(struct sock *sk, void __user *arg)
 	if (copy_from_user(&req, arg, sizeof(req)))
 		return -EFAULT;
 
-	BT_DBG("sk %p dev_id %id flags 0x%x", sk, req.dev_id, req.flags);
+	BT_DBG("sk %p dev_id %d flags 0x%x", sk, req.dev_id, req.flags);
 
 	if (req.flags != NOCAP_FLAGS && !capable(CAP_NET_ADMIN))
 		return -EPERM;
@@ -370,7 +381,7 @@ static int rfcomm_release_dev(void __user *arg)
 	if (copy_from_user(&req, arg, sizeof(req)))
 		return -EFAULT;
 
-	BT_DBG("dev_id %id flags 0x%x", req.dev_id, req.flags);
+	BT_DBG("dev_id %d flags 0x%x", req.dev_id, req.flags);
 
 	if (!(dev = rfcomm_dev_get(req.dev_id)))
 		return -ENODEV;
@@ -415,6 +426,8 @@ static int rfcomm_get_dev_list(void __user *arg)
 
 	list_for_each(p, &rfcomm_dev_list) {
 		struct rfcomm_dev *dev = list_entry(p, struct rfcomm_dev, list);
+		if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
+			continue;
 		(di + n)->id      = dev->id;
 		(di + n)->flags   = dev->flags;
 		(di + n)->state   = dev->dlc->state;
-- 
1.5.1.1

Re: [Bluez-devel] [Patch] Keep rfcomm device in list until it's freed

[Marcel Holtmann]

Hi Ville,

> Here is patch for rfcomm to keep rfcomm device in list until it's really
> unused.

        dev = __rfcomm_dev_get(id);
+
+       if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
+               dev = NULL;
+
        if (dev)
                rfcomm_dev_hold(dev);

a test_bit() and then return NULL at the beginning makes more sense. No
need to take the lock since test_bit() is atomic anyway.

Regards

Marcel



-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Patch] Keep rfcomm device in list until it's freed

[Ville Tervo]

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

Hi Marcel,

> > Here is patch for rfcomm to keep rfcomm device in list until it's really
> > unused.
> 
>         dev = __rfcomm_dev_get(id);
> +
> +       if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
> +               dev = NULL;
> +
>         if (dev)
>                 rfcomm_dev_hold(dev);
> 
> a test_bit() and then return NULL at the beginning makes more sense. No
> need to take the lock since test_bit() is atomic anyway.

How do I get flags then? Function only gets device id.

I noticed another bug. If __rfcomm_dev_get returns null we end up using
NULL pointer. Fixed version attached.

-- 
Ville

[-- Attachment #2: 20070604-0001-Bluetooth-Keep-rfcomm_dev-in-list-until-it-is-free.patch --]
[-- Type: text/x-diff, Size: 3551 bytes --]

>From 2a97626c89dea1e9850e002320b4e3624a544cef Mon Sep 17 00:00:00 2001
From: Ville Tervo <ville.tervo@nokia.com>
Date: Mon, 4 Jun 2007 11:30:56 +0300
Subject: [PATCH 1/1] [Bluetooth] Keep rfcomm_dev in list until it is freed

This patch changes rfcomm tty release process so that tty is kept in list until
it is really freed. New device flag is introdused to keep track of released
ttys.

Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
---
 include/net/bluetooth/rfcomm.h |    1 +
 net/bluetooth/rfcomm/tty.c     |   27 ++++++++++++++++++++-------
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h
index 3c563f0..25aa575 100644
--- a/include/net/bluetooth/rfcomm.h
+++ b/include/net/bluetooth/rfcomm.h
@@ -323,6 +323,7 @@ int  rfcomm_connect_ind(struct rfcomm_session *s, u8 channel, struct rfcomm_dlc
 #define RFCOMM_RELEASE_ONHUP  1
 #define RFCOMM_HANGUP_NOW     2
 #define RFCOMM_TTY_ATTACHED   3
+#define RFCOMM_TTY_RELEASED   4
 
 struct rfcomm_dev_req {
 	s16      dev_id;
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index b2b1cce..b663452 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -41,7 +41,7 @@
 #include <net/bluetooth/hci_core.h>
 #include <net/bluetooth/rfcomm.h>
 
-#ifndef CONFIG_BT_RFCOMM_DEBUG
+#ifdef CONFIG_BT_RFCOMM_DEBUG
 #undef  BT_DBG
 #define BT_DBG(D...)
 #endif
@@ -95,6 +95,10 @@ static void rfcomm_dev_destruct(struct rfcomm_dev *dev)
 
 	BT_DBG("dev %p dlc %p", dev, dlc);
 
+	write_lock_bh(&rfcomm_dev_lock);
+	list_del_init(&dev->list);
+	write_unlock_bh(&rfcomm_dev_lock);
+
 	rfcomm_dlc_lock(dlc);
 	/* Detach DLC if it's owned by this dev */
 	if (dlc->owner == dev)
@@ -156,6 +160,10 @@ static inline struct rfcomm_dev *rfcomm_dev_get(int id)
 	read_lock(&rfcomm_dev_lock);
 
 	dev = __rfcomm_dev_get(id);
+
+	if (dev && test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
+		dev = NULL;
+
 	if (dev)
 		rfcomm_dev_hold(dev);
 
@@ -265,6 +273,12 @@ out:
 
 	dev->tty_dev = tty_register_device(rfcomm_tty_driver, dev->id, NULL);
 
+	if (IS_ERR(dev->tty_dev)) {
+		list_del(&dev->list);
+		kfree(dev);
+		return PTR_ERR(dev->tty_dev);
+	}
+
 	return dev->id;
 }
 
@@ -272,10 +286,7 @@ static void rfcomm_dev_del(struct rfcomm_dev *dev)
 {
 	BT_DBG("dev %p", dev);
 
-	write_lock_bh(&rfcomm_dev_lock);
-	list_del_init(&dev->list);
-	write_unlock_bh(&rfcomm_dev_lock);
-
+	set_bit(RFCOMM_TTY_RELEASED, &dev->flags);
 	rfcomm_dev_put(dev);
 }
 
@@ -329,7 +340,7 @@ static int rfcomm_create_dev(struct sock *sk, void __user *arg)
 	if (copy_from_user(&req, arg, sizeof(req)))
 		return -EFAULT;
 
-	BT_DBG("sk %p dev_id %id flags 0x%x", sk, req.dev_id, req.flags);
+	BT_DBG("sk %p dev_id %d flags 0x%x", sk, req.dev_id, req.flags);
 
 	if (req.flags != NOCAP_FLAGS && !capable(CAP_NET_ADMIN))
 		return -EPERM;
@@ -370,7 +381,7 @@ static int rfcomm_release_dev(void __user *arg)
 	if (copy_from_user(&req, arg, sizeof(req)))
 		return -EFAULT;
 
-	BT_DBG("dev_id %id flags 0x%x", req.dev_id, req.flags);
+	BT_DBG("dev_id %d flags 0x%x", req.dev_id, req.flags);
 
 	if (!(dev = rfcomm_dev_get(req.dev_id)))
 		return -ENODEV;
@@ -415,6 +426,8 @@ static int rfcomm_get_dev_list(void __user *arg)
 
 	list_for_each(p, &rfcomm_dev_list) {
 		struct rfcomm_dev *dev = list_entry(p, struct rfcomm_dev, list);
+		if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
+			continue;
 		(di + n)->id      = dev->id;
 		(di + n)->flags   = dev->flags;
 		(di + n)->state   = dev->dlc->state;
-- 
1.5.1.1

Re: [Bluez-devel] [Patch] Keep rfcomm device in list until it's freed

[Marcel Holtmann]

Hi Ville,

> > > Here is patch for rfcomm to keep rfcomm device in list until it's really
> > > unused.
> > 
> >         dev = __rfcomm_dev_get(id);
> > +
> > +       if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
> > +               dev = NULL;
> > +
> >         if (dev)
> >                 rfcomm_dev_hold(dev);
> > 
> > a test_bit() and then return NULL at the beginning makes more sense. No
> > need to take the lock since test_bit() is atomic anyway.
> 
> How do I get flags then? Function only gets device id.

good point. I overlooked that part.

> I noticed another bug. If __rfcomm_dev_get returns null we end up using
> NULL pointer. Fixed version attached.

Please remove this part:

-#ifndef CONFIG_BT_RFCOMM_DEBUG
+#ifdef CONFIG_BT_RFCOMM_DEBUG

And use this code:

	if (dev) {
		if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
			dev = NULL;
		else
	                rfcomm_dev_hold(dev);
	}

It makes it a little bit more readable and easier to understand what we
are doing there.

Regards

Marcel



-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [Patch] Keep rfcomm device in list until it's freed

[Ulisses Furquim]

Hi Marcel,

On 6/4/07, Marcel Holtmann <marcel@holtmann.org> wrote:
> Hi Ville,
>
> > > > Here is patch for rfcomm to keep rfcomm device in list until it's really
> > > > unused.
> > >
> > >         dev = __rfcomm_dev_get(id);
> > > +
> > > +       if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
> > > +               dev = NULL;
> > > +
> > >         if (dev)
> > >                 rfcomm_dev_hold(dev);
> > >
> > > a test_bit() and then return NULL at the beginning makes more sense. No
> > > need to take the lock since test_bit() is atomic anyway.
> >
> > How do I get flags then? Function only gets device id.
>
> good point. I overlooked that part.
>
> > I noticed another bug. If __rfcomm_dev_get returns null we end up using
> > NULL pointer. Fixed version attached.
>
> Please remove this part:
>
> -#ifndef CONFIG_BT_RFCOMM_DEBUG
> +#ifdef CONFIG_BT_RFCOMM_DEBUG
>
> And use this code:
>
>         if (dev) {
>                 if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
>                         dev = NULL;
>                 else
>                         rfcomm_dev_hold(dev);
>         }
>
> It makes it a little bit more readable and easier to understand what we
> are doing there.

Where is this patch? I couldn't find it. Does it solve that problem we
had with RFCOMM_HANGUP_NOW flag?

Regards,

-- Ulisses

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [Patch] Keep rfcomm device in list until it's freed

[Marcel Holtmann]

Hi Ulisses,

> > > > > Here is patch for rfcomm to keep rfcomm device in list until it's really
> > > > > unused.
> > > >
> > > >         dev = __rfcomm_dev_get(id);
> > > > +
> > > > +       if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
> > > > +               dev = NULL;
> > > > +
> > > >         if (dev)
> > > >                 rfcomm_dev_hold(dev);
> > > >
> > > > a test_bit() and then return NULL at the beginning makes more sense. No
> > > > need to take the lock since test_bit() is atomic anyway.
> > >
> > > How do I get flags then? Function only gets device id.
> >
> > good point. I overlooked that part.
> >
> > > I noticed another bug. If __rfcomm_dev_get returns null we end up using
> > > NULL pointer. Fixed version attached.
> >
> > Please remove this part:
> >
> > -#ifndef CONFIG_BT_RFCOMM_DEBUG
> > +#ifdef CONFIG_BT_RFCOMM_DEBUG
> >
> > And use this code:
> >
> >         if (dev) {
> >                 if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
> >                         dev = NULL;
> >                 else
> >                         rfcomm_dev_hold(dev);
> >         }
> >
> > It makes it a little bit more readable and easier to understand what we
> > are doing there.
> 
> Where is this patch? I couldn't find it. Does it solve that problem we
> had with RFCOMM_HANGUP_NOW flag?

it was attached to Ville's email. And it doesn't solve the issue with
setting RFCOMM_HANGUP_NOW flag, but that should already be solved and
the patch for that is upstream in 2.6.22-rc3.

Regards

Marcel



-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [Patch] Keep rfcomm device in list until it's freed

[Ulisses Furquim]

Hi Marcel,

On 6/4/07, Marcel Holtmann <marcel@holtmann.org> wrote:
> it was attached to Ville's email. And it doesn't solve the issue with

Hmm... sorry, but I don't see it. Which e-mail from Ville? Maybe I'm
having problems with the list?

Regards,

-- Ulisses

-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Patch] Keep rfcomm device in list until it's freed

[Ville Tervo]

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

On Mon, Jun 04, 2007 at 10:49:04AM +0200, ext Marcel Holtmann wrote:
> Hi Ville,
> 
> > > > Here is patch for rfcomm to keep rfcomm device in list until it's really
> > > > unused.
> > > 
> > >         dev = __rfcomm_dev_get(id);
> > > +
> > > +       if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
> > > +               dev = NULL;
> > > +
> > >         if (dev)
> > >                 rfcomm_dev_hold(dev);
> > > 
> > > a test_bit() and then return NULL at the beginning makes more sense. No
> > > need to take the lock since test_bit() is atomic anyway.
> > 
> > How do I get flags then? Function only gets device id.
> 
> good point. I overlooked that part.
> 
> > I noticed another bug. If __rfcomm_dev_get returns null we end up using
> > NULL pointer. Fixed version attached.
> 
> Please remove this part:
> 
> -#ifndef CONFIG_BT_RFCOMM_DEBUG
> +#ifdef CONFIG_BT_RFCOMM_DEBUG

Removed.

> 
> And use this code:
> 
> 	if (dev) {
> 		if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
> 			dev = NULL;
> 		else
> 	                rfcomm_dev_hold(dev);
> 	}
> 
> It makes it a little bit more readable and easier to understand what we
> are doing there.

Agreed. New version attached.

-- 
Ville

[-- Attachment #2: 20070604-0001-Bluetooth-Keep-rfcomm_dev-in-list-until-it-is-free.patch --]
[-- Type: text/x-diff, Size: 3432 bytes --]

>From ec26804b4233376633061bc462f0c3a77b4d0780 Mon Sep 17 00:00:00 2001
From: Ville Tervo <ville.tervo@nokia.com>
Date: Mon, 4 Jun 2007 13:25:09 +0300
Subject: [PATCH 1/1] [Bluetooth] Keep rfcomm_dev in list until it is freed

This patch changes rfcomm tty release process so that tty is kept in list until
it is really freed. New device flag is introdused to keep track of released
ttys.

Signed-off-by: Ville Tervo <ville.tervo@nokia.com>
---
 include/net/bluetooth/rfcomm.h |    1 +
 net/bluetooth/rfcomm/tty.c     |   30 ++++++++++++++++++++++--------
 2 files changed, 23 insertions(+), 8 deletions(-)

diff --git a/include/net/bluetooth/rfcomm.h b/include/net/bluetooth/rfcomm.h
index 3c563f0..25aa575 100644
--- a/include/net/bluetooth/rfcomm.h
+++ b/include/net/bluetooth/rfcomm.h
@@ -323,6 +323,7 @@ int  rfcomm_connect_ind(struct rfcomm_session *s, u8 channel, struct rfcomm_dlc
 #define RFCOMM_RELEASE_ONHUP  1
 #define RFCOMM_HANGUP_NOW     2
 #define RFCOMM_TTY_ATTACHED   3
+#define RFCOMM_TTY_RELEASED   4
 
 struct rfcomm_dev_req {
 	s16      dev_id;
diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index b2b1cce..12a84b5 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -95,6 +95,10 @@ static void rfcomm_dev_destruct(struct rfcomm_dev *dev)
 
 	BT_DBG("dev %p dlc %p", dev, dlc);
 
+	write_lock_bh(&rfcomm_dev_lock);
+	list_del_init(&dev->list);
+	write_unlock_bh(&rfcomm_dev_lock);
+
 	rfcomm_dlc_lock(dlc);
 	/* Detach DLC if it's owned by this dev */
 	if (dlc->owner == dev)
@@ -156,8 +160,13 @@ static inline struct rfcomm_dev *rfcomm_dev_get(int id)
 	read_lock(&rfcomm_dev_lock);
 
 	dev = __rfcomm_dev_get(id);
-	if (dev)
-		rfcomm_dev_hold(dev);
+
+	if (dev) {
+		if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
+			dev = NULL;
+		else
+			rfcomm_dev_hold(dev);
+	}
 
 	read_unlock(&rfcomm_dev_lock);
 
@@ -265,6 +274,12 @@ out:
 
 	dev->tty_dev = tty_register_device(rfcomm_tty_driver, dev->id, NULL);
 
+	if (IS_ERR(dev->tty_dev)) {
+		list_del(&dev->list);
+		kfree(dev);
+		return PTR_ERR(dev->tty_dev);
+	}
+
 	return dev->id;
 }
 
@@ -272,10 +287,7 @@ static void rfcomm_dev_del(struct rfcomm_dev *dev)
 {
 	BT_DBG("dev %p", dev);
 
-	write_lock_bh(&rfcomm_dev_lock);
-	list_del_init(&dev->list);
-	write_unlock_bh(&rfcomm_dev_lock);
-
+	set_bit(RFCOMM_TTY_RELEASED, &dev->flags);
 	rfcomm_dev_put(dev);
 }
 
@@ -329,7 +341,7 @@ static int rfcomm_create_dev(struct sock *sk, void __user *arg)
 	if (copy_from_user(&req, arg, sizeof(req)))
 		return -EFAULT;
 
-	BT_DBG("sk %p dev_id %id flags 0x%x", sk, req.dev_id, req.flags);
+	BT_DBG("sk %p dev_id %d flags 0x%x", sk, req.dev_id, req.flags);
 
 	if (req.flags != NOCAP_FLAGS && !capable(CAP_NET_ADMIN))
 		return -EPERM;
@@ -370,7 +382,7 @@ static int rfcomm_release_dev(void __user *arg)
 	if (copy_from_user(&req, arg, sizeof(req)))
 		return -EFAULT;
 
-	BT_DBG("dev_id %id flags 0x%x", req.dev_id, req.flags);
+	BT_DBG("dev_id %d flags 0x%x", req.dev_id, req.flags);
 
 	if (!(dev = rfcomm_dev_get(req.dev_id)))
 		return -ENODEV;
@@ -415,6 +427,8 @@ static int rfcomm_get_dev_list(void __user *arg)
 
 	list_for_each(p, &rfcomm_dev_list) {
 		struct rfcomm_dev *dev = list_entry(p, struct rfcomm_dev, list);
+		if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
+			continue;
 		(di + n)->id      = dev->id;
 		(di + n)->flags   = dev->flags;
 		(di + n)->state   = dev->dlc->state;
-- 
1.5.1.1

Re: [Bluez-devel] [Patch] Keep rfcomm device in list until it's freed

[Marcel Holtmann]

Hi Ville,

> > > > > Here is patch for rfcomm to keep rfcomm device in list until it's really
> > > > > unused.
> > > > 
> > > >         dev = __rfcomm_dev_get(id);
> > > > +
> > > > +       if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
> > > > +               dev = NULL;
> > > > +
> > > >         if (dev)
> > > >                 rfcomm_dev_hold(dev);
> > > > 
> > > > a test_bit() and then return NULL at the beginning makes more sense. No
> > > > need to take the lock since test_bit() is atomic anyway.
> > > 
> > > How do I get flags then? Function only gets device id.
> > 
> > good point. I overlooked that part.
> > 
> > > I noticed another bug. If __rfcomm_dev_get returns null we end up using
> > > NULL pointer. Fixed version attached.
> > 
> > Please remove this part:
> > 
> > -#ifndef CONFIG_BT_RFCOMM_DEBUG
> > +#ifdef CONFIG_BT_RFCOMM_DEBUG
> 
> Removed.
> 
> > 
> > And use this code:
> > 
> > 	if (dev) {
> > 		if (test_bit(RFCOMM_TTY_RELEASED, &dev->flags))
> > 			dev = NULL;
> > 		else
> > 	                rfcomm_dev_hold(dev);
> > 	}
> > 
> > It makes it a little bit more readable and easier to understand what we
> > are doing there.
> 
> Agreed. New version attached.

do you have a simple re-producer for it. I need to test this on my Quad
G5 before pushing this upstream.

Regards

Marcel



-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Patch] Keep rfcomm device in list until it's freed

[Ville Tervo]

Hi Marcel,

> Hi Ville,
> 
> do you have a simple re-producer for it. I need to test this on my Quad
> G5 before pushing this upstream.

Nope. Claudio said that he had some way to reproduce the bug with some
cvs version. I haven't tried that yet. I need to install some distro to
test machine first. However I tested this with N800 and it works fine on
arm.

1. utils/serial/port.c comment line 151 "g_io_channel_close"
2. comment the last line of utils/serial/test-serial : "serial.DisconnectService(device)"
call  "test-serial address channel"  the first time,  after establish
the connection press ctrl+c and execute the same command again

-- 
Ville

Re: [Bluez-devel] [Patch] Keep rfcomm device in list until it's freed

[Marcel Holtmann]

Hi Ville,

> > do you have a simple re-producer for it. I need to test this on my Quad
> > G5 before pushing this upstream.
> 
> Nope. Claudio said that he had some way to reproduce the bug with some
> cvs version. I haven't tried that yet. I need to install some distro to
> test machine first. However I tested this with N800 and it works fine on
> arm.

but I need to make sure that this is also working on SMP systems and all
other desktops.

Regards

Marcel



-------------------------------------------------------------------------
This SF.net email is sponsored by DB2 Express
Download DB2 Express C - the FREE version of DB2 express and take
control of your XML. No limits. Just data. Click to get it now.
http://sourceforge.net/powerbar/db2/
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel