Re: [PR] Fix ble_gap_unpair_oldest_peer to prevent writing to invalid memory [mynewt-nimble]
andrzej-kaczmarek commented on PR #1650: URL: https://github.com/apache/mynewt-nimble/pull/1650#issuecomment-1942672005 `ble_gap_unpair_oldest_except` uses `BLE_STORE_MAX_BONDS` because it needs to retrieve all bonds in order to apply filter. I don't really see how `ble_gap_unpair_oldest_peer` can fail because of using `1` instead of `BLE_STORE_MAX_BONDS`. Actually I've just checked it in my app and it works perfectly fine. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix ble_gap_unpair_oldest_peer to prevent writing to invalid memory [mynewt-nimble]
darshan7patel commented on PR #1650: URL: https://github.com/apache/mynewt-nimble/pull/1650#issuecomment-1935383385 Okay, earlier when I attempted to bond 3 devices by setting MAX_BOND_COUNT to 2, it didn't work correctly. The correct behavior should be to delete the oldest peer when bonding with the 3rd device. Therefore, I made changes, and now it functions correctly. Additionally, in `ble_gap_unpair_oldest_except`, we are passing an array and `BLE_STORE_MAX_BONDS` as parameters in `ble_store_util_bonded_peers`, which is correct. So, I took reference from there and modified `ble_gap_unpair_oldest_peer` to pass an array and `BLE_STORE_MAX_BONDS` as arguments, resulting in the correct behaviour when the number of devices is greater than 2. However, I will conduct more tests to ensure everything is functioning properly. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix ble_gap_unpair_oldest_peer to prevent writing to invalid memory [mynewt-nimble]
andrzej-kaczmarek commented on PR #1650: URL: https://github.com/apache/mynewt-nimble/pull/1650#issuecomment-1935077674 That behavior is on purpose because we only need single peer returned by `ble_store_util_bonded_peers` in `ble_store_util_delete_oldest_peer`. The `status` field set from `ble_store_util_iterate_unique_peer` is simply ignored because we don't care if there were more peers than requested (i.e. 1). We only care about return value from the function call and that can only fail if there was actual error when reading from store. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix ble_gap_unpair_oldest_peer to prevent writing to invalid memory [mynewt-nimble]
darshan7patel commented on PR #1650: URL: https://github.com/apache/mynewt-nimble/pull/1650#issuecomment-1933928947 Hi @andrzej-kaczmarek, Upon review, I noticed that instead of passing **BLE_STORE_MAX_BONDS** as the argument for **ble_store_util_bonded_peers** in **ble_gap_unpair_oldest_peer**, a hardcoded value of 1 is being used. Consequently, within **ble_store_iterate** called from **ble_store_util_bonded_peers**, the callback function **ble_store_util_iter_unique_peer** is invoked. It **returns 1** since **set->num_peers >= set->max_peers** evaluates to true, **causing an overflow**. As a result, it updates the set status to **BLE_HS_ENOMEM (6)** due to resource exhaustion. Thus, this is the place where overflow occurred. These changes should address the resource exhaustion error and enable proper deletion of the oldest peer. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix ble_gap_unpair_oldest_peer to prevent writing to invalid memory [mynewt-nimble]
andrzej-kaczmarek commented on PR #1650: URL: https://github.com/apache/mynewt-nimble/pull/1650#issuecomment-1833484136 Again, please point me to exact location in code which returns this error code. I don't see how `ble_store_read` can return `rc=6` in any case for either config or ram stores. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix ble_gap_unpair_oldest_peer to prevent writing to invalid memory [mynewt-nimble]
darshan7patel commented on PR #1650: URL: https://github.com/apache/mynewt-nimble/pull/1650#issuecomment-1833375283 > `ble_store_util_bonded_peers` returns `rc=6` only if `ble_store_iterate` returns `rc=6` so assuming you're right, please point to the location in file which returns this error code in described scenario. The error code rc=6 is returned from `ble_store_read` within the `ble_store_iterate` function, indicating resource exhaustion. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix ble_gap_unpair_oldest_peer to prevent writing to invalid memory [mynewt-nimble]
andrzej-kaczmarek commented on PR #1650: URL: https://github.com/apache/mynewt-nimble/pull/1650#issuecomment-1820495034 `ble_store_util_bonded_peers` returns `rc=6` only if `ble_store_iterate` returns `rc=6` so assuming you're right, please point to the location in file which returns this error code in described scenario. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix ble_gap_unpair_oldest_peer to prevent writing to invalid memory [mynewt-nimble]
darshan7patel commented on PR #1650: URL: https://github.com/apache/mynewt-nimble/pull/1650#issuecomment-1818307690 > `max_peers` parameter passed to `ble_store_util_bonded_peers` is set to `1` which means `ble_store_iterate` will stop iterating after 1st matched peer. There's no need for an array because no more than 1 peer is returned. But in the `ble_store_util_bonded_peer` function, the `max_peer` parameter (which is the third parameter), is hardcoded to 1. This could be problematic when the maximum count is 2 (or BLE_STORE_MAX_BONDS > 1). When the count reaches 2, it returns an error code (rc=6) indicating resource exhaustion, and consequently, the `ble_gap_unpair` function is not called, leading to the oldest peer not being deleted. Previously, when an array was not used, the function appeared to work, but there was a mismatch between the list count and the actual stored values and it's possible that the data was being stored outside the allocated memory. This changes should resolve the resource exhaustion error and enable the proper deletion of the oldest peer. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix ble_gap_unpair_oldest_peer to prevent writing to invalid memory [mynewt-nimble]
andrzej-kaczmarek commented on PR #1650: URL: https://github.com/apache/mynewt-nimble/pull/1650#issuecomment-1814248204 `max_peers` parameter passed to `ble_store_util_bonded_peers` is set to `1` which means `ble_store_iterate` will stop iterating after 1st matched peer. There's no need for an array because no more than 1 peer is returned. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix ble_gap_unpair_oldest_peer to prevent writing to invalid memory [mynewt-nimble]
darshan7patel commented on PR #1650: URL: https://github.com/apache/mynewt-nimble/pull/1650#issuecomment-1814239868 > This function removes 1 bond which is 1st on the returned list and thus it only needs to retrieve that single bond. There's no need to retrieve all bonded devices only to pick 1st one anyway... That's correct. However, it's necessary to provide **BLE_STORE_MAX_BONDS** as a parameter to **ble_store_util_bonded_peers** to manage the maximum bond count. Subsequently, **ble_store_util_iter_unique_peer** will be invoked. In this process, when it accesses `set->peer_id_addrs[set->num_peers]`, it requires an array to access that specific index. Therefore, an array is essential when the maximum bond count is greater than one. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix ble_gap_unpair_oldest_peer to prevent writing to invalid memory [mynewt-nimble]
andrzej-kaczmarek commented on PR #1650: URL: https://github.com/apache/mynewt-nimble/pull/1650#issuecomment-1814110559 This function removes 1 bond which is 1st on the returned list and thus it only needs to retrieve that single bond. There's no need to retrieve all bonded devices only to pick 1st one anyway... -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix ble_gap_unpair_oldest_peer to prevent writing to invalid memory [mynewt-nimble]
darshan7patel commented on PR #1650: URL: https://github.com/apache/mynewt-nimble/pull/1650#issuecomment-1813983309 > what exactly was the issue? original code retrieves only 1 address so it looks fine. - Changes were made to accommodate a maximum of BLE_STORE_MAX_BONDS bonds. Previously, it was hardcoded to support only a single bond, limiting the system to handle just one bond. - Additionally, the function **ble_store_util_bonded_peers** is designed to store bonded peers and return a list. To ensure proper memory allocation, it is essential to pass an array as the function argument. Failure to do so may result in the data being stored outside the allocated memory. Therefore, passing an array is crucial for correct functionality. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
Re: [PR] Fix ble_gap_unpair_oldest_peer to prevent writing to invalid memory [mynewt-nimble]
andrzej-kaczmarek commented on PR #1650: URL: https://github.com/apache/mynewt-nimble/pull/1650#issuecomment-1812527168 what exactly was the issue? original code retrieves only 1 address so it looks fine. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@mynewt.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
