rfcomm_dev_del is called twice if RFCOMM_HANGUP_NOW flag is used

PATCH In review Ville Tervo 2 participants 3 messages Aug 24, 2006 – Aug 25, 2006

Subsystems (2)

Bluetooth drivers Bluetooth

lore

Share & tools

lore

Ville Tervo

author

2006-08-24 15:09 UTC #0 lore raw

PATCH 1/1 rfcomm_dev_del is called twice if RFCOMM_HANGUP_NOW flag is used

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?

Signature · 1 line

Ville

net/bluetooth/rfcomm/tty.c +6 -1

index bd8d671..330760f 100644
@@ -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;
 }

-------------------------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel

Ulisses Furquim

2006-08-24 18:29 UTC #1 lore raw

Hi Ville,

Quoted 2 lines · Ville Tervo

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?

Quoted 4 lines

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

Ville Tervo

author

2006-08-25 07:42 UTC #2 lore raw

Quoted 9 lines · ext Ulisses Furquim

Hi Ville,
Quoted 2 lines · Ville Tervo
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.

Quoted 16 lines

Quoted 4 lines
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:

Quoted 5 lines

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

}

You are right.

Signature · 12 lines

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
[email protected]
https://lists.sourceforge.net/lists/listinfo/bluez-devel