Fix OTA silently transferring a zero-length image - #2466
Open
fluffyspace wants to merge 1 commit into
Open
Conversation
DfuService::OnServiceData arms the 10s one-shot inactivity timer before dispatching each access. The image-size write then calls DfuImage::Erase(), which erases the whole image slot a sector at a time and takes tens of seconds. That timer runs on the FreeRTOS timer task, so it fires part-way through the erase and calls Reset(), zeroing applicationSize. The erase then finishes and the handler carries on regardless: it sets state = Init and reports success. ReceiveFirmwareImage later calls dfuImage.Init(20, 0, crc), so IsComplete() -- totalWriteIndex == totalSize, now 0 == 0 -- is already true when the first data packet arrives. The watch tells the host the entire image has been received, emits no packet-receipt notifications, writes nothing to flash, and validation CRCs zero bytes. From the host side this looks like an instant "whole image received" reply a fraction of a second after Begin DFU, followed by a validation failure - with no indication that the erase timed out. Recovery mode is unaffected because its erase is fast enough to finish inside the 10s window, which is why OTA can fail from the running firmware while succeeding from recovery. Stop the timer around the erase and restart it afterwards. Tested on a PineTime: before this change every OTA update from the running firmware failed as described; after it, updates complete and validate normally without needing recovery mode.
|
Build size and comparison to main:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The bug
DfuService::OnServiceDataarms the 10 s one-shot inactivity timer before dispatching each access:The image-size write then calls
DfuImage::Erase(), which erases the whole image slot a sector at a time and takes tens of seconds. That timer runs on the FreeRTOS timer task, so it fires part-way through the erase and callsReset(), which zeroesapplicationSize.The erase then completes and the handler carries on regardless — it sets
state = Initand reports success.ReceiveFirmwareImagelater callsdfuImage.Init(20, 0, crc), soIsComplete()(totalWriteIndex == totalSize, now0 == 0) is already true when the first data packet arrives. The watch tells the host the entire image has been received, emits no packet-receipt notifications, writes nothing to flash, and validation CRCs zero bytes.What it looks like from the host
10 03 01("whole image received") arrives a fraction of a second after Begin DFU — no real transfer can be that fast11 …packet-receipt notifications for the entire "transfer"There is no indication that the erase timed out, which makes this very hard to attribute. It cost me a couple of evenings before I instrumented the watch.
Recovery mode is unaffected: its erase finishes inside the 10 s window. That is why OTA can fail consistently from the running firmware while succeeding from recovery — which reads like a host or radio problem rather than a firmware one.
The fix
Stop the timer around the erase and restart it afterwards. Three lines.
Testing
Tested on a PineTime (bootloader 1.0.1, InfiniTime 1.16.0). Before this change, every OTA from the running firmware failed exactly as described. After it, updates complete and validate normally, and recovery mode is no longer needed to flash — verified across several successful ~390 kB updates.