On 6 Jan 2022, at 23:22, brian m. carlson <sandals@xxxxxxxxxxxxxxxxxxxx> wrote: > > On 2022-01-05 at 13:23:24, Jessica Clarke wrote: >> Currently git_qsort_s allocates a buffer on the stack that has no >> alignment, and mem_pool_alloc assumes uintmax_t's size is adequate >> alignment for any type. >> >> On CHERI, and thus Arm's Morello prototype, pointers are implemented as >> hardware capabilities which, as well as having a normal integer address, >> have additional bounds, permissions and other metadata in a second word, >> so on a 64-bit architecture they are 128-bit quantities, including their >> alignment requirements. Despite being 128-bit, their integer component >> is still only a 64-bit field, so uintmax_t remains 64-bit, and therefore >> uintmax_t does not sufficiently align an allocation. >> >> Moreover, these capabilities have an additional "129th" tag bit, which >> tracks the validity of the capability and is cleared on any invalid >> operation that doesn't trap (e.g. partially overwriting a capability >> will invalidate it) which, combined with the architecture's strict >> checks on capability manipulation instructions, ensures it is >> architecturally impossible to construct a capability that gives more >> rights than those you were given in the first place. To store these tag >> bits, each capability sized and aligned word in memory gains a single >> tag bit that is stored in unaddressable (to the processor) memory. This >> means that it is impossible to store a capability at an unaligned >> address: a normal load or store of a capability will always take an >> alignment fault even if the (micro)architecture supports unaligned >> loads/stores for other data types, and a memcpy will, if the destination >> is not appropriately aligned, copy the byte representation but lose the >> tag, meaning that if it is eventually copied back and loaded from an >> aligned location any attempt to dereference it will trap with a tag >> fault. Thus, even char buffers that are memcpy'ed to or from must be >> properly aligned on CHERI architectures if they are to hold pointers. > > I think this is going to be a problem in a lot of places, not just in > Git. I'm pretty sure that copying data this way is specifically allowed > by C and POSIX, and thus this approach is going to break a whole lot of > things. The standard says you can copy anything to an unsigned char array to get its object representation. This we support just fine. The standard does not say you can copy that object representation back to an object of the original type and get a valid object of the original type. We only support that if the unsigned char array is aligned enough. Technically git is already wrong here by using char not unsigned char, though that doesn’t make a difference for CHERI, nor any implementation I know of. > For example, casting a void * to uintptr_t and back should produce two > pointers that compare equal. The C standard says that two pointers > compare equal if they're both null, both point to the same object, or > one points one past the end of an array and the other happens to point > to the beginning of another object. If the pointers aren't null and the > original one points to valid data, then the resulting pointer (after the > two casts) would point to the same object (since that's the only valid > option that's left), and therefore could be used to access it, but that > wouldn't necessarily work in this case. All of this CHERI supports. Casting to uintptr_t and back works just fine. > The CHERI paper I'm reading also specifically says it is not changing > uintmax_t, which is a direct violation of the C standard. If uintptr_t > must be larger than 64 bits, then so must uintmax_t, even if that > happens to be inconvenient (because it changes the ABI from the normal > system ABI). It sounds like, in fact, that you can't actually provide > uintptr_t with the current architecture, because it can't be provided in > a standard-compliant way. Every C implementation that provides 128-bit integers violates this. Nothing cares. As explained in my reply to Junio this thread it is not possible to provide a uintmax_t that can hold a uintptr_t on CHERI in general. So we don’t. And almost no software cares. Not providing a uintptr_t breaks basically all systems code ever, except the code that’s so old it assumes unsigned long is the right type and is already broken for CHERI as a result. > Is there something I'm missing here, or is it the case that CHERI's > behavior isn't compliant with the C standard? There are some cases where we’re slightly non-compliant. So is every implementation that exists. We believe the few restrictions we add, which can all be easily worked around, are vastly outweighed by the benefit of having hardware that enforces fine-grained memory safety efficiently, and the empirical evidence is that these restrictions are not an issue for the vast majority of real-world C code. Jess
