mm_load.c: validate DataSize locally in MM_PokeMem before dispatch

[nurdymuny]

Summary

MM_PokeMem() in fsw/src/mm_load.c switches on
CmdPtr->Payload.DataSize to choose between
CFE_PSP_MemWrite8/16/32(). The existing comment above the empty
default: case explicitly documents the upstream-validation
reliance:

/*
** We don't need a default case, a bad DataSize will get caught
** in the MM_VerifyPeekPokeParams function and we won't get here
*/
default:
    break;

This is correct under the current MM_PokeCmd() dispatch path, which
always calls MM_VerifyPeekPokeParams() before MM_PokeMem(), but
the local function is the one that actually issues the PSP write
calls and should not depend on a cross-translation-unit invariant
for correctness.

Failure mode

Any future code path that reaches MM_PokeMem() without going
through MM_VerifyPeekPokeParams() — a refactor of MM_PokeCmd
dispatch, a new ground-command entry that delegates to
MM_PokeMem, a test harness, or an EDS-generated wrapper — would
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. The failure
would be silent.

Fix

Add an explicit DataSize check at function entry. On the new error
path, raise MM_DATA_SIZE_BITS_ERR_EID (same EID
MM_VerifyPeekPokeParams() uses for the same condition) and
return CFE_PSP_ERROR so the caller's command-counter machinery
behaves correctly. No behavior change on valid DataSize values.

How I found this

Found by the Davis Manifold geometric vulnerability detector
(`scj-hunt`) while ranking MM functions by attacker-input-handling
density. MM_PokeMem ranked top of the cFS MM bundle under the
psp_memwrite_unguarded compound rule.

Relationship to nasa/fprime#5262

This is the same brick-wall hardening template as
nasa/fprime#5262, which
M. Starch merged earlier today for FileUplink::File::open. Both
PRs address the same shape: a local function whose memory-safety
correctness relies entirely on an upstream type narrowing or
upstream verifier in a different translation unit, with no
diagnostic on the silent-failure path if the upstream invariant is
ever broken.

Future work

The same defensive pattern applies to MM_PokeEeprom,
MM_LoadMem{8,16,32}FromFile, MM_DumpMem{8,16,32}ToFile, and the
MM_FillMem* family. Happy to scope a follow-up sweep covering
those once this template is accepted.

Test plan

• No behaviour change on valid DataSize
  (BYTE_BIT_WIDTH / WORD_BIT_WIDTH / DWORD_BIT_WIDTH).
• On invalid DataSize, function now returns CFE_PSP_ERROR
  and raises MM_DATA_SIZE_BITS_ERR_EID instead of returning the
  initialised CFE_PSP_ERROR_NOT_IMPLEMENTED with no event.
• Existing unit tests in unit-test/mm_load_tests.c should
  continue to pass.
• A unit test exercising the new error path would be valuable; I am
  happy to add one if reviewers can advise whether to put it under
  MM_PokeMem_Test_* in mm_load_tests.c or as a stand-alone
  fault-injection case.

[nurdymuny]

Extended this PR with a second commit (4eeced8) applying the identical hardening template to MM_PokeEeprom().

Identical bug shape: 3-way switch on CmdPtr->Payload.DataSize, brick-wall reliance on MM_VerifyPeekPokeParams(), same default: break; with the same doc-comment about the upstream validator. EEPROM writes are particularly worth hardening because they persist across power cycles — a silent failure from a cross-translation-unit invariant breaking would leave the spacecraft in an unintended persistent state across reboots.

Methodology note: MM_PokeEeprom wasn't surfaced by the original psp_memwrite_unguarded compound rule (which keys off CFE_PSP_MemWrite{8,16,32} sinks, not the EEPROM variants CFE_PSP_EepromWrite{8,16,32} — a gap in the catalog). It was surfaced by a patch-twin query using the new gigi brain primitive ATTEND:

$ scj-hunt twins --bundle cfs_apps_mm_v01 --of MM_PokeMem
rank 12: MM_PokeEeprom    score 5.5

ATTEND clustered the two functions together based on their structural similarity (same dispatch shape, same brick-wall pattern, same doc-comment) even though the MM_PokeMem rule didn't fire on MM_PokeEeprom. This is the patch-twin discovery the Davis Manifold framework's WISH / ATTEND verbs are designed for — finding sibling-shaped code the keyword-based rules missed.

Happy to continue the sweep on MM_LoadMem{8,16,32}FromFile, MM_DumpMemToFile, and the MM_FillMem family if the same template applied to the EEPROM case lands cleanly — ATTEND ranked MM_LoadMemWIDCmd (the WID dispatcher for the LoadMem family) at rank 3 right above MM_PokeEeprom.

[nurdymuny]

Quick follow-up on the LoadMem / DumpMem / FillMem family sweep I offered to do.

I swept all the remaining family functions and the literal pattern doesn't recur — MM_PokeMem and MM_PokeEeprom look like the only two that share the brick-wall shape. Reporting honestly rather than padding the PR:

Function	Pattern	Already defended?
MM_LoadMemFromFileCmd	switch (MMFileHeader.MemType)	yes — default: Status = OS_ERROR; at mm_cmds.c:543
MM_FillMemCmd	switch (Msg->Payload.MemType)	yes — default: Status = CFE_PSP_ERROR; at mm_cmds.c:690
MM_DumpMemToFileCmd	switch (MMFileHeader.MemType)	yes — default: Status = CFE_PSP_ERROR; at mm_cmds.c:874
MM_LoadMemWIDCmd	no switch — MM_VerifyLoadDumpParams + memcpy	n/a — different shape
MM_DumpInEventCmd	no switch — MM_VerifyLoadDumpParams	n/a — different shape
MM_FillMem8 / 16 / 32	type pre-specialized by file (MEM8 / MEM16 / MEM32)	n/a — no DataSize axis

So this PR stays at the two original commits. I'd rather report a clean negative than upsize the diff for the sake of it.

If you'd prefer a separate hardening pass on something else in MM that is worth doing — e.g. tightening the MM_VerifyPeekPokeParams family or adding a static_assert that the three MM_INTERNAL_*_BIT_WIDTH constants stay in sync with the switch arms — I'm happy to file that as a fresh PR. Just say the word.