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

Differentiate "get capability value" and "get capability low half" #341

Open
nwf-msr opened this issue Aug 7, 2024 · 10 comments
Open

Differentiate "get capability value" and "get capability low half" #341

nwf-msr opened this issue Aug 7, 2024 · 10 comments

Comments

@nwf-msr
Copy link

nwf-msr commented Aug 7, 2024

(On second thought, moving this out from #340)

There's a subtle assumption in most CHERI work to date that the address field of a capability is a full machine word and is one half of the CHERI capability. There are a few reasons that might not always be true.

See https://github.com/CTSRD-CHERI/cheri-specification/wiki/Tracking-discussion-for-CGetAddr-vs-CToPtr-vs-as-integer-alias-vs-CGetLow for (many) more but incomplete thoughts on untangling this knot. It might be nice if the base Zcheri specification took this possible division into account and reserved the relevant opcodes, even if most CHERI capability encodings will have identical behavior for some pairs.

@jrtc27
Copy link
Collaborator

jrtc27 commented Aug 7, 2024

PAC and MTE, and all the various TBI things, expose the non-address portion in C as part of the address. Not doing so is unnecessary complication, and becomes problematic very quickly for uintptr_t code generation.

@nwf-msr
Copy link
Author

nwf-msr commented Aug 7, 2024

But we can't permit CSetAddr to change the MTE tag, for example.

@jrtc27
Copy link
Collaborator

jrtc27 commented Aug 7, 2024

uintptr_t needs to have full XLEN range for arithmetic, that's non-negotiable in the real world. That means the address needs to be the full XLEN bits.

@davidchisnall
Copy link

uintptr_t needs to have full XLEN range for arithmetic, that's non-negotiable in the real world. That means the address needs to be the full XLEN bits.

Not quite. uintptr_t needs to have a full XLEN range for arithmetic on untagged values. For tagged values, it does not have this range in practice because the capability bounds never permit it. The complexity in separating the get/set address and get/set low bits comes from the fact that address accessing needs to behave differently based on the tag bit, but that's a handful of gates.

@jrtc27
Copy link
Collaborator

jrtc27 commented Aug 7, 2024

uintptr_t needs to have full XLEN range for arithmetic, that's non-negotiable in the real world. That means the address needs to be the full XLEN bits.

Not quite. uintptr_t needs to have a full XLEN range for arithmetic on untagged values. For tagged values, it does not have this range in practice because the capability bounds never permit it. The complexity in separating the get/set address and get/set low bits comes from the fact that address accessing needs to behave differently based on the tag bit, but that's a handful of gates.

I don't think a world where CGetAddr(CClearTag(foo)) != CGetAddr(foo) is a good idea. We've been down roads like that before and it's been porting pain.

@davidchisnall
Copy link

I don't think a world where CGetAddr(CClearTag(foo)) != CGetAddr(foo) is a good idea. We've been down roads like that before and it's been porting pain.

I'd like to see a concrete example of something that this would break. I've not encountered one yet, but I can believe it would exist.

@jrtc27
Copy link
Collaborator

jrtc27 commented Aug 7, 2024

I don't think a world where CGetAddr(CClearTag(foo)) != CGetAddr(foo) is a good idea. We've been down roads like that before and it's been porting pain.

I'd like to see a concrete example of something that this would break. I've not encountered one yet, but I can believe it would exist.

T *old_p = p; /* or uintptr_t, as commonly done to silence use-after-realloc warnings, or ptraddr_t */
T *p = realloc(p, new_size)
/* update all the pointers in p based on their offset from old_p */ 

If revocation occurs during the update thanks to some other thread's free, the addresses seen for the pointers to the old allocation will change, because they'll no longer be tagged, giving incorrect offsets.

One can argue about what ISO C actually says about whether this is defined behaviour, but this is a real world pattern seen all over the place that would break if the address changes when the tag is cleared.

@davidchisnall
Copy link

I see. Do you have a reference to where you've seen that? I've heard people mention it but never actually seen it.

@jrtc27
Copy link
Collaborator

jrtc27 commented Aug 7, 2024

I see. Do you have a reference to where you've seen that? I've heard people mention it but never actually seen it.

We found some in the desktop porting work that had the wrong provenance, so here are a few from that that come to mind. There are surely many others though, I remember seeing others, just not what they were in.

https://gitlab.freedesktop.org/xorg/xserver/-/blob/386b54fbe95711e6ecb5c23cfdbf25a1571acf7b/dix/privates.c#L203
https://gitlab.freedesktop.org/xorg/lib/libx11/-/blob/ed9fb5535efe1e5278654b6b3994a34337b4bf1a/src/xlibi18n/lcDB.c#L517
https://gitlab.freedesktop.org/xorg/lib/libx11/-/blob/ed9fb5535efe1e5278654b6b3994a34337b4bf1a/modules/im/ximcp/imLcPrs.c#L682

We're also often only aware of the ones that get the provenance wrong, the rest work fine and don't show up when porting.

@davidchisnall
Copy link

Thanks. I shouldn’t be surprised it’s xlib.

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