Export to GitHub

btstack - issue #402

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


Posted on Jun 16, 2014 by Swift Dog

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.

Comment #1

Posted on Aug 9, 2014 by Swift Ox

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)

Comment #2

Posted on Aug 9, 2014 by Swift Ox

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.

Comment #3

Posted on Aug 9, 2014 by Swift Ox

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)

Comment #4

Posted on Aug 21, 2014 by Swift Dog

Thanks, sounds good.

Comment #5

Posted on Sep 5, 2014 by Swift Ox

(No comment was entered for this change.)

Status: Done

Labels:
Type-Defect Priority-Critical