[Bluez-devel] [PATCH] rfcomm_dev_del is called twice if RFCOMM_HANGUP_NOW flag is used

[Bluez-devel] [PATCH] rfcomm_dev_del is called twice if RFCOMM_HANGUP_NOW flag is used

[Ville Tervo]

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

Hi Marcel,

I noticed that rfcomm_dev_del() may be called twice if
RFCOMM_HANGUP_NOW flags is used with RFCOMMRELEASEDEV ioctl.

I made a patch that checks if dev is still in the device list before
calling deleting device.

The checking is done now in rfcomm_release_dev. Maybe better place would
be rfcomm_dev_del?

-- 
Ville

[-- Attachment #2: 20060824_rfcomm_tty_dev_double_del_fix.txt --]
[-- Type: text/plain, Size: 544 bytes --]

diff --git a/net/bluetooth/rfcomm/tty.c b/net/bluetooth/rfcomm/tty.c
index bd8d671..330760f 100644
--- a/net/bluetooth/rfcomm/tty.c
+++ b/net/bluetooth/rfcomm/tty.c
@@ -364,8 +364,13 @@ static int rfcomm_release_dev(void __use
 	if (req.flags & (1 << RFCOMM_HANGUP_NOW))
 		rfcomm_dlc_close(dev->dlc, 0);
 
-	rfcomm_dev_del(dev);
 	rfcomm_dev_put(dev);
+
+	/* dev might be deleted allready by rfcomm_dlc_close.
+	 * Check that device is still on the list. */
+	if ((dev = rfcomm_dev_get(req.dev_id)))
+		rfcomm_dev_del(dev);
+
 	return 0;
 }
 

[-- Attachment #3: Type: text/plain, Size: 373 bytes --]

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642

[-- Attachment #4: Type: text/plain, Size: 164 bytes --]

_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] rfcomm_dev_del is called twice if RFCOMM_HANGUP_NOW flag is used

[Ulisses Furquim]

Hi Ville,

On 8/24/06, Ville Tervo <ville.tervo@nokia.com> wrote:
> I noticed that rfcomm_dev_del() may be called twice if
> RFCOMM_HANGUP_NOW flags is used with RFCOMMRELEASEDEV ioctl.

I think you're right but we must have the RFCOMM_RELEASE_ONHUP bit set
on dev->flags also, right?

> I made a patch that checks if dev is still in the device list before
> calling deleting device.
> The checking is done now in rfcomm_release_dev. Maybe better place would
> be rfcomm_dev_del?

Actually calling list_del_init() on an item thas was already deleted
with list_del_init() won't hurt anyone but it seems we can mess up the
reference counting by calling rfcomm_dev_del() twice.

I think your patch doesn't completely solve the problem because
rfcomm_dev_get() will hold a reference to the device if we still
manage to find it on the list and we will have reference counting
problems too. Adding a call to rfcomm_dev_put() before the call to
rfcomm_dev_del() in your patch will keep the reference counting
correct, I guess. Something like this:

  if ((dev = rfcomm_dev_get(req.dev_id))) {
		rfcomm_dev_put(dev);
		rfcomm_dev_del(dev);
  }

What do you think, Marcel?

Best regards,

-- Ulisses

-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Re: [Bluez-devel] [PATCH] rfcomm_dev_del is called twice if RFCOMM_HANGUP_NOW flag is used

[Ville Tervo]

On Thu, Aug 24, 2006 at 03:29:04PM -0300, ext Ulisses Furquim wrote:
> Hi Ville,
> 
> On 8/24/06, Ville Tervo <ville.tervo@nokia.com> wrote:
> > I noticed that rfcomm_dev_del() may be called twice if
> > RFCOMM_HANGUP_NOW flags is used with RFCOMMRELEASEDEV ioctl.
> 
> I think you're right but we must have the RFCOMM_RELEASE_ONHUP bit set
> on dev->flags also, right?
> 

Yep that is right.

> > I made a patch that checks if dev is still in the device list before
> > calling deleting device.
> > The checking is done now in rfcomm_release_dev. Maybe better place would
> > be rfcomm_dev_del?
> 
> Actually calling list_del_init() on an item thas was already deleted
> with list_del_init() won't hurt anyone but it seems we can mess up the
> reference counting by calling rfcomm_dev_del() twice.
> 
> I think your patch doesn't completely solve the problem because
> rfcomm_dev_get() will hold a reference to the device if we still
> manage to find it on the list and we will have reference counting
> problems too. Adding a call to rfcomm_dev_put() before the call to
> rfcomm_dev_del() in your patch will keep the reference counting
> correct, I guess. Something like this:
> 
>   if ((dev = rfcomm_dev_get(req.dev_id))) {
> 		rfcomm_dev_put(dev);
> 		rfcomm_dev_del(dev);
>   }
> 

You are right.

-- 
Ville


-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
Bluez-devel mailing list
Bluez-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/bluez-devel