ARM-software / CMSIS_6

CMSIS version 6 (successor of CMSIS_5)
https://arm-software.github.io/CMSIS_6/
Apache License 2.0
194 stars 64 forks source link

Core Armv8 ARM_MPU_SetMemAttrEx include undefined behaviour #169

Open bentank opened 6 months ago

bentank commented 6 months ago
__STATIC_INLINE void ARM_MPU_SetMemAttrEx(MPU_Type* mpu, uint8_t idx, uint8_t attr)
{
  const uint8_t reg = idx / 4U;
  const uint32_t pos = ((idx % 4U) * 8U);
  const uint32_t mask = 0xFFU << pos;

  if (reg >= (sizeof(mpu->MAIR) / sizeof(mpu->MAIR[0]))) {
    return; // invalid index
  }

  mpu->MAIR[reg] = ((mpu->MAIR[reg] & ~mask) | ((attr << pos) & mask));
}

Specifically attr << pos has the potential to shift uint8_t attr greater than its width causing UB

A simple fix would be to promote attr to 32 bit before shifting.

__STATIC_INLINE void ARM_MPU_SetMemAttrEx(MPU_Type* mpu, uint8_t idx, uint8_t attr)
{
  const uint8_t reg = idx / 4U;
  const uint32_t pos = ((idx % 4U) * 8U);
  const uint32_t mask = 0xFFU << pos;
  const uint32_t val = (uint32_t)attr << pos;

  if (reg >= (sizeof(mpu->MAIR) / sizeof(mpu->MAIR[0]))) {
    return; // invalid index
  }

  mpu->MAIR[reg] = (mpu->MAIR[reg] & ~mask) | (val & mask);
}
JonatanAntoni commented 5 months ago

@bentank, thanks for raising this. May I ask you to migrate your PR to this new repo as well?