search

NordicSemiconductor / Android-nRF-Connect-Device-Manager

A mobile management library for devices running Apache Mynewt and Zephyr (DFU, logs, stats, config, etc.)
Apache License 2.0
81 stars 40 forks source link

Corrupted response while uploading file makes libary unresponsive #163

Closed Maartenwn closed 1 month ago

Maartenwn commented 2 months ago

When uploading a file I sometimes see incorrect/corrupted data being received. I have checked with a BLE sniffer and the data is being send correctly from the device however somewhere on the phone this data gets corrupted see line 23 This corrupted has already occurred in the HCI logs. The library then tries to resend the message again and the response that is received is correctly, however the library doesn't handle this message so it keeps retrying and eventually fails.

Edit: It also seems that this only happens with certain phones.

Maartenwn commented 2 months ago

https://github.com/NordicSemiconductor/Android-nRF-Connect-Device-Manager/blob/76817fff22924f04d2e4c3ea58cb97d01dd4d714/mcumgr-ble/src/main/java/io/runtime/mcumgr/ble/callback/SmpMerger.java#L41 It seems that the expectedLenght that is retrieved from the first message (if this message is corrupted) is a random value that will never equal the length of the incoming data causing the library to be stuck.

philips77 commented 1 month ago

Hmm... It's hard to do anything with it. We could try to ignore a message if the header is "horribly wrong", but that can lead to other issues later.

Maartenwn commented 1 month ago

Hmm... It's hard to do anything with it. We could try to ignore a message if the header is "horribly wrong", but that can lead to other issues later.

We have kinda solved it by resetting the BLE connection when an timeout happens. This causes the SmpMerger to be in a correct state again.

philips77 commented 1 month ago

I added minor validation. Length is expected to be < 2500 bytes. In your case, it was way above, but as you say - it's garbage, so sometimes a smaller value can be passed.

When such large length is received, the SmpMerger will return true, returning the full packet as a single packet, not first of.