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

Safety: intern::symbol::TaggedArcPtr::try_as_arc_owned is unsafe #18499

Open
sam-mccall opened this issue Nov 9, 2024 · 1 comment
Open

Safety: intern::symbol::TaggedArcPtr::try_as_arc_owned is unsafe #18499

sam-mccall opened this issue Nov 9, 2024 · 1 comment
Labels
C-bug Category: bug

Comments

@sam-mccall
Copy link
Contributor

An audit of unsafe in ra_ap_intern found this possible issue:

pub(crate) fn try_as_arc_owned(self) -> Option<ManuallyDrop<Arc<Box<str>>>> {
// Unpack the tag from the alignment niche
let tag = Strict::addr(self.packed.as_ptr()) & Self::BOOL_BITS;
if tag != 0 {
// Safety: We checked that the tag is non-zero -> true, so we are pointing to the data offset of an `Arc`
Some(ManuallyDrop::new(unsafe {
Arc::from_raw(self.pointer().as_ptr().cast::<Box<str>>())
}))
} else {
None
}
}

This must be unsafe, because the safety invariants of ManuallyDrop permit patterns that would be unsound, like calling try_as_arc_owned twice on the same TaggedArcPtr and invoking ManuallyDrop::drop or ::take on both of those two separate ManuallyDrop values, easily producing a use-after-free.

This is a non-blocking issue because it is an internal-only interface which is used correctly.

This doesn't prevent our use of rust-analyzer, but I'm sharing it here in case you agree and want to clarify the safety in the code.

@sam-mccall sam-mccall added the C-bug Category: bug label Nov 9, 2024
@ChayimFriedman2
Copy link
Contributor

As said, this method is pub(crate), so it doesn't have to be unsafe. Still this would be an improvement, and I think a PR will be fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: bug
Projects
None yet
Development

No branches or pull requests

2 participants