-
Notifications
You must be signed in to change notification settings - Fork 173
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
Maybe unsound in watchdog::descriptor::confirm #1683
Comments
Thanks for you report! |
Hello, @lwz23 !
|
FYI: Since watchdog is a pub mod, this structure and function are also pub. According to the rust security specification, any rust function that may trigger UB should be marked as unsafe or the field should be set to private. Although UB may not be triggered by users in the current project, since this mod may be used by other projects or engineering, for safety reasons, I still recommend setting this field to private if there is no reason to set it to pub. |
Yes, you are right. I'll change it to be unsafe, thank you |
Describe the bug
Hello, thank you for your contribution in this project, I am scanning the unsoundness problem in rust project.
I notice the following code:
Considering that
pub mod watchdog
,pub struct OwnedDescriptor + pub atomic: *const AtomicU64
,andconfirm
is also a pub function. I assume that users can directly call this function and control theatomic
field. This potential situation could result in(*self.atomic)
being operating on a null pointer, I suppose it might trigger undefined behavior (UB). For safety reasons, I felt it necessary to report this issue. If you have performed checks elsewhere that ensure this is safe, please don’t take offense at my raising this issue.I suggest Several possible fixes:
OwnedDescriptor
, it should not marked aspub
at least itsatomic
field should not be pub.confirm
method should add additional check for invalid pointer.confirm
method as unsafe and proper doc to let users know that they should provide valid Pointers.To reproduce
System info
I think it can happen in any platform.
The text was updated successfully, but these errors were encountered: