mm_load.c: validate DataSize locally in MM_PokeMem before dispatch

.github/workflows/static-analysis.yml

@@ -1,6 +1,5 @@
 name: Static Analysis
 
-# Run on all push and pull requests
 on:
   push:
     branches:
@@ -16,4 +15,6 @@ on:
 jobs:
   static-analysis:
     name: Static Analysis
-    uses: nasa/cFS/.github/workflows/app-static-analysis-reusable.yml@dev
+    uses: nasa/cFS/.github/workflows/static-analysis-reusable.yml@dev
+    with:
+      source-dir: 'source/fsw'

fsw/inc/mm_internal_cfg.h

@@ -479,7 +479,7 @@
  *       or equal to zero.
  */
 #define MM_INTERNAL_MISSION_REV         MM_INTERNAL_CFGVAL(MISSION_REV)
-#define DEFAULT_MM_INTERNAL_MISSION_REV 0
+#define DEFAULT_MM_INTERNAL_MISSION_REV 0xFF
 
 /**
  * \brief Maximum dump bytes in an event string

fsw/src/mm_load.c

@@ -56,6 +56,34 @@ CFE_Status_t MM_PokeMem(const MM_PokeCmd_t *CmdPtr, cpuaddr DestAddress)
     size_t       DataSize       = 0; /* only used for giving MEM type/size in events */
     uint32       EventID        = 0;
 
+    /*
+     * Defense-in-depth: re-validate the DataSize field before
+     * dispatching to the platform write primitives.  Today the
+     * validity of this field is also enforced upstream in
+     * MM_VerifyPeekPokeParams() before MM_PokeMem() is called,
+     * but this function is the one that holds the
+     * CFE_PSP_MemWrite{8,16,32}() calls and should not depend on
+     * a cross-translation-unit invariant for memory safety.  Any
+     * future code path that reaches MM_PokeMem() without going
+     * through MM_VerifyPeekPokeParams() (refactor of MM_PokeCmd
+     * dispatch, new ground-command entry that delegates to
+     * MM_PokeMem, etc.) would otherwise drop straight into the
+     * switch with PSP_Status left at its initialised value of
+     * CFE_PSP_ERROR_NOT_IMPLEMENTED and the command counter
+     * unchanged, but no diagnostic event sent --- silently
+     * failing to update HK and silently failing to log.
+     */
+    if (CmdPtr->Payload.DataSize != MM_INTERNAL_BYTE_BIT_WIDTH &&
+        CmdPtr->Payload.DataSize != MM_INTERNAL_WORD_BIT_WIDTH &&
+        CmdPtr->Payload.DataSize != MM_INTERNAL_DWORD_BIT_WIDTH)
+    {
+        CFE_EVS_SendEvent(MM_DATA_SIZE_BITS_ERR_EID,
+                          CFE_EVS_EventType_ERROR,
+                          "MM_PokeMem invalid data size: %u",
+                          (unsigned int)CmdPtr->Payload.DataSize);
+        return CFE_PSP_ERROR;
+    }
+
     /* Write input number of bits to destination address */
     switch (CmdPtr->Payload.DataSize)
     {
@@ -141,6 +169,37 @@ CFE_Status_t MM_PokeEeprom(const MM_PokeCmd_t *CmdPtr, cpuaddr DestAddress)
     uint32       DataValue      = 0;
     size_t       BytesProcessed = 0;
 
+    /*
+     * Defense-in-depth: re-validate the DataSize field before
+     * dispatching to the platform write primitives.  Today the
+     * validity of this field is also enforced upstream in
+     * MM_VerifyPeekPokeParams() before MM_PokeEeprom() is called,
+     * but this function is the one that holds the
+     * CFE_PSP_EepromWrite{8,16,32}() calls and should not depend
+     * on a cross-translation-unit invariant for memory safety.
+     * EEPROM writes are particularly sensitive because they
+     * persist across power cycles --- a silent failure here can
+     * leave the spacecraft in an unintended persistent state
+     * across reboots.  Any future code path that reaches
+     * MM_PokeEeprom() without going through MM_VerifyPeekPokeParams()
+     * (refactor of MM_PokeCmd dispatch, new ground-command entry
+     * that delegates to MM_PokeEeprom, etc.) would otherwise drop
+     * straight into the switch with PSP_Status left at its
+     * initialised value of CFE_PSP_ERROR_NOT_IMPLEMENTED, no
+     * diagnostic event sent, and the perf-log entry/exit pair
+     * unbalanced relative to the actual work performed.
+     */
+    if (CmdPtr->Payload.DataSize != MM_INTERNAL_BYTE_BIT_WIDTH &&
+        CmdPtr->Payload.DataSize != MM_INTERNAL_WORD_BIT_WIDTH &&
+        CmdPtr->Payload.DataSize != MM_INTERNAL_DWORD_BIT_WIDTH)
+    {
+        CFE_EVS_SendEvent(MM_DATA_SIZE_BITS_ERR_EID,
+                          CFE_EVS_EventType_ERROR,
+                          "MM_PokeEeprom invalid data size: %u",
+                          (unsigned int)CmdPtr->Payload.DataSize);
+        return CFE_PSP_ERROR;
+    }
+
     CFE_ES_PerfLogEntry(MM_EEPROM_POKE_PERF_ID);
 
     /* Write input number of bits to destination address */