Skip to content
New issue

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

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

Already on GitHub? # to your account

Mark init function as unsafe #16

Open
kitcatier opened this issue Mar 17, 2023 · 3 comments
Open

Mark init function as unsafe #16

kitcatier opened this issue Mar 17, 2023 · 3 comments

Comments

@kitcatier
Copy link

Hello, I found a soundness issue in this crate.

bkernel/stm32f4/nvic.rs

Lines 113 to 130 in 521e100

pub fn init(nvic: &NvicInit) {
unsafe {
if nvic.enable {
let mut tmppriority = (0x700 - (AIRCR.get() & 0x700)) >> 0x08;
let tmppre = 0x4 - tmppriority;
let tmpsub = 0x0F >> tmppriority;
tmppriority = u32::from(nvic.priority) << tmppre;
tmppriority |= u32::from(nvic.subpriority) & tmpsub;
tmppriority <<= 0x04;
IPR[nvic.irq_channel as usize].set(tmppriority);
ISER[nvic.irq_channel as usize >> 5].set(0x1 << (nvic.irq_channel as u8 & 0x1F));
} else {
ICER[nvic.irq_channel as usize >> 5].set(0x1 << (nvic.irq_channel as u8 & 0x1F));
}
}
}

It is not a good choice to mark the entire function body as unsafe, which will make the caller ignore the safety requirements that the function parameters must guarantee.
Marking them unsafe also means that callers must make sure they know what they're doing.

@rasendubi
Copy link
Owner

rasendubi commented Mar 17, 2023 via email

@kitcatier
Copy link
Author

Cool, what unsoundness is possible?

get()/set() functions needs to ensure that the parameter must be:
https://doc.rust-lang.org/core/intrinsics/fn.volatile_load.html

  • src must be valid for reads.
  • src must be properly aligned.
  • src must point to a properly initialized value of type T.
    https://doc.rust-lang.org/core/ptr/fn.write_volatile.html
  • dst must be valid for writes.
  • dst must be properly aligned.
    It seems that the safety requirements of the parameters are not clear to others callers.

@rasendubi
Copy link
Owner

rasendubi commented Mar 17, 2023 via email

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants