CTSRD-CHERI / clang

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

taking a reference to a member of a struct via a capability pointer should produce a capability #180

Closed brooksdavis closed 6 years ago

brooksdavis commented 6 years ago

I have a hypothesis that this code should compile without warnings in hybrid mode. Currently it requires a (__cheri_tocap int * __capability) cast in the return statement which seems needlessly awkward. I think it would be more consistent if you had to cast away the capability (and thus the bounds) explicitly rather than discarding the bounds if no cast is present.

struct astruct {
        int     member1;
        int     member2;
};

int * __capability func(struct astruct * __capability asp);

int * __capability
func(struct astruct * __capability asp)
{
        return (&asp->member2);
}

For a more motivating example this is IMO absurd:

static int
pollout(struct thread *td, struct pollfd * __capability fds,
    struct pollfd * __capability ufds, u_int nfd)
{
...
  error = copyout_c((__cheri_tocap short * __capability)&fds->revents,
    (__cheri_tocap short * __capability)&ufds->revents,
    sizeof(ufds->revents));
...
davidchisnall commented 6 years ago

It looks as if we're not propagating the capability annotation into member-of expressions. This is probably easy to fix for C, but it may be exciting for C++ where the member-of operator can be overloaded.

arichardson commented 6 years ago

I believe this is because the address-of operator always gives a pointer in the default address space. In hybrid mode this happens to be a plain pointer so we need the cast after every &. I wonder if we can just infer it based on the LHS of the expression. It might need a bit of refactoring in order to pass that to the address-of code but I believe it should almost always be available (expect when the LHS uses auto or dependent types).

davidchisnall commented 6 years ago

Yes, I guess this is fallout from moving the capability property to the pointer, rather than the object. This sequence is:

  1. Do some pointer arithmetic on a capability
  2. Turn that into an lvalue
  3. Take the address of that lvalue

We are at least passing the capability property through the second step, because if you just dereference the lvalue then you get a capability load / store. Hopefully we can use the same information to fix this, but we'll need it earlier.

davidchisnall commented 6 years ago

As of the latest commit, we now correctly handle this with both struct and array bases, which is hopefully a little bit more consistent than just doing it for arrays.

brooksdavis commented 6 years ago

For the record: the better reason this needs to be true is that there's no guarantee that a capability is inside DDC.