https://github.com/ojhunt requested changes to this pull request.
Thoughts:
This should be opt-in on a field or struct granularity, not just a global
behavior.
In the RFC I think you mentioned not applying PFP to C types, but I'm unsure
how you're deciding what is a C type?
There are a lot of special cases being added in places that should not need to
be aware of PFP - the core type queries should be returning correct values for
types containing PFP fields.
A lot of the "this read/write/copy/init special work" exactly aligns with the
constraints of pointer auth, and I think the overall implementation could be
made better by introducing the concept of a `HardenedPointerQualifier` or
similar, that could be either PointerAuthQualifier or PFPQualifier. In
principle this could even just be treated as a different PointerAuth key set.
That approach would also help with some of the problems you have with offsetof
and pointers to PFP fields - the thing that causes the problems with the
pointer to a PFP field is that the PFP schema for a given field is in reality
part of the type, so given
```cpp
struct A {
void *field1;
void *field2;
};
```
It looks you are trying to maintain the idea that `A::field1` and `A::field2`
have the same type, which makes this code valid according to the type system:
```cpp
void f(void** obj);
void g(A *a) {
f(&a->field1);
f(&a->field2);
}
```
but the real types are not the same. The semantic equivalent under pointer auth
is something akin to
```cpp
struct A {
void * __ptrauth(1, 1, hash("A::field1")) field1;
void * __ptrauth(1, 1, hash("A::field2")) field2;
};
```
Which makes it very clear that the types are different, and I think trying to
pretend they are not is part of what is causing problems.
The more I think about it, the more I feel that this could be implemented
almost entirely on top of the existing pointer auth work.
Currently for pointer auth there's a set of ARM keys specified, and I think you
just need to create a different set, PFP_Keys or similar, and set up the
appropriate schemas (basically null schemas for all the existing cases), and
add a new schema: DefaultFieldSchema or similar, and apply that schema to every
pointer field in an object unless there's an explicit qualifier applied already.
Then it would in principle just be a matter of making adding the appropriate
logic to the ptrauth intrinsics in llvm.
Now there are some moderately large caveats to that idea
* the existing ptrauth semantics simply say that any struct containing address
discriminated values is no-longer a pod type and so gets copied through the
stack, which is something you're trying to avoid, so you'd still need to keep
that logic (though as I said, that should be done by generating and then
calling the functions to do rather than regenerating the entire copy
repeatedly).
* the way that pointer auth is currently implemented assumes that there is only
a single pointer auth abi active so you wouldn't be able to trivially have both
pointer auth and PFP live at the same time. That's what makes me think having a
SecuredPointer abstraction might be a batter approach, but it might also not be
too difficult to update the PointerAuthQualifier to include the ABI being used
-- I'm really not sure.
https://github.com/llvm/llvm-project/pull/133538
_______________________________________________
llvm-branch-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
