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

Too strict requirments for TaggedPtr::new and ::get/::ptr? #4

Closed
Alibenbaba opened this issue Dec 17, 2023 · 5 comments
Closed

Too strict requirments for TaggedPtr::new and ::get/::ptr? #4

Alibenbaba opened this issue Dec 17, 2023 · 5 comments

Comments

@Alibenbaba
Copy link

The documentation requires a dereferenceable pointer provided to ::new.
This seems like a too strict precondition, even the cited core::ptr only restricts read and write operations to dereferenceable pointers.

So, from my understanding, either the documentation is too strict, or the implementation of ::new/::get/::ptr needs to reserve the option to read or write to the pointer passed in order to tag it.

This basically prevents an allocator to have a tagged end-pointer (pointing just beyond the allocated memory) - such an end pointer is not dereferenceable, and thus not useable in a TaggedPtr.

Did I miss something?

@taylordotfish
Copy link
Owner

Tagging a one-past-the-end pointer might work, but the issue is that once you add the tag, the pointer now points to memory strictly outside of the allocated object, which may make some assumptions invalid (will the provenance still be valid once the original pointer is recovered, will align_offset still be (practically) guaranteed to work, etc.).

I would have to have more assurance that end pointer tagging wouldn't cause unsoundness or spurious failures or subtly alter the characteristics of stored pointers before I would feel comfortable relaxing the conditions.

@Alxandr
Copy link

Alxandr commented Feb 10, 2024

Am I to read this as you can't also tag NonNull::dangling()? Also - aren't you allowed to have a pointer that points outside of it's provenance as long as you never dereference it? I mean, I assume you remove the tag before actually trying to read/write to it.

@taylordotfish
Copy link
Owner

I was mostly concerned about this part of std::ptr's documentation on strict provenance:

Notably, CHERI relies on a compression scheme that can’t handle a pointer getting offset “too far” out of bounds. If this happens, the address returned by addr will be the value you expect, but the provenance will get invalidated and using it to read/write will fault.

But that's admittedly an implementation detail of CHERI, and one that's unlikely to occur in practice (given the “kilobytes” of offset the documentation implies are necessary to trigger the phenomenon).

It does look like both the case of tagging one-past-the-end pointers and tagging NonNull::dangling() work reliably in Miri (with -Zmiri-symbolic-alignment-check -Zmiri-strict-provenance), so I'll look into relaxing the constraints. I do want to clarify the guarantees about the pointer you get back out of TaggedPtr::get/ptr in that case—maybe specifying that it may be the result of adding, then subtracting, at most 2BITS - 1 bytes to the original pointer, but is otherwise equivalent. Individual users can decide whether that provides sufficient guarantees for their case.

@Alxandr
Copy link

Alxandr commented Feb 11, 2024

I decided to forgo pointer tagging (for my usecase) cause I had a perfectly good cap: usize that I could tag instead - to not have to think about provance and valid pointers :P. But I agree that this should probably be clarified a bit. Also, I also had an issue with the constructor not being const - so that's maybe something you want to do something about?

@taylordotfish
Copy link
Owner

The constructor can't be const because pointer::align_offset isn't const. The constructor uses it to verify the alignment before adding the tag. However, if/when const_pointer_is_aligned is stabilized, a const version should be possible.

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

No branches or pull requests

3 participants