https://github.com/pcc commented:
Hi Oliver, thanks for your comments! I'll address them below.
> Thoughts:
>
> This should be opt-in on a field or struct granularity, not just a global
> behavior.
This would certainly be easier if it were an opt-in behavior, as it would allow
avoiding a substantial amount of complexity that you point out below. The
problem is that would not make the solution very effective as a UAF mitigation,
as it would only help with code that was opted in, which is likely a tiny
fraction of an entire codebase. That fraction is unlikely to be the fraction
with UAF bugs, not only because of the size of the fraction but also because if
developers aren't thinking about memory safety issues, they certainly won't be
manually opting into this. With PFP, the problem that I set out to solve was to
find a way to automatically protect as many pointers stored in memory as
possible in standards-conforming code.
> 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?
This is based on whether the type has standard layout. C does not permit
declaring a type that is non-standard layout.
> 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.
Agreed on type queries. The current adjustment to how trivially-relocatable is
defined is a workaround that will be removed once P2786 is fully implemented in
libc++, and at that point we shouldn't need `__has_non_relocatable_fields`
either. The intent is that turning on PFP should be invisible to the developer
(in other words, the modification to the storage format of pointer fields in
certain structs is permitted by the as-if rule). As a result, most of the
implementation is in CodeGen and below.
> 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
>
> ```c++
> 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:
>
> ```c++
> 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
>
> ```c++
> 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.
In the early stages of the project, I wanted to make it so that the PFP fields
would implicitly receive qualifiers, similar to what you proposed, and I was
trying to come up with an approach that would make it work, but I couldn't see
a way because of the inter-convertibility of pointers from different structs
(and pointers outside of structs entirely), and disallowing conversions between
differently qualified pointer types would render the compiler unable to compile
standard C++. The only way that I could see to make it work was to utilize the
as-if rule and invisibly modify the in-memory representation.
For pointers in standard-layout types (i.e. pointers that explicitly opt in) I
agree with you that the right approach would be to use a qualifier, but I think
that should be done as a separate change, possibly as a followup. In this
change, I would like to implement the solution for implicitly protected fields.
> 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.
Yes, something like that is how I would imagine implementing this for opt-in
fields.
https://github.com/llvm/llvm-project/pull/133538
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits
