Export to GitHub

btstack - issue #422

Daemon crashes if HCI commands are issued over USB too fast


Posted on Oct 16, 2014 by Helpful Rabbit

What steps will reproduce the problem? 1. Use daemon with libusb transport 2. Send a command at the wrong time

What is the expected output? What do you see instead? Daemon crashes with "Error submitting control transfer -6"

What version of the product are you using? On what operating system? svn trunk on Linux

The error message comes from platforms/posix/src/hci_transport_h2_libusb.c in usb_send_cmd_packet. It is informative to move the hci_dump_packet() call in this function to before the error/return; I can submit a patch if you like.

I note that the stack normally never issues two CMDs in a row without waiting for a response. In this particular example, the user packet handler seems to receive a PIN request before the connection create command is complete. There are a number of other ways to trigger the issue (eg. from HCI_EVENT_CONNECTION_COMPLETE event handling).

An example log in this case ends with: [2014-10-16 12:20:20.038] LOG -- L2CAP_CREATE_CHANNEL_MTU addr 00:0A:95:04:93:23 psm 0x13 mtu 150 [2014-10-16 12:20:20.038] LOG -- Create_connection to 00:0A:95:04:93:23 [2014-10-16 12:20:20.038] LOG -- create_connection_for_addr 00:0A:95:04:93:23 [2014-10-16 12:20:20.038] LOG -- conn state 0 [2014-10-16 12:20:20.038] CMD => 05 04 0D 23 93 04 95 0A 00 18 CC 00 00 00 00 01 [2014-10-16 12:20:20.038] CMD => 0D 04 17 23 93 04 95 0A 00 04 30 30 30 30 00 6C 69 6E 6B 20 6B 65 79 3A 20 25 [2014-10-16 12:20:20.039] LOG -- Error submitting control transfer -6

Comment #1

Posted on Oct 16, 2014 by Helpful Rabbit

I should note that this particular trigger was delivered by sending hci_pin_code_request_reply from the HCI_EVENT_COMMAND_COMPLETE handler by accident. Would a saner test case be helpful?

Comment #2

Posted on Oct 16, 2014 by Helpful Rabbit

OK, this happens because daemon.c calls hci_send_cmd_packet(), without first checking hci_can_send_command_packet_now().

The question is - what's the right thing to do? Can the command(s) be queued somehow and issued later? It would be nice not to have to push the issue into the client.

Comment #3

Posted on Oct 16, 2014 by Swift Ox

hi

thanks for pointing that one out. I though I'd added checks for can_send_now everywhere. In the embedded version, this has to be handled by the app itself, which is necessary to save RAM. For the daemon, it cannot be handled by the client, as multiple clients potentially could send an HCI command at the very same time.

the daemon already has a dirty hack to "park" client connections if the current packet cannot be handled.

if you like to try, you can add if (!hci_can_send_command_packet_now) return BTSTACK_ACL_BUFFERS_FULL; before the hci_send_cmd_packet() call and let me know if that helps.

Comment #4

Posted on Oct 16, 2014 by Helpful Rabbit

It certainly stops the crashes, of course! The client doesn't get terminally confused either, so it's not getting stuck in park. But it's also not clear that the original command is being run; I'll take another look tomorrow.

Comment #5

Posted on Oct 22, 2014 by Helpful Rabbit

This patch has been working well for me. Checks for parked connections when HCI commands complete, too.

Comment #6

Posted on Oct 22, 2014 by Helpful Rabbit

Previous patch used return instead of break and prevented clients from receiving command complete events! Fixed here.

Attachments

Status: New

Labels:
Type-Defect Priority-Medium