Hi. I have implemented a rfcomm server to use it as a serial port service using the example rfcomm client as the base. I tried to connect to my server from windows. (I used the SDP server for windows to find my device as a serial port and install a virtual com port) When I close the connection and wait for a while and then connect again there is no problem. But when I close and connect immediately my server stops with a segmentation fault. (It is an embedded system actually so I can say that a pointer stores an invalid address)

I searched and finally found that in function l2cap_signaling_handler_dispatch (l2cap.c) there is a for loop at the end:

// Find channel for this sig_id and connection handle
for (it = (linked_item_t *) l2cap_channels; it ; it = it->next){ l2cap_channel_t * channel = (l2cap_channel_t *) it; if (channel->handle == handle) { if (code & 1) { // match odd commands by previous signaling identifier if (channel->sig_id == sig_id) { l2cap_signaling_handler_channel(channel, command); } } else { // match even commands by local channel id if (channel->local_cid == dest_cid) { l2cap_signaling_handler_channel(channel, command); } } } }

Segmentation fault happens here. The problem is while looping, l2cap_channels link list can be changed (some channel can be removed) from the l2cap_signaling_handler_channel function. And in my situation (when there is more than one channel) it changes.

I tried to put "break;" after l2cap_signaling_handler_channel calls like this and it works now (although I didn't test very much):

// Find channel for this sig_id and connection handle
for (it = (linked_item_t *) l2cap_channels; it ; it = it->next){ l2cap_channel_t * channel = (l2cap_channel_t *) it; if (channel->handle == handle) { if (code & 1) { // match odd commands by previous signaling identifier if (channel->sig_id == sig_id) { l2cap_signaling_handler_channel(channel, command); break; } } else { // match even commands by local channel id if (channel->local_cid == dest_cid) { l2cap_signaling_handler_channel(channel, command); break; } } } }

I am looking forward to hear your thoughts about the fix.

Thanks

As a note when I connect to my device from windows first it does a sdp search and then connect to my rfcomm server. It does the sdp search everytime. I think because to find which rfcomm channel my service uses.

hi. thanks for reporting this. I recently had to rewrite multiple places where I iterate over a list and remove elements, but do some processing first. I guess you're right here: when an element is removed/freed, the it->next access will fail. I'll look into & fix this soon.

congrats on writing a rfcomm server. so far, my rfcomm implementation is the most complex so far, with about 1.5 k LOC, compared to 1k for L2CAP and 0.5 for HCI. And it still doesn't handle all cases, I'm afraid. OS X also does an SDP scan on every connection btw.

This issue was closed by revision r934.

your fix is fine. there should only be a single channel addressed, so jumping out of the loop is both correct and fixes the problem

Hi again, sorry for being late on this one. I found this problem in two other places. First one is here and this is my fix:

static void l2cap_handle_connection_failed_for_addr(bd_addr_t address, uint8_t status){ linked_item_t *it; uint8_t loop_again; do { loop_again = 0; for (it = (linked_item_t *) l2cap_channels; it ; it = it->next){ l2cap_channel_t * channel = (l2cap_channel_t *) it; if ( ! BD_ADDR_CMP( channel->address, address) ){ if (channel->state == L2CAP_STATE_CLOSED) { // failure, forward error code l2cap_emit_channel_opened(channel, status); // discard channel linked_list_remove(&l2cap_channels, (linked_item_t *) channel); free (channel); loop_again = 1; break; } } } } while (loop_again); }

Second one is here with my similar fix:

void l2cap_event_handler( uint8_t *packet, uint16_t size ){ uint8_t loop_again;

...

case HCI_EVENT_DISCONNECTION_COMPLETE:
    // send l2cap disconnect events for all channels on this handle
    handle = READ_BT_16(packet, 3);
    // only access next element to allows for removal

do {
    loop_again = 0;
    for (it = (linked_item_t *) &l2cap_channels; it->next ; it = it->next){
        l2cap_channel_t * channel = (l2cap_channel_t *) it->next;
        if ( channel->handle == handle ){
            // update prev item before free'ing next element
            it->next = it->next->next;
            l2cap_finialize_channel_close(channel);
            loop_again = 1;
        break;
        }
    }
} while (loop_again);

    break;

...

}

Thanks

hi. thanks for finding more. (After several tries...) I've settled on a while loop that handles iteration and deletion. I'll use that for the 2 places your found, too, to get it consistent. I'm close to writing a list iterator that calls a function for every element and handles deletions properly. .. .maybe later.

hey! I've already fixed those two cases in revision r892: http://code.google.com/p/btstack/source/detail?r=892

Sorry for that, I wish I had seen those fix before, so both me and you did not have to waste time. I had checked out before that revision and port that to arm and after that I looked to new revisions but missed that one. Sorry again and thank you.

hi. no problem. the other two bugs were real, and the SDP too. I just got my "iPhone as Bluetooth Keyboard" working with a PS3 after fixing SDP. :)

What are you working on? Is this commercial private? (you may answer in private if you like :)