pcc wrote:

> I have some small reservations about using ifunc resolvers like this. Mostly 
> in that we are using a mechanism invented for a different purpose, and 
> relying on some specific linker behaviour to make this case work.
>
> This is similar to comments made in the Discourse post 
> https://discourse.llvm.org/t/rfc-structure-protection-a-family-of-uaf-mitigation-techniques/85555/9
>  but repeating them here as this is closest to the implementation.
>
> As I understand it, this has a more limited and more specific use case than 
> ifuncs. Traditional ifuncs which can be address taken or called, possibly in 
> multiple ways, so it makes sense to use a symbol type STT_GNU_IFUNC rather 
> than special relocation directives. The initializers for structure field 
> protection are compiler generated, can not be legally called or address taken 
> from user code, and only have one relocation type R_ABS64 (or 32 on a 32-bit 
> platform). With an addition of a single relocation, something like 
> R_ADDRINIT64 which would target a STT_FUNC resolver symbol. We can isolate 
> the structure field initialization use case from an actual ifunc.

I see, so the idea would be that an ADDRINIT64 pointing to an STT_FUNC would 
generate an IRELATIVE pointing to the symbol address if the symbol is 
non-preemptible and result in a linker error otherwise.

> I guess it all comes down to whether structure field initialization needs, or 
> benefits from being distinguished from an ifunc. Ifuncs seem to be quite easy 
> to get wrong so being able to isolate this case has some attraction to me at 
> least. It also handles the structure field that points to an ifunc relatively 
> gracefully.

I don't necessarily see usage errors as a convincing argument for ADDRINIT64. 
Because the relocations and resolvers are compiler generated (and the resolvers 
cannot be named by the user due to the use of local symbols) it is hard to 
commit a usage error if you are not a compiler developer who is expected to 
know what they are doing.

I do think that there are two convincing arguments for ADDRINIT64:
- We want linkers that do not support the new behavior to loudly error out 
instead of silently generating incorrect code. You'll note that this is the 
same argument that justifies INST32 (or whatever we end up calling it) when 
ABS32 already exists and could be made to retrofit the semantics that we want.
- It reduces linker complexity because the IRELATIVE relocations can be added 
in a single pass.

> As you pointed out in your response, this does mean adding 2 relocations to 
> every psABI that supports structure field protection rather than just one. 
> Although I'd expect the alternative of having relocations that alternatively 
> write "directly call" ifunc resolver or take address of function might 
> require new relocations too?

Yes, we'd still the relocation to select between calling the resolver and 
taking the address unless we decide to not support other use cases for the 
inner ifuncs (in which case we'd just generate code to take the address and the 
IPLT would be used as the address of affected ifuncs). This may be fine as it's 
the status quo for ifuncs referred to by an ABS64.

https://github.com/llvm/llvm-project/pull/133531
_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

Reply via email to