EEPROM_Emul memory corruption when FLASH_LINES_128B + EE_ReadVariable32bits

[SuperFriendlyGuy]

Hello,
I have filed an issue but thought I would post this here for reference, this bug took me quite a while to hunt down and hope to save others time possibly:


I have found a memory-safety defect in EEPROM_Emul on STM32U5 128-bit flash-line config.

Config:
- FLASH_LINES_128B enabled
- EE_ACCESS_32BITS enabled

Issue:
- EE_ReadVariable32bits() passes a single EE_DATA_TYPE buffer:
Core/EEPROM_Emul/Core/eeprom_emul.c:758, :762
- In FLASH_LINES_128B mode, ReadVariable() writes two words:
Core/EEPROM_Emul/Core/eeprom_emul.c:1184, :1185
- This writes beyond the caller’s single-word buffer and can corrupt stack/register state (we observed HardFault with corrupted stacked PC).

Why “wrong API usage” is not a valid dismissal:
1) The API is publicly exposed in this build mode (Core/EEPROM_Emul/Core/eeprom_emul.h:126 with FLASH_LINES_128B APIs also present at :130).
2) An exported API must not corrupt memory under any call pattern; unsupported combinations must fail explicitly.
3) This component already uses status-return semantics, so unsupported mode should return an error, not perform out-of-bounds write.

Expected fix:
- Either make 32/16/8-bit read APIs unavailable when FLASH_LINES_128B is enabled (compile-time guard),
- Or keep them supported by using a local 2-word buffer internally and returning only the intended value,
- Or return a clear error code for unsupported mode before touching caller buffers.

Requested:
- Confirm defect
- Provide upstream patch/version where fixed

[TDK]

The latest X-CUBE-EEPROM code (v8.0.0) appears to already do this. Data is read out to a (64-bit) temporary buffer than then only a uint32_t is written to the pointer.

Since the line numbers in your post don't match up with what's in the latest code, I suspect you are using out of date code and the issue has already been fixed. Updating to the latest release should resolve your issues.

/**
  * @brief  Returns the last stored variable data, if found, which correspond to
  *         the passed virtual address
  *  VirtAddress Variable virtual address on 16 bits (can't be 0x0000 or 0xFFFF)
  *   pData Variable containing the 32bits read variable value
  * @retval EE_Status
  *           - EE_OK: if variable was found
  *           - EE error code: if an error occurs
  */
EE_Status EE_ReadVariable32bits(uint16_t VirtAddress, uint32_t* pData)
{
  if((VirtAddress != 0x0000) && (VirtAddress != 0xFFFF))
  {
    EE_DATA_TYPE datatmp = 0U;
    EE_Status status = EE_OK;

    /* Read variable of size EE_DATA_TYPE, then cast it to 32bits */
    status = ReadVariable(VirtAddress, &datatmp);
    *pData = (uint32_t) datatmp;

    return status;
  }
  else
  {
    return EE_INVALID_VIRTUALADDRESS;
  } 
}

[SuperFriendlyGuy]

Hello,

Thank you for the reply! I will look into updating the code in the project.

However, from inspection I believe the pattern you quoted is not inherently safe for FLASH_LINES_128B, and in this codebase it is demonstrably unsafe.

Based on a source audit of this repo:

- FLASH_LINES_128B is enabled in the build.
- EE_DATA_TYPE is uint64_t (8 bytes).
- In 128B mode, ReadVariable writes two uint64_t values (16 bytes total).
- Alignment and access semantics assume a uint64_t pointer with two elements.

Given that, the EE_ReadVariable32bits pattern you quoted:

- allocates a single EE_DATA_TYPE local (8 bytes),
- passes its address to ReadVariable,
- then truncates to 32 bits after the call, can still overflow the local buffer in 128B mode, because the internal writes
two 64-bit outputs.

The truncation happens after the overwrite and does not prevent stack corruption.

The write side has the same class of issue: the ST 8/16/32-bit write APIs pass undersized buffers while the 128B internals read two 64-bit words.

In this repo, the robust fix is at the wrapper level:
- for FLASH_LINES_128B, allocate uint64_t[2],
- call the explicit 96-bit read/write APIs,
- then truncate or pack at the wrapper boundary.

That matches the internal expectations and avoids the memory safety issue entirely. All current application call paths go through those wrappers, so the problem is mitigated here.