Skip to content
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.

Sign up for GitHub

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

[K32W] Fix apply action corner case in OTATlvProcessor interface #33214

Merged
merged 5 commits into from
May 13, 2024
Merged

[K32W] Fix apply action corner case in OTATlvProcessor interface #33214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
068864e
[k32w] Add mShouldNotApply flag in OTATlvProcessor interface
marius-alex-tache Apr 23, 2024
6c10849
[k32w0] Use mShouldNotApply flag in ExitAction
marius-alex-tache Apr 23, 2024
5b0dae9
[k32w1] Use mShouldNotApply during ExitAction
marius-alex-tache Apr 23, 2024
9c2e869
[k32w] Update OTA error naming
marius-alex-tache May 8, 2024
0f94347
[k32w] Replace boolean mShouldNotApply with an enum class
marius-alex-tache May 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions src/platform/nxp/k32w/common/OTAImageProcessorImpl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ CHIP_ERROR OTAImageProcessorImpl::ProcessPayload(ByteSpan & block)
}

status = mCurrentProcessor->Process(block);
if (status == CHIP_OTA_CHANGE_PROCESSOR)
if (status == CHIP_ERROR_OTA_CHANGE_PROCESSOR)
{
mAccumulator.Clear();
mAccumulator.Init(sizeof(OTATlvHeader));
Expand Down Expand Up @@ -180,7 +180,7 @@ CHIP_ERROR OTAImageProcessorImpl::SelectProcessor(ByteSpan & block)
if (pair == mProcessorMap.end())
{
ChipLogError(SoftwareUpdate, "There is no registered processor for tag: %" PRIu32, header.tag);
return CHIP_OTA_PROCESSOR_NOT_REGISTERED;
return CHIP_ERROR_OTA_PROCESSOR_NOT_REGISTERED;
}

ChipLogDetail(SoftwareUpdate, "Selected processor with tag: %ld", pair->first);
Expand All @@ -197,7 +197,7 @@ CHIP_ERROR OTAImageProcessorImpl::RegisterProcessor(uint32_t tag, OTATlvProcesso
if (pair != mProcessorMap.end())
{
ChipLogError(SoftwareUpdate, "A processor for tag %" PRIu32 " is already registered.", tag);
return CHIP_OTA_PROCESSOR_ALREADY_REGISTERED;
return CHIP_ERROR_OTA_PROCESSOR_ALREADY_REGISTERED;
}

mProcessorMap.insert({ tag, processor });
Expand Down Expand Up @@ -249,7 +249,7 @@ void OTAImageProcessorImpl::HandleStatus(CHIP_ERROR status)
mParams.downloadedBytes += mBlock.size();
FetchNextData(0);
}
else if (status == CHIP_OTA_FETCH_ALREADY_SCHEDULED)
else if (status == CHIP_ERROR_OTA_FETCH_ALREADY_SCHEDULED)
{
mParams.downloadedBytes += mBlock.size();
}
Expand Down
11 changes: 9 additions & 2 deletions src/platform/nxp/k32w/common/OTATlvProcessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ namespace chip {
#if OTA_ENCRYPTION_ENABLE
constexpr uint8_t au8Iv[] = { 0x00, 0x00, 0x00, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18, 0x00, 0x00, 0x00, 0x00 };
#endif

CHIP_ERROR OTATlvProcessor::ApplyAction()
{
return mApplyState == ApplyState::kApply ? CHIP_NO_ERROR : CHIP_ERROR_OTA_PROCESSOR_DO_NOT_APPLY;
}

CHIP_ERROR OTATlvProcessor::Process(ByteSpan & block)
{
CHIP_ERROR status = CHIP_NO_ERROR;
Expand All @@ -50,7 +56,7 @@ CHIP_ERROR OTATlvProcessor::Process(ByteSpan & block)
// If current block was processed fully and the block still contains data, it
// means that the block contains another TLV's data and the current processor
// should be changed by OTAImageProcessorImpl.
return CHIP_OTA_CHANGE_PROCESSOR;
return CHIP_ERROR_OTA_CHANGE_PROCESSOR;
}
}
}
Expand All @@ -63,14 +69,15 @@ void OTATlvProcessor::ClearInternal()
mLength = 0;
mProcessedLength = 0;
mWasSelected = false;
mApplyState = ApplyState::kApply;
#if OTA_ENCRYPTION_ENABLE
mIVOffset = 0;
#endif
}

bool OTATlvProcessor::IsError(CHIP_ERROR & status)
{
return status != CHIP_NO_ERROR && status != CHIP_ERROR_BUFFER_TOO_SMALL && status != CHIP_OTA_FETCH_ALREADY_SCHEDULED;
return status != CHIP_NO_ERROR && status != CHIP_ERROR_BUFFER_TOO_SMALL && status != CHIP_ERROR_OTA_FETCH_ALREADY_SCHEDULED;
}

void OTADataAccumulator::Init(uint32_t threshold)
Expand Down
72 changes: 46 additions & 26 deletions src/platform/nxp/k32w/common/OTATlvProcessor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,20 @@ namespace chip {
#define CHIP_ERROR_TLV_PROCESSOR(e) \
ChipError(ChipError::Range::kLastRange, ((uint8_t) ChipError::Range::kLastRange << 3) | e, __FILE__, __LINE__)

#define CHIP_OTA_TLV_CONTINUE_PROCESSING CHIP_ERROR_TLV_PROCESSOR(0x01)
#define CHIP_OTA_CHANGE_PROCESSOR CHIP_ERROR_TLV_PROCESSOR(0x02)
#define CHIP_OTA_PROCESSOR_NOT_REGISTERED CHIP_ERROR_TLV_PROCESSOR(0x03)
#define CHIP_OTA_PROCESSOR_ALREADY_REGISTERED CHIP_ERROR_TLV_PROCESSOR(0x04)
#define CHIP_OTA_PROCESSOR_CLIENT_INIT CHIP_ERROR_TLV_PROCESSOR(0x05)
#define CHIP_OTA_PROCESSOR_MAKE_ROOM CHIP_ERROR_TLV_PROCESSOR(0x06)
#define CHIP_OTA_PROCESSOR_PUSH_CHUNK CHIP_ERROR_TLV_PROCESSOR(0x07)
#define CHIP_OTA_PROCESSOR_IMG_AUTH CHIP_ERROR_TLV_PROCESSOR(0x08)
#define CHIP_OTA_FETCH_ALREADY_SCHEDULED CHIP_ERROR_TLV_PROCESSOR(0x09)
#define CHIP_OTA_PROCESSOR_IMG_COMMIT CHIP_ERROR_TLV_PROCESSOR(0x0A)
#define CHIP_OTA_PROCESSOR_CB_NOT_REGISTERED CHIP_ERROR_TLV_PROCESSOR(0x0B)
#define CHIP_OTA_PROCESSOR_EEPROM_OFFSET CHIP_ERROR_TLV_PROCESSOR(0x0C)
#define CHIP_OTA_PROCESSOR_EXTERNAL_STORAGE CHIP_ERROR_TLV_PROCESSOR(0x0D)
#define CHIP_OTA_PROCESSOR_START_IMAGE CHIP_ERROR_TLV_PROCESSOR(0x0E)
#define CHIP_ERROR_OTA_CHANGE_PROCESSOR CHIP_ERROR_TLV_PROCESSOR(0x02)
#define CHIP_ERROR_OTA_PROCESSOR_NOT_REGISTERED CHIP_ERROR_TLV_PROCESSOR(0x03)
#define CHIP_ERROR_OTA_PROCESSOR_ALREADY_REGISTERED CHIP_ERROR_TLV_PROCESSOR(0x04)
#define CHIP_ERROR_OTA_PROCESSOR_CLIENT_INIT CHIP_ERROR_TLV_PROCESSOR(0x05)
#define CHIP_ERROR_OTA_PROCESSOR_MAKE_ROOM CHIP_ERROR_TLV_PROCESSOR(0x06)
#define CHIP_ERROR_OTA_PROCESSOR_PUSH_CHUNK CHIP_ERROR_TLV_PROCESSOR(0x07)
#define CHIP_ERROR_OTA_PROCESSOR_IMG_AUTH CHIP_ERROR_TLV_PROCESSOR(0x08)
#define CHIP_ERROR_OTA_FETCH_ALREADY_SCHEDULED CHIP_ERROR_TLV_PROCESSOR(0x09)
#define CHIP_ERROR_OTA_PROCESSOR_IMG_COMMIT CHIP_ERROR_TLV_PROCESSOR(0x0A)
#define CHIP_ERROR_OTA_PROCESSOR_CB_NOT_REGISTERED CHIP_ERROR_TLV_PROCESSOR(0x0B)
#define CHIP_ERROR_OTA_PROCESSOR_EEPROM_OFFSET CHIP_ERROR_TLV_PROCESSOR(0x0C)
#define CHIP_ERROR_OTA_PROCESSOR_EXTERNAL_STORAGE CHIP_ERROR_TLV_PROCESSOR(0x0D)
#define CHIP_ERROR_OTA_PROCESSOR_START_IMAGE CHIP_ERROR_TLV_PROCESSOR(0x0E)
#define CHIP_ERROR_OTA_PROCESSOR_DO_NOT_APPLY CHIP_ERROR_TLV_PROCESSOR(0x0F)

// Descriptor constants
constexpr size_t kVersionStringSize = 64;
Expand Down Expand Up @@ -77,13 +77,19 @@ struct OTATlvHeader
class OTATlvProcessor
{
public:
enum class ApplyState : uint8_t
{
kApply = 0,
kDoNotApply
};

virtual ~OTATlvProcessor() {}

virtual CHIP_ERROR Init() = 0;
virtual CHIP_ERROR Clear() = 0;
virtual CHIP_ERROR ApplyAction() = 0;
virtual CHIP_ERROR AbortAction() = 0;
virtual CHIP_ERROR ExitAction() { return CHIP_NO_ERROR; }
virtual CHIP_ERROR ApplyAction();

CHIP_ERROR Process(ByteSpan & block);
void RegisterDescriptorCallback(ProcessDescriptor callback) { mCallbackProcessDescriptor = callback; }
Expand All @@ -102,21 +108,21 @@ class OTATlvProcessor
* If more image chunks are needed, CHIP_ERROR_BUFFER_TOO_SMALL error is returned.
* Other error codes indicate that an error occurred during processing. Fetching
* next data is scheduled automatically by OTAImageProcessorImpl if the return value
* is neither an error code, nor CHIP_OTA_FETCH_ALREADY_SCHEDULED (which implies the
* is neither an error code, nor CHIP_ERROR_OTA_FETCH_ALREADY_SCHEDULED (which implies the
* scheduling is done inside ProcessInternal or will be done in the future, through a
* callback).
*
* @param block Byte span containing a subsequent Matter OTA image chunk. When the method
* returns CHIP_NO_ERROR, the byte span is used to return a remaining part
* of the chunk, not used by current TLV processor.
*
* @retval CHIP_NO_ERROR Block was processed successfully.
* @retval CHIP_ERROR_BUFFER_TOO_SMALL Provided buffers are insufficient to decode some
* metadata (e.g. a descriptor).
* @retval CHIP_OTA_FETCH_ALREADY_SCHEDULED Should be returned if ProcessInternal schedules
* fetching next data (e.g. through a callback).
* @retval Error code Something went wrong. Current OTA process will be
* canceled.
* @retval CHIP_NO_ERROR Block was processed successfully.
* @retval CHIP_ERROR_BUFFER_TOO_SMALL Provided buffers are insufficient to decode some
* metadata (e.g. a descriptor).
* @retval CHIP_ERROR_OTA_FETCH_ALREADY_SCHEDULED Should be returned if ProcessInternal schedules
* fetching next data (e.g. through a callback).
* @retval Error code Something went wrong. Current OTA process will be
* canceled.
*/
virtual CHIP_ERROR ProcessInternal(ByteSpan & block) = 0;

Expand All @@ -130,9 +136,23 @@ class OTATlvProcessor
/* Expected byte size of the OTAEncryptionKeyLength */
static constexpr size_t kOTAEncryptionKeyLength = 16;
#endif
uint32_t mLength = 0;
uint32_t mProcessedLength = 0;
bool mWasSelected = false;
uint32_t mLength = 0;
uint32_t mProcessedLength = 0;
bool mWasSelected = false;

/**
* @brief A flag to account for corner cases during OTA apply
*
* Used by the default ApplyAction implementation.
*
* If something goes wrong during ExitAction of the TLV processor,
* then mApplyState should be set to kDoNotApply and the image processor
* should abort. In this case, the BDX transfer was already finished
* and calling CancelImageUpdate will not abort the transfer, hence
* the device will reboot even though it should not have. If ApplyAction
* fails during HandleApply, then the process will be aborted.
*/
ApplyState mApplyState = ApplyState::kApply;
ProcessDescriptor mCallbackProcessDescriptor = nullptr;
};

Expand Down
26 changes: 11 additions & 15 deletions src/platform/nxp/k32w/k32w0/OTAFirmwareProcessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,21 +28,21 @@ namespace chip {

CHIP_ERROR OTAFirmwareProcessor::Init()
{
ReturnErrorCodeIf(mCallbackProcessDescriptor == nullptr, CHIP_OTA_PROCESSOR_CB_NOT_REGISTERED);
ReturnErrorCodeIf(mCallbackProcessDescriptor == nullptr, CHIP_ERROR_OTA_PROCESSOR_CB_NOT_REGISTERED);
mAccumulator.Init(sizeof(Descriptor));
#if OTA_ENCRYPTION_ENABLE
mUnalignmentNum = 0;
#endif
ReturnErrorCodeIf(gOtaSuccess_c != OTA_ClientInit(), CHIP_OTA_PROCESSOR_CLIENT_INIT);
ReturnErrorCodeIf(gOtaSuccess_c != OTA_ClientInit(), CHIP_ERROR_OTA_PROCESSOR_CLIENT_INIT);

auto offset = OTA_GetCurrentEepromAddressOffset();
if (offset != 0)
{
offset += 1;
}

ReturnErrorCodeIf(OTA_UTILS_IMAGE_INVALID_ADDR == OTA_SetStartEepromOffset(offset), CHIP_OTA_PROCESSOR_EEPROM_OFFSET);
ReturnErrorCodeIf(gOtaSuccess_c != OTA_StartImage(mLength - sizeof(Descriptor)), CHIP_OTA_PROCESSOR_START_IMAGE);
ReturnErrorCodeIf(OTA_UTILS_IMAGE_INVALID_ADDR == OTA_SetStartEepromOffset(offset), CHIP_ERROR_OTA_PROCESSOR_EEPROM_OFFSET);
ReturnErrorCodeIf(gOtaSuccess_c != OTA_StartImage(mLength - sizeof(Descriptor)), CHIP_ERROR_OTA_PROCESSOR_START_IMAGE);

return CHIP_NO_ERROR;
}
Expand Down Expand Up @@ -95,7 +95,7 @@ CHIP_ERROR OTAFirmwareProcessor::ProcessInternal(ByteSpan & block)
if (gOtaSuccess_c != status)
{
ChipLogError(SoftwareUpdate, "Failed to make room for next block. Status: %d", status);
return CHIP_OTA_PROCESSOR_MAKE_ROOM;
return CHIP_ERROR_OTA_PROCESSOR_MAKE_ROOM;
}
#if OTA_ENCRYPTION_ENABLE
status = OTA_PushImageChunk((uint8_t *) mBlock.data(), (uint16_t) mBlock.size(), NULL, NULL);
Expand All @@ -105,10 +105,10 @@ CHIP_ERROR OTAFirmwareProcessor::ProcessInternal(ByteSpan & block)
if (gOtaSuccess_c != status)
{
ChipLogError(SoftwareUpdate, "Failed to write image block. Status: %d", status);
return CHIP_OTA_PROCESSOR_PUSH_CHUNK;
return CHIP_ERROR_OTA_PROCESSOR_PUSH_CHUNK;
}

return CHIP_OTA_FETCH_ALREADY_SCHEDULED;
return CHIP_ERROR_OTA_FETCH_ALREADY_SCHEDULED;
}

CHIP_ERROR OTAFirmwareProcessor::ProcessDescriptor(ByteSpan & block)
Expand All @@ -122,12 +122,6 @@ CHIP_ERROR OTAFirmwareProcessor::ProcessDescriptor(ByteSpan & block)
return CHIP_NO_ERROR;
}

CHIP_ERROR OTAFirmwareProcessor::ApplyAction()
{

return CHIP_NO_ERROR;
}

CHIP_ERROR OTAFirmwareProcessor::AbortAction()
{
OTA_CancelImage();
Expand All @@ -144,13 +138,15 @@ CHIP_ERROR OTAFirmwareProcessor::ExitAction()
if (OTA_CommitImage(NULL) != gOtaSuccess_c)
{
ChipLogError(SoftwareUpdate, "Failed to commit firmware image.");
return CHIP_OTA_PROCESSOR_IMG_COMMIT;
mApplyState = ApplyState::kDoNotApply;
return CHIP_ERROR_OTA_PROCESSOR_IMG_COMMIT;
}

if (OTA_ImageAuthenticate() != gOtaImageAuthPass_c)
{
ChipLogError(SoftwareUpdate, "Failed to authenticate firmware image.");
return CHIP_OTA_PROCESSOR_IMG_AUTH;
mApplyState = ApplyState::kDoNotApply;
return CHIP_ERROR_OTA_PROCESSOR_IMG_AUTH;
}

OTA_AddNewImageFlag();
Expand Down
1 change: 0 additions & 1 deletion src/platform/nxp/k32w/k32w0/OTAFirmwareProcessor.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ class OTAFirmwareProcessor : public OTATlvProcessor

CHIP_ERROR Init() override;
CHIP_ERROR Clear() override;
CHIP_ERROR ApplyAction() override;
CHIP_ERROR AbortAction() override;
CHIP_ERROR ExitAction() override;

Expand Down
17 changes: 9 additions & 8 deletions src/platform/nxp/k32w/k32w1/OTAFirmwareProcessor.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,16 +26,16 @@ namespace chip {

CHIP_ERROR OTAFirmwareProcessor::Init()
{
ReturnErrorCodeIf(mCallbackProcessDescriptor == nullptr, CHIP_OTA_PROCESSOR_CB_NOT_REGISTERED);
ReturnErrorCodeIf(mCallbackProcessDescriptor == nullptr, CHIP_ERROR_OTA_PROCESSOR_CB_NOT_REGISTERED);
mAccumulator.Init(sizeof(Descriptor));

ReturnErrorCodeIf(gOtaSuccess_c != OTA_SelectExternalStoragePartition(), CHIP_OTA_PROCESSOR_EXTERNAL_STORAGE);
ReturnErrorCodeIf(gOtaSuccess_c != OTA_SelectExternalStoragePartition(), CHIP_ERROR_OTA_PROCESSOR_EXTERNAL_STORAGE);

otaResult_t ota_status;
ota_status = OTA_ServiceInit(&mPostedOperationsStorage[0], NB_PENDING_TRANSACTIONS * TRANSACTION_SZ);

ReturnErrorCodeIf(ota_status != gOtaSuccess_c, CHIP_OTA_PROCESSOR_CLIENT_INIT);
ReturnErrorCodeIf(gOtaSuccess_c != OTA_StartImage(mLength - sizeof(Descriptor)), CHIP_OTA_PROCESSOR_START_IMAGE);
ReturnErrorCodeIf(ota_status != gOtaSuccess_c, CHIP_ERROR_OTA_PROCESSOR_CLIENT_INIT);
ReturnErrorCodeIf(gOtaSuccess_c != OTA_StartImage(mLength - sizeof(Descriptor)), CHIP_ERROR_OTA_PROCESSOR_START_IMAGE);

return CHIP_NO_ERROR;
}
Expand Down Expand Up @@ -70,7 +70,7 @@ CHIP_ERROR OTAFirmwareProcessor::ProcessInternal(ByteSpan & block)
if (gOtaSuccess_c != status)
{
ChipLogError(SoftwareUpdate, "Failed to make room for next block. Status: %d", status);
return CHIP_OTA_PROCESSOR_MAKE_ROOM;
return CHIP_ERROR_OTA_PROCESSOR_MAKE_ROOM;
}
}
else
Expand All @@ -82,10 +82,10 @@ CHIP_ERROR OTAFirmwareProcessor::ProcessInternal(ByteSpan & block)
if (gOtaSuccess_c != status)
{
ChipLogError(SoftwareUpdate, "Failed to write image block. Status: %d", status);
return CHIP_OTA_PROCESSOR_PUSH_CHUNK;
return CHIP_ERROR_OTA_PROCESSOR_PUSH_CHUNK;
}

return CHIP_OTA_FETCH_ALREADY_SCHEDULED;
return CHIP_ERROR_OTA_FETCH_ALREADY_SCHEDULED;
}

CHIP_ERROR OTAFirmwareProcessor::ProcessDescriptor(ByteSpan & block)
Expand Down Expand Up @@ -117,7 +117,8 @@ CHIP_ERROR OTAFirmwareProcessor::ExitAction()
if (OTA_CommitImage(NULL) != gOtaSuccess_c)
{
ChipLogError(SoftwareUpdate, "Failed to commit firmware image.");
return CHIP_OTA_PROCESSOR_IMG_COMMIT;
mApplyState = ApplyState::kDoNotApply;
return CHIP_ERROR_OTA_PROCESSOR_IMG_COMMIT;
}

return CHIP_NO_ERROR;
Expand Down
Loading