opengitway / btstack

Automatically exported from code.google.com/p/btstack
0 stars 0 forks source link

daemon: Use after free when clients with active rfcomm connections disconnects #402

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Connect a client to the daemon.
2. Create an rfcomm connection. 
3. Disconnect the client.
4. rfcomm_close_connection will be closed, which will update the rfcomm state 
to RFCOMM_CHANNEL_W4_UA_AFTER_UA and send a disconnection packet to the device.
5. socket_connection_hci_process Will free the connection struct.
6. Later, when the response from the device arrives, rfcomm_emit_channel_closed 
with an invalid connection (when handling the RFCOMM_CHANNEL_W4_UA_AFTER_UA 
state).
7. I guess the this might be relevant to other l2cap connections 
(l2cap_close_connection is also called when a client disconnects).

What version of the product are you using? On what operating system?
Present in the latest r2651.

I guess one possible solution is to provide a way for letting the rfcomm\l2cap 
layers know the connection is dead.

Original issue reported on code.google.com by kob...@mce-sys.com on 16 Jun 2014 at 12:07

GoogleCodeExporter commented 9 years ago
the idea of rfcomm_close_connection has been to let rfcomm know, that the 
client connection is gone. So, I'll just have to fix the use after free 
(similarly, l2cap_close_connection is called)

Original comment by matthias.ringwald@gmail.com on 9 Aug 2014 at 11:35

GoogleCodeExporter commented 9 years ago
i've set the connection field to NULL in both l2cap_close_connection and 
rfcomm_close_connection in r2716. Now, all other clients would get the close 
event, that's not elegant. Will think about it a bit more.

Original comment by matthias.ringwald@gmail.com on 9 Aug 2014 at 11:44

GoogleCodeExporter commented 9 years ago
Thinking is over: :)

l2cap/rfcomm shouldn't know anything about "daemon connections". I'll remove 
the x_close_connections into daemon.c and keep a list of open channels and 
services linked to the connection there. That's a bit of extra memory in the 
daemon, but leads to cleaner code in l2cap/rfcomm. (then, I'll rename 
connection into context, as it is common with many APIs)

Original comment by matthias.ringwald@gmail.com on 9 Aug 2014 at 12:30

GoogleCodeExporter commented 9 years ago
Thanks, sounds good.

Original comment by kob...@mce-sys.com on 21 Aug 2014 at 1:51

GoogleCodeExporter commented 9 years ago

Original comment by matthias.ringwald@gmail.com on 5 Sep 2014 at 7:16