-
Notifications
You must be signed in to change notification settings - Fork 622
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
GATTSERVICE_SUBEVENT_HID_SERVICE_CONNECTED not received in Pico W in latest develop branch #651
Comments
attached the the One |
mmm... and the
|
I did some debugging and the function that has the issue is in static void hci_emit_event(uint8_t * event, uint16_t size, int dump){
// dump packet
if (dump) {
hci_dump_packet( HCI_EVENT_PACKET, 1, event, size);
}
// Added by me
printf_hexdump(event, size);
// dispatch to all event handlers
btstack_linked_list_iterator_t it;
btstack_linked_list_iterator_init(&it, &hci_stack->event_handlers);
int i=0;
while (btstack_linked_list_iterator_has_next(&it)){
btstack_packet_callback_registration_t * entry = (btstack_packet_callback_registration_t*) btstack_linked_list_iterator_next(&it);
// Added by me
printf("about to call, iteration: %d, addr: %p, callback: %p\n", i, entry, entry ? entry->callback : 0);
i++;
entry->callback(HCI_EVENT_PACKET, 0, event, size);
}
} and before crashing the values are:
if you pay attention, only the first byte is different.
Somewhere there is a off-by-one overflow... or so it seems |
It seems that the byte just before Buffer defined here: I tried adding a "zero buffer" right before So, perhpas it is a pico-sdk bug ? and not a BTstack bug ? |
Thanks for the troubleshooting. I'm currently at 38C3 in Hamburg. We're added 4 bytes to all GATT Client events and have to use more pre-buffer bytes. In theory, a compile time check should have caught/prevented this issue. Can you check and report HCI_INCOMING_PRE_BUFFER_SIZE in your or the pico-sdk setup? |
nice. Enjoy!
8 I put a hardware breapoint on my own variable (the one that gets overwritten), and I got:
In particular, the watchpoint gets triggered here: static uint8_t *
setup_long_characteristic_value_packet(const gatt_client_t *gatt_client, uint8_t type, uint16_t attribute_handle,
uint16_t offset, uint8_t *value, uint16_t length) {
#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
// avoid using pre ATT headers.
// copy value into test packet for testing
static uint8_t packet[1000];
memcpy(&packet[LONG_CHARACTERISTIC_VALUE_EVENT_HEADER_SIZE], value, length);
#else
// before the value inside the ATT PDU
uint8_t * packet = value - LONG_CHARACTERISTIC_VALUE_EVENT_HEADER_SIZE;
#endif
// ********** WATCHDOG GETS TRIGGERED HERE **************
packet[0] = type;
packet[1] = LONG_CHARACTERISTIC_VALUE_EVENT_HEADER_SIZE - 2 + length;
little_endian_store_16(packet, 2, gatt_client->con_handle);
little_endian_store_16(packet, 4, gatt_client->service_id);
little_endian_store_16(packet, 6, gatt_client->connection_id);
little_endian_store_16(packet, 8, attribute_handle);
little_endian_store_16(packet, 10, offset);
little_endian_store_16(packet, 12, length);
return packet;
} Besides that there is some pointer math that looks suspicious, wondering it might be related to the |
After looking at the stack trace, it doesn't seem related to the Instead, So either nit: if possible, it would be great to have more documentation about the constants. E.g: Why |
When using BTstack 1.6.2 (latest stable version), the microcontroller might crash due to buffer underflow. The byte before the first byte of hci_packet_with_pre_buffer will get overwritten. In particular the problem was that BTstack `setup_long_characteristic_value_packet()` was receiving `&hci_packet_with_pre_buffer[13]`, and in that function the packet gets overwritten starting from "- LONG_CHARACTERISTIC_VALUE_EVENT_HEADER_SIZE", which is 14. So the byte before hci_packet_with_pre_buffer gets overwritten. See: https://github.com/bluekitchen/btstack/blob/5d4d8cc7b1d35a90bbd6d5ffd2d3050b2bfc861c/src/ble/gatt_client.c#L1060 This PR follows the same logic implemented in BTstack ESP32 port. See: https://github.com/bluekitchen/btstack/blob/develop/port/esp32/components/btstack/btstack_port_esp32.c#L104 Fixes bluekitchen/btstack#651
@mringwal I think I found the root cause. Sent PR against pico-sdk. |
Describe the bug
Using BTstack with Pico W. Trying to connect a gamepad (using Bluepad32) that uses BLE. Everything works Ok.
When I switch to latest
develop
BTstack (hash: 16e6dda), the gamepad fails to finish the connection... or at least the eventGATTSERVICE_SUBEVENT_HID_SERVICE_CONNECTED
is not received.Only happens on Pico W.
Tested on Posix, ESP32, ESP32-C6, ESP32-H2 and works Ok.
I noticed this bug a few months ago in develop branch, but didn't have the time to investigate it property.... but a few days ago when I re-tested everything on latest btstack develop branch, I was no longer able to reproduce it.
To give you more detail, the flow is more or less like the following in BLE:
gap_connect()
sm_packet_handler()
will try to pairSM_EVENT_REENCRYPTION_COMPLETE
orSM_EVENT_PAIRING_COMPLETE
device_information_service_client_query()
is startedGATTSERVICE_SUBEVENT_DEVICE_INFORMATION_DONE
hids_client_connect()
hids_client_packet_handler()
... nothing... not an error, nothing.To Reproduce
I'll try to create a test case based on
hog_host_demo.c
Expected behavior
HCI Packet Logs
I'll attach logs in about ~7 days... sorry for the lack of logs. But wanted to file the bug before going on vacation.
I did a quick test using
WANT_HCI_DUMP
... but couldn't make it work, but probably something wrong on my side.Environment: (please complete the following information):
Additional context
Add any other context about the problem here.
The text was updated successfully, but these errors were encountered: