search

zephyrproject-rtos / zephyr

Primary Git Repository for the Zephyr Project. Zephyr is a new generation, scalable, optimized, secure RTOS for multiple hardware architectures.
https://docs.zephyrproject.org
Apache License 2.0
10.08k stars 6.19k forks source link

Bluetooth: Host: Incomplete handling of CCCs when identity is resolved and CCC Lazy Loading is disabled #64042

Open alstrzebonski opened 8 months ago

alstrzebonski commented 8 months ago

Describe the bug The scenario is as follows:

  1. GATT client connects, bonds, writes some CCC and disconnects. CCC Lazy Loading is disabled on GATT server side.
  2. The GATT client loses bond and connects from its RPA address.
  3. The GATT client writes CCC before bonding. The previously saved CCC is not overwritten, because it has peer's identity address and new CCC has peer's RPA address. It's expected behavior.
  4. The peer bonds. On identity_resolved callback, the RPA address in struct bt_gatt_ccc_cfg cfg[] array in subsys/bluetooth/host/gatt.c is converted to peer's identity address. We end up with struct bt_gatt_ccc_cfg cfg[] array that has two slots occupied by two CCCs with the same peer address. It can prevent new devices from writing CCC because there may be no more space in RAM for new CCCs as the struct bt_gatt_ccc_cfg cfg[] array length is equal to (CONFIG_BT_MAX_PAIRED + CONFIG_BT_MAX_CONN). Moreover, the newer CCC is never written to flash, because on save operation, the program iterates through the array and when it finds CCC with address that matches the bond address, it saves this address and stops iterating, so it never reaches the 2nd CCC. On reboot, new CCC disappears from the cfg array as it was never saved in flash.

When CCC Lazy Loading is enabled, the problem doesn't occur. The CCC is updated in flash after the peer bonds again.

When the peer connects from its identity address instead of RPA address, the problem also doesn't occur, because the CCC is overwritten immediately (new CCC has the same peer address as the old one).

To Reproduce Steps to reproduce the behavior:

  1. Build and flash zephyr/samples/bluetooth/peripheral sample with CONFIG_BT_SETTINGS_CCC_LAZY_LOADING disabled.
  2. Bond with the sample from your Bluetooth central (e.g. phone) that uses RPA address to connect.
  3. Write some CCC e.g. Battery Level in Battery Service.
  4. Disconnect.
  5. Remove bond on the phone side.
  6. Connect (without bonding) to sample.
  7. Write that CCC again (different value).
  8. Bond with the sample.
  9. Read the CCC. See that new value has not been saved.
  10. Repeat steps 4-7.
  11. See that the CCC write fails with warning message: No space to store CCC cfg. That's because the cfg array is full.

Expected behavior The CCC write should succeed regardless of CONFIG_BT_SETTINGS_CCC_LAZY_LOADING being enabled or not and regardless of RPA address being used or not. On identity_resolved callback, the CCCs with the same address should be merged - new CCC should overwrite the old one.

Impact Google Fast Pair Service protocol (https://developers.google.com/nearby/fast-pair/specifications/introduction) requires using GATT before bonding. Because of that, if a phone loses bond multiple times, it is unable to bond again using Fast Pair, because it is unable to write CCC when CONFIG_BT_SETTINGS_CCC_LAZY_LOADING is disabled and cfg array is full.

Environment (please complete the following information):

github-actions[bot] commented 6 months ago

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

jori-nordic commented 5 months ago

@alstrzebonski would you be able to come up with a patch for this? It seems you analyzed the issue pretty well.

alstrzebonski commented 4 months ago

@alstrzebonski would you be able to come up with a patch for this? It seems you analyzed the issue pretty well.

I'm sorry for the late response. I will need to see if I have the capacity to make the fix. I will get back to you after sprint planning. In most of our projects we have the CCC Lazy Loading enabled so the issue doesn't affect us currently. However, it may come up in the future.

github-actions[bot] commented 2 months ago

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

alstrzebonski commented 2 months ago

I will fix this issue. I have it in my TODO list, but I struggle with finding the capacity for it.

github-actions[bot] commented 2 weeks ago

This issue has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this issue will automatically be closed in 14 days. Note, that you can always re-open a closed issue at any time.

alstrzebonski commented 2 weeks ago

Please remove the stale label. I will fix it.