[clang] [llvm] [X86] Enhance kCFI type IDs with a 3-bit arity indicator. (PR #117121)
maurer wrote: This is not a Rust concern, but re-reading the initial post, it *looks* like your own statistics suggest that consuming 3 bits for arity costs more than it buys you. As stated, (didn't check your math, just going off what you said) prior to your change, we expect 0.01383765 collisions in your sample environment. After your change, we expect to have the *sum* of your right hand column in collisions, which comes out to 0.0266774 - nearly double the rate of collisions we have with the basic implementation. In fact, I think that any scheme like this will always going to increase the number of overall collisions, given that the arity is implicitly hashed into the representation already. The main reason I could see to consider this is if for some reason a cross-arity collision is more dangerous than a same-arity collision in terms of exploitability, which I can't immediately argue, but perhaps you have something for this that was just assumed in the initial post? https://github.com/llvm/llvm-project/pull/117121 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86] Enhance kCFI type IDs with a 3-bit arity indicator. (PR #117121)
maurer wrote: Flag guarding this feature seems like it would also be good for any existing C users - for example, if trying to build a kernel module intended to load against a kernel image built with an older `clang`, you need to select the same type ID projection that the kernel did. https://github.com/llvm/llvm-project/pull/117121 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86] Enhance kCFI type IDs with a 3-bit arity indicator. (PR #117121)
maurer wrote: I'm not sure I buy the argument that cross-arity functions are significantly more exploitable than same-arity mismatches. Restating your argument to make sure I've understood it correctly: When an attacker swaps in a function pointer of higher arity, a dead variable (or even a live one expected to be restored afterwards), may be used improperly by the function. The attacker is more likely to be able to control this than other parameters. (This last part is the piece that I don't see the justification for.) However, this argument applies to *every* argument consumed by a function - each is as likely to be attacker controlled as the contents of a dead register, and if the function pointer thinks it's getting a value of a different type (e.g. believes it's getting a pointer, gets an attacker controlled flags value). This seems to be an argument that higher arity functions are more likely to be exploitable because it's more likely that the attacker controls at least one of their argument. Additionally, the increased likelihood of exploitability of a mismatch needs to be quite high to justify what you're describing. Let `p` be the probability that a type mismatch with matching arity is exploitable, and `q` be the probability that a type mismatch with an arity change is exploitable. Let X be the base collision rate. Then the old scheme has an exploitability rate of: `0.01375771 * (p * 0.25 + q * 0.75)` and the new one has a rate of `0.02660650 * p`. Setting equal and reducing to find the breakeven point, ``` 0.022575 p = 0.010275 q 2.197 p = q ``` So if you want to claim this is to increase the strength of the scheme, you need to argue that an attacker is more than twice as likely to be able to exploit an arity mismatched function as a type mismatched one, which seems like a bit of a stretch - attackers often have direct control of intentionally passed arguments... https://github.com/llvm/llvm-project/pull/117121 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86] Enhance kCFI type IDs with a 3-bit arity indicator. (PR #117121)
maurer wrote: I think Scott's point may be the relevant one - this may be similar strength or slightly weaker, but having an indicator stating which registers are live is potentially needed to allow FineIBT to poison them during speculative execution (I would be interested if you have a doc explaining your plan there, but just from a personal perspective, not from this PR). Given how much padding is used in the X86 kernel around function headers, have you considered just stealing another byte to encode the data for your arity stuff, and considering it to be a separate mitigation from KCFI tags? The rest of CFI is arch-independent, but in order to know which *registers* are in use, you need arch dependent information, because you care about the calling convention, packing rules, etc. This is part of why Ramon thought your design choices were odd above - this isn't really a CFI enhancement or modification, this is another piece of information you need for speculation defenses that occur at a different abstraction level. https://github.com/llvm/llvm-project/pull/117121 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [X86] Extend kCFI with a 3-bit arity indicator (PR #121070)
maurer wrote: > > If hashing is changed, consider replacing xxhash64 with xxh3+_64bits > > @MaskRay This PR does not change the hashing scheme at all. I think their point is that even if you are not changing the hash scheme, you are proposing breaking compatibility of the identifier with existing code. Since we don't want to do this many times, if we are breaking compatibility with existing code, they would like to batch it with another breaking update so that it doesn't need to be done again. (This isn't me reviewing this PR, just trying to clear up some confusion.) https://github.com/llvm/llvm-project/pull/121070 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
