Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core Armv8 ARM_MPU_SetMemAttrEx include undefined behaviour #169

Open
bentank opened this issue May 24, 2024 · 1 comment
Open

Core Armv8 ARM_MPU_SetMemAttrEx include undefined behaviour #169

bentank opened this issue May 24, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@bentank
Copy link

bentank commented May 24, 2024

__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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants