CTSRD-CHERI / clang

DO NOT USE. Use llvm-project instead
Other
9 stars 8 forks source link

Add a warning when casting from capability to pointer (and make C++ implicit capability conversions work like they do in C) #126

Closed arichardson closed 7 years ago

arichardson commented 7 years ago

Only (u)intptr_t and vaddr_t (aka __memaddr_t) are acceptable cast targets

This is quite noisy when compiling cheribsd hybrid mode code so I'm not sure I should add this yet.

I also have a change for cheribsd that tags vaddr_t as attribute((memory_address)) and fixes a few cast warnings. There are a lot of cases where we are casting capabilities to the corresponging pointer type in the hybrid ABI and I'm not sure how best to silence those? Possibly with a __builtin_cheri_cap_to_pointer() builtin?

arichardson commented 7 years ago

What do you suggest we do about the foo* __capability -> foo * casts? Silence the warning for pointer casts in hybrid mode or add some explicit casting?

A lot of them are in cheriabi_sysstubs.h, some in libc_c where we do void*__capability x = (void*__capability)calloc(...) and then later free((void*)x)

khilangudka commented 7 years ago

I would vote for explicit casting so we know all the places where such a conversion is occurring.

brooksdavis commented 7 years ago

Hmm. In the case of lib/libcheri_syscalls/cheri_syscalls.c we can change which casts we make, but it would be nice to not have do have an #ifdef between hybrid and pure modes. I suppose that since this is boundary code, it would be ok at have the moral equivalent of __DECONST() macros.

davidchisnall commented 7 years ago

We've so far had a policy of requiring all casts between X* and __capability X* to be explicit. Casts from the __capability version to the non-__capability version are not valid in the general case and so should be checked by a human. Casts in the other direction are valid, but should come with explicit bounds truncation and so should also be checked by a human.

The same should apply for all casts from capability to and from __intcap_t, though casts between __intcap_t and integer types are probably fine as implicit casts.

brooksdavis commented 7 years ago

The things in cheri_syscalls.c are of the form:

int
foo_stub(void __capability *arg)
{

    return (foo((void *)arg);
}

The (void *)arg cast is intended to be a NOP in pure-cability mode and cast away the capability in hybrid mode. I don't much care how we spell it, but being able to express that in C rather than CPP would be nice.

davidchisnall commented 7 years ago

(For reference, writing up some of the face-to-face discussion).

We should add explicit cast kinds for these modelled on Objective-C bridging casts. The following should not cause warnings:

void * __capability cap = ...;
void *x = (__cheri_cast void*)cap;
cap = (__cheri_cast void * __capability)x;

This code can then be made compatible with non-CHERI systems with a #define __cheri_cast in a header.

We also pondered adding a __cheri_bounded_cast for non-capability to capability casts, which would set the bounds of the new pointer to the size of the pointee. Arrays would need an explicit bounds setting operation (though we might want to accept (__cheri_bounded_cast struct foo[42]) and (__cheri_bounded_cast struct foo[x])to cast to a pointer with a bounded length.

brooksdavis commented 7 years ago

__cheri_cast sounds good to me.

arichardson commented 7 years ago

This now also contains commits that disallow implicit conversions from capabilities to pointer/pointers to capabilities in C++ because splitting that into a separate pull request would require some nontrivial history rewriting