fix: perf log DataStart offset, %u format specifiers, ResourceId narrowing conversion#2758
Open
stark256-spec wants to merge 2 commits into
Conversation
stark256-spec
force-pushed
the
fix/perf-metadata-format-specifiers-resourceid-narrowing
branch
from
d205909 to
6ee5fc2
Compare
CFE_EVS_ProcessCommandPacket() and CFE_ES_TaskPipe() both cast the SB buffer directly to *SendHkCmd_t without calling VerifyCmdLength first. Every other command code in ProcessGroundCommand/ProcessGroundCmd is guarded by a VerifyCmdLength check before the cast. A truncated or malformed SEND_HK_MID packet silently bypasses validation and causes an OOB read when the handler accesses struct fields beyond the actual packet boundary. Add the missing VerifyCmdLength guard in both dispatch functions, matching the existing pattern used for all ground commands. Fixes nasa/cFS#987 (EVS) Fixes nasa/cFS#986 (ES)
…rrowing Three independent defects: 1. CFE_ES_RunPerfLogDump (cfe_es_perf.c): State->DataPos was initialized to Perf->MetaData.DataStart when entering WRITE_PERF_ENTRIES. The dump loop now writes entries in order starting from DataStart, so the output file always begins at offset 0 — but MetaData.DataStart was left unchanged, causing parsers to skip entries before the original start index. Fix: initialize DataPos to 0 to match the actual file layout. Fixes nasa#2637. 2. CFE_ES_PerfLogAdd (cfe_es_perf.c): Marker (uint32) and CFE_MISSION_ES_PERF_MAX_IDS were printed with %d (signed) format specifiers. Changed to %u and added explicit (unsigned int) cast on the macro value to silence -Wformat warnings. Fixes nasa#2638. 3. CFE_ResourceId_FromInteger (cfe_resourceid.h): In strict mode CFE_RESOURCEID_WRAP expands to a compound literal initializer for a struct containing uint32. Passing an unsigned long directly triggers a narrowing-conversion warning/error when compiling C++ with -Werror. Added explicit (uint32) cast on Value before the wrap. Fixes nasa#2664.
stark256-spec
force-pushed
the
fix/perf-metadata-format-specifiers-resourceid-narrowing
branch
from
6ee5fc2 to
896b9c5
Compare
Author
|
This branch is rebased on current |
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
2 participants
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.
Summary
Three independent defects across two files.
Fix 1 — Perf log metadata
DataStartmismatches file layout (closes #2637)File:
modules/es/fsw/src/cfe_es_perf.cCFE_ES_RunPerfLogDumprewrites the dump buffer in order starting atDataStart, so the output file always begins at index 0. However,State->DataPoswas still initialised toPerf->MetaData.DataStartwhen enteringWRITE_PERF_ENTRIES, leavingMetaData.DataStartunchanged in the file. Parsers that honour this field skip every entry before the original start index — the number of skipped entries varies with wherever the circular buffer happened to stop.Fix: set
State->DataPos = 0to match the actual file layout.Fix 2 —
%dformat specifiers for unsigned values (closes #2638)File:
modules/es/fsw/src/cfe_es_perf.cCFE_ES_PerfLogAddformatsMarker(uint32) andCFE_MISSION_ES_PERF_MAX_IDSwith%d(signed). Changed both to%uand added an explicit(unsigned int)cast on the macro expression to satisfy-Wformat.Fix 3 — Narrowing conversion in
CFE_ResourceId_FromInteger(closes #2664)File:
modules/core_api/fsw/inc/cfe_resourceid.hIn strict mode
CFE_RESOURCEID_WRAP(x)expands to a compound-literal struct initializer{ x }where the field isuint32. Passing anunsigned longdirectly is a narrowing conversion that becomes a hard error when compiling C++ with-Werror=narrowingon 64-bit Linux (whereunsigned longis 64 bits).Fix: add an explicit
(uint32)cast onValuebefore the wrap, consistent with the suggestion in the issue.Test plan
-Werror=narrowingwithOMIT_DEPRECATED=ONC++ build