CMSIS_6 icon indicating copy to clipboard operation
CMSIS_6 copied to clipboard

Core Armv8 ARM_MPU_SetMemAttrEx include undefined behaviour

Open bentank opened this issue 1 year ago • 1 comments

__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);
}

bentank avatar May 24 '24 21:05 bentank

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

JonatanAntoni avatar Jun 03 '24 08:06 JonatanAntoni