CFI: Fix many vtable-related problems by maurer · Pull Request #121962 · rust-lang/rust (original) (raw)

I'd also like to reiterate my request for, in addition to breaking these changes into a set of smaller self-contained pairs of issues and fixes, which seems you started to do, also making these changes for KCFI only (since they aren't needed for CFI, and I'm fixing those corner cases for CFI using a solution that doesn't require any shims).

In general, I'd like to see what the absolute minimum necessary use of shims even for KCFI looks like. For example, for function items, closures, and Fn trait objects to be used interchangeably, there is a solution I mentioned there I'm working on (see #116404) by signature_unclosure'ing/transforming those, exactly as the compiler does at the call sites and CFI does for other equivalent types1 that works even for KCFI, so the shims aren't necessary in this case even for KCFI. Therefore, if you also wait these changes to be merged, less shims are necessary.

Since KCFI landed pretty recently (for the Linux kernel standards), I'm still not convinced that it is too widely deployed yet that makes it very difficult to explore any changes to it. There are also the points I made about the possibility of inadvertently introducing KCFI bypasses, the possibility of general performance and cache coherence/locality impact of the shim/trampoline being placed farther away from the actual destination, that I think need to be looked at, and more generally what is the right thing to do longer term, both for the Rust project and the Linux kernel. After all, there is a reason the CFI project is taking long, and it's because we've been trying to do the right thing, and not rush things--if we wanted, we could have everything building and running already, but would that have been the right thing to do?

Footnotes

  1. Which follows the same design of the rest of the CFI implementation by transforming/coalescing types we want to be semantically equivalent into a single entity and passing them for encoding. ↩