CFI: Fix many vtable-related problems #121962

maurer · 2024-03-04T05:37:18Z

This PR makes it possible to use trait objects with CFI enabled, as checked by the various tests added.

Non-landable tests I also did:

Before I rebased (and was on a version of LLVM that matched my system LLVM), I was able to run ./x.py build --stage 2 with CFI hacked on for all build targets after #[no_sanitize]ing parts of the proc-macro crate. This gives me some confidence in the completeness of these CFI modifications.
I am able to build and run a KCFI'd Rust-language driver in the Linux kernel, and use it to boot Android:

mmaurer@anyblade:~/android/platform$ adb shell cat /proc/config.gz | gunzip | grep 'BINDER\|CFI'
CONFIG_FUNCTION_PADDING_CFI=11
CONFIG_ARCH_SUPPORTS_CFI_CLANG=y
CONFIG_ARCH_USES_CFI_TRAPS=y
CONFIG_CFI_CLANG=y
# CONFIG_CFI_PERMISSIVE is not set
# CONFIG_SND_SOC_FSL_MICFIL is not set
# CONFIG_ANDROID_BINDER_IPC is not set
CONFIG_ANDROID_BINDER_IPC_RUST=y
CONFIG_ANDROID_BINDERFS_RUST=y
CONFIG_ANDROID_BINDER_DEVICES_RUST="binder,hwbinder,vndbinder"
mmaurer@anyblade:~/android/platform$

The general approach taken here is is to make Virtual instances and the instances put into vtables match their abstracted type, without changing the type of the underlying function instances.

cc @rcvalle
cc @tmandry
cc @workingjubilee

I'm not sure who an appropriate reviewer is for this, so I'll let rustbot take a guess and post this PR in the zulip thread.

rustbot · 2024-03-04T05:37:26Z

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

rustbot · 2024-03-04T05:37:28Z

Some changes occurred to MIR optimizations

cc @rust-lang/wg-mir-opt

Some changes occurred to the CTFE / Miri engine

cc @rust-lang/miri

petrochenkov · 2024-03-04T13:39:43Z

I'm not sure who an appropriate reviewer is for this, so I'll let rustbot take a guess and post this PR in the zulip thread.

Looks like rustbot chose me due to the rustc_target changes, but I'm not an appropriate reviewer here.
r? compiler

workingjubilee · 2024-03-04T16:02:34Z

@maurer Is there no chance at factoring this down the line of the mir -> cg_ssa changeover?

Why does this require changes in rustc_const_eval?

RalfJung · 2024-03-04T16:07:33Z

Why does this require changes in rustc_const_eval?

It adds a new kind of MIR shim. But the question then is, why is that needed?
@maurer please provide a description of your design and the reasons for it. Do not expect the reader to know how CFI works, so you may want to start with a problem statement that explains the relevant parts of CFI. This sounds like it adds an extra function indirection to virtual calls, did you benchmark that?

Maybe someone from @rust-lang/wg-mir-opt can help evaluate whether having this new shim is a good idea. (I don't think we have a ping group for MIR in general, just for MIR opts?)

RalfJung · 2024-03-04T16:10:39Z

compiler/rustc_middle/src/ty/instance.rs

+    /// For `target_instance` which are generatable shims, this is done via inserting a cast at the
+    /// beginning of the shim generated by that instance.
+    /// For `Item`s, we instead build a direct call shim with that cast inserted.
+    CfiShim { target_instance: &'tcx InstanceDef<'tcx>, invoke_ty: Ty<'tcx> },


Does this mean we can remove the code in the interpreter and codegen that currently does receiver type adjustments? In the interpreter, that's here.

If that code is still needed then I don't understand what this achieves.

That code is still needed at the moment because I am conditionally generating shims - they are only generated when CFI is enabled, since they will result in worse code generation (If it's an item, there's a cast trampoline, and if it's not an item, there's a variant of the shim produced). Even if all calls were inlined, it'd still produce extra bulk as each vtable the type appears in gets its own shim.

I could try testing disabling that code when .cfi_shims() is true if you think that would be interesting.

I could try testing disabling that code when .cfi_shims() is true if you think that would be interesting.

No, I was just going off of the PR description which lacks key information it seems. I'll wait for more details to be added. :)

(Incorrect statement, ignore)

maurer · 2024-03-04T16:25:13Z

@maurer Is there no chance at factoring this down the line of the mir -> cg_ssa changeover?

I'm unsure how that would work - if I have

impl MyTrait for Foo {
   fn bar(&self) { // ... }
}
trait OtherTrait : MyTrait {}
impl OtherTrait for Foo {}
fn main() {
  Foo.bar(); // Base instance, no shims
  let x = &Foo as &dyn MyTrait;
  x.bar(); // Shimmed instance, new invocation type `dyn MyTrait`
  let y = &Foo as &dyn OtherTrait;
  y.bar(); // Shimmed instance, new invocation type `dyn OtherTrait`
}

how could I introduce this cutover at the stage of MIR->SSA? I was under the impression that by the time that was occurring, we were supposed to be doing a 1:1 transformation of Instance -> codegen structure, not Instance -> several. VTable generation has also already occurred above that abstraction level, so I don't know how I would get the vtables to point to each of the three specialized allocations.

RalfJung · 2024-03-04T16:30:36Z

So this makes the two vtables point to two different functions for that example? That sounds pretty bad for code size, doesn't it? And for perf too -- either there's now extra calls and stack traffic, or it gets inlined and there's even more code bloat and the same code has to be optimized and passed to the backend multiple times.

I guess I'll wait for your design document. Currently that doesn't sound like somewhat we want to do for regular builds. Maybe this only happens for CFI builds, the PR description doesn't say.

workingjubilee · 2024-03-04T16:33:46Z

how could I introduce this cutover at the stage of MIR->SSA? I was under the impression that by the time that was occurring, we were supposed to be doing a 1:1 transformation of Instance -> codegen structure, not Instance -> several. VTable generation has also already occurred above that abstraction level, so I don't know how I would get the vtables to point to each of the three specialized allocations.

I only meant in terms of landing the changes that need to happen before cg_ssa in a separate PR, so that they can be reviewed separately. But if that isn't very realistic and not very useful in your opinion, then don't worry about it: we have bigger fish to fry. It just is common for knowledge of the compiler to be split on that bifurcating point, so this patch will likely require multiple reviewers.

maurer · 2024-03-04T16:38:17Z

It adds a new kind of MIR shim. But the question then is, why is that needed? @maurer please provide a description of your design and the reasons for it. Do not expect the reader to know how CFI works, so you may want to start with a problem statement that explains the relevant parts of CFI. This sounds like it adds an extra function indirection to virtual calls, did you benchmark that?

I'll provide a larger writeup in a bit, but here's the summary:

CFI works by checking the type of the function pointer it expects to call before it calls it, and making sure it matches. These function pointer types are annotated to LLVM, and it provides the facility to test the type.
Rust vtables today intentionally have an implicit cast at the site of any indirect call. This is why you see the thin pointer logic you pointed to - you're implicitly casting a *dyn MyTrait to a &Foo while discarding the vtable so that the caller's expected ABI matches the callee's expected ABI, even if the signatures don't match.
This patch makes it so that rather than taking a &Foo, vtable entries will take a *dyn MyTrait. It does this by creating an additional shim for every entry, either grafted on for generated code or via a call trampoline otherwise, which has a MIR cast from the general type to the specific type.

With regards to performance, I have not benchmarked it, though I do expect it to hurt performance. Since most of these methods would previously crash when CFI was enabled, and the shims are only generated with CFI enabled, I didn't think it was critical.

If you'd like, I can benchmark enabling shims without enabling CFI, do you have a suggested workload?

maurer · 2024-03-04T16:42:35Z

how could I introduce this cutover at the stage of MIR->SSA? I was under the impression that by the time that was occurring, we were supposed to be doing a 1:1 transformation of Instance -> codegen structure, not Instance -> several. VTable generation has also already occurred above that abstraction level, so I don't know how I would get the vtables to point to each of the three specialized allocations.

I only meant in terms of landing the changes that need to happen before cg_ssa in a separate PR, so that they can be reviewed separately. But if that isn't very realistic and not very useful in your opinion, then don't worry about it: we have bigger fish to fry. It just is common for knowledge of the compiler to be split on that bifurcating point, so this patch will likely require multiple reviewers.

Sorry, I misunderstood and thought you wanted me to push all the logic into cg_ssa or beyond. I could probably cut the PR at CFI: Apply CFI shims to drops, and land each patch beyond in individual PRs, it's only the initial block that kind of all needs to go in at once to do anything useful/meaningful. Would that be helpful? Last I checked there wasn't a good way to stack PRs on github, but I could file separate ones at each reviewable point if that would help people work through things.

RalfJung · 2024-03-04T16:48:45Z

I'll provide a larger writeup in a bit, but here's the summary:

Seems unfortunate that CFI doesn't have a better way to encode the correct semantics here and requires some contortions in the generated code. (At least, it doesn't seem obvious that having such trampoline calls actually increases security compared to CFI that would be able to detect the correct calls directly.) But well, I guess it's what it is...

If you'd like, I can benchmark enabling shims without enabling CFI, do you have a suggested workload?

I didn't realize this would be enabled only for CFI, that makes perf less of a concern then.

It introduces new risks though, if these codepaths are not executed in regular builds at all. I see you added some tests :) . I would encourage you to not unnecessarily split up tests into multiple files; the overhead of separate compiler invocations does add up across a large test suite like ours. (But if they need different flags then they do need different files.)

workingjubilee · 2024-03-04T16:49:48Z

CFI works by checking the type of the function pointer it expects to call before it calls it, and making sure it matches. These function pointer types are annotated to LLVM, and it provides the facility to test the type.

Hmm. Why exactly is the KCFI model incompatible with this particular style of pointer cast? My understanding is that usually, other kinds of pointer-casting can be accepted, without necessarily erasing all type-checking entropy, and that in many cases, C code that needs to do similar will be performing such an erasure manually?

maurer · 2024-03-04T16:49:52Z

So this makes the two vtables point to two different functions for that example? That sounds pretty bad for code size, doesn't it? And for perf too -- either there's now extra calls and stack traffic, or it gets inlined and there's even more code bloat and the same code has to be optimized and passed to the backend multiple times.

Yes, they point to two different functions. This is one way to make it work for regular CFI, and mandatory for KCFI where each function is only allowed a single type. We might be able to get rid of shims some day for regular CFI by leveraging the ability to associate multiple types with a function, but KCFI will always need these shims. KCFI might be able to use only one shim if we make trait upcasting a load-bearing part of virtual calls (since then you'd only ever shim the function at the trait it was implemented for, not all child traits), but that would be future work.

I guess I'll wait for your design document. Currently that doesn't sound like somewhat we want to do for regular builds. Maybe this only happens for CFI builds, the PR description doesn't say.
I don't think we want to generate these cast shims for regular builds. The only change that I made that currently affects regular builds is the last one on the stack which strips auto traits off the receivers of virtual calls.

maurer · 2024-03-04T16:56:28Z

CFI works by checking the type of the function pointer it expects to call before it calls it, and making sure it matches. These function pointer types are annotated to LLVM, and it provides the facility to test the type.

Hmm. Why exactly is the KCFI model incompatible with this particular style of pointer cast? My understanding is that usually, other kinds of pointer-casting can be accepted, without necessarily erasing all type-checking entropy, and that in many cases, C code that needs to do similar will be performing such an erasure manually?

KCFI only supports exactly one principal type for a function. This means that no casting at the indirect call site is allowed. The actual implementation mechanism is that the compiler takes the principal type, encodes it to a string, hashes it, and then puts the hashed type in a prelude bundle before the function. When a caller wants to call a function, the compiler takes the type, encodes it to a string, hashes it, and then inserts a check that the function has that hashed type right before it.

In the one place where KCFI is heavily used, Linux, C code does indeed perform this erasure manually. Since they don't have objects, their equivalent of vtables are usually structs of function pointers. Their usual pattern is that your implementation takes a pointer to a struct of the abstract type, which is sometimes data-bearing. If it is not data-bearing, it will usually just be a void* you do an explicit cast to your real object on. If it is, then usually it is present as a field inside your object, and the container_of macro is used to do pointer math and a cast on it, assuming that it is a pointer into your object.

It is possible to avoid most of these shims (drop shims can't be entirely avoided) for LLVM-CFI with appropriate multiple attachments.

workingjubilee · 2024-03-04T17:03:17Z

Sorry, I misunderstood and thought you wanted me to push all the logic into cg_ssa or beyond. I could probably cut the PR at CFI: Apply CFI shims to drops, and land each patch beyond in individual PRs, it's only the initial block that kind of all needs to go in at once to do anything useful/meaningful. Would that be helpful? Last I checked there wasn't a good way to stack PRs on github, but I could file separate ones at each reviewable point if that would help people work through things.

Possibly. You're right about stacked diffs not being a thing on GH. I'll wait for someone from mir-opt to make a peep about whether they think also-seeing the LLVM and symbol-mangling logic is helpful or just visual noise.

Although, now that I look more closely at it, could you possibly pluck these out? They don't seem like they depend on the rest of the changes at all, though I could be mistaken. Those could be landed in their own PR, if so. ( They do touch the same file, so they should probably land together for obvious merge conflict reasons, but they both seem to be "pure fixup" with none of the design considerations involved, so that should go quickly. )

rcvalle · 2024-03-04T17:11:47Z

Is it possible to break these changes into a set of smaller self-contained pairs of issues and fixes (like I've been fixing the CFI known issues)? The reason I ask is that not only it will make it easier for me to understand the problems and what is being proposed, but also because I think some of the issues here are fixed already or are in the process of being fixed for CFI using type metadata IDs/encoding only.

I understand that CFI and KCFI perform type tests differently and are implemented as different LLVM passes for KCFI to not require LTO, and because of that KCFI has the limitation that a function may have one type id assigned only, and thus why shims may be necessary. However, CFI support for Rust was designed to be as unobtrusive as possible by just generating type tests and using type metadata IDs/encoding for its implementation, and I'd like to keep it that way (at least for CFI, if not possible for KCFI).

Introducing shims/trampolines may result in CFI bypasses and affect CFI performance and I'd like to avoid that. Therefore, would it be also possible to (in addition to breaking these changes into a set of smaller self-contained pairs of issues and fixes), also making them for KCFI only (and not for CFI)?

I'm not sure (and others let me know what they think), but I think that--even for KCFI only--this may require at least a design doc and/or an MCP explaining what is being proposed in detail--it'd for sure help me better understand and review it.

Have you investigated adding support for functions to have multiple type ids assigned to the LLVM passes for KCFI instead, and weighted the costs x benefits of doing that instead of implementing a workaround for it in the Rust compiler? I think it's important it's also documented there.

workingjubilee · 2024-03-04T17:31:16Z

Despite my normally laissez-faire attitude about encoding certain type relationships, I strongly agree that we wish to handle generating code that produces more gadgets that are essentially explicitly designed to allow breaking down restrictions on type encodings with extra care.

bjorn3 · 2024-03-04T17:31:40Z

In the case of

impl MyTrait for Foo {
   fn bar(&self) { // ... }
}
trait OtherTrait : MyTrait {}
impl OtherTrait for Foo {}

can't we generate the <Foo as MyTrait>::bar instance with *const MyTrait as self type even without the shim? For a <Foo as OtherTrait>::bar call, we can still use *const MyTrait as self type, so that doesn't need a shim either. It may require a shim for <Foo as MyTrait>::bar as fn(&Foo), but we already have InstanceDef::ReifyShim for that (it is technically for <dyn MyTrait as MyTrait>::bar, but I don't see why it wouldn't work for the concrete case either). Casting trait methods to a function pointer is less common than calling them through a trait object and less common than casting a free method to a function pointer, so may be fine to emit that unconditionally.

maurer · 2024-03-04T18:05:22Z

In the case of
impl MyTrait for Foo {
   fn bar(&self) { // ... }
}
trait OtherTrait : MyTrait {}
impl OtherTrait for Foo {}
can't we generate the <Foo as MyTrait>::bar instance with *const MyTrait as self type even without the shim? For a <Foo as OtherTrait>::bar call, we can still use *const MyTrait as self type, so that doesn't need a shim either.

This isn't true, see the address-taken test I added. I can elaborate more when I'm not stuck on my phone if that doesn't explain things.

bjorn3 · 2024-03-04T18:07:23Z

This isn't true, see the address-taken test I added.

That is a cast to a function pointer, right? See the rest of my comment:

It may require a shim for <Foo as MyTrait>::bar as fn(&Foo), but we already have InstanceDef::ReifyShim for that (it is technically for <dyn MyTrait as MyTrait>::bar, but I don't see why it wouldn't work for the concrete case either).

rcvalle · 2024-03-14T07:06:20Z

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 types¹ 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?

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. ↩

workingjubilee · 2024-03-14T18:48:17Z

@rcvalle Let's separate out discussion of possible changes to KCFI itself from discussing this PR. If Rust decides to reject this PR because we decide this PR is too-baroque an implementation in Rust, for KCFI or otherwise, that would happen independently of whether KCFI would be changed in the future. I am not against discussing it (obviously), but I would rather do so in the Zulip thread I started for such. I will treat further discussion in this PR of that subject as off-topic and mark it appropriately.

rcvalle · 2024-03-20T03:17:53Z

For anyone that commented on this PR and also anyone new to this PR, see my comments in #116404 (comment).

…ilee CFI: Skip non-passed arguments Rust will occasionally rely on fn((), X) -> Y being compatible with fn(X) -> Y, since () is a non-passed argument. Relax CFI by choosing not to encode non-passed arguments. This PR was split off from rust-lang#121962 as part of fixing the larger vtable compatibility issues. r? `@workingjubilee`

…ilee CFI: Skip non-passed arguments Rust will occasionally rely on fn((), X) -> Y being compatible with fn(X) -> Y, since () is a non-passed argument. Relax CFI by choosing not to encode non-passed arguments. This PR was split off from rust-lang#121962 as part of fixing the larger vtable compatibility issues. r? ``@workingjubilee``

CFI: Skip non-passed arguments Rust will occasionally rely on fn((), X) -> Y being compatible with fn(X) -> Y, since () is a non-passed argument. Relax CFI by choosing not to encode non-passed arguments. This PR was split off from rust-lang#121962 as part of fixing the larger vtable compatibility issues. r? `@workingjubilee`

…, r=workingjubilee CFI: Support self_cell-like recursion Current `transform_ty` attempts to avoid cycles when normalizing `#[repr(transparent)]` types to their interior, but runs afoul of this pattern used in `self_cell`: ``` struct X<T> { x: u8, p: PhantomData<T>, } #[repr(transparent)] struct Y(X<Y>); ``` When attempting to normalize Y, it will still cycle indefinitely. By using a types-visited list, this will instead get expanded exactly one layer deep to X<Y>, and then stop, not attempting to normalize `Y` any further. This PR was split off from rust-lang#121962 as part of fixing the larger vtable compatibility issues. r? `@workingjubilee`

…, r=workingjubilee CFI: Support self_cell-like recursion Current `transform_ty` attempts to avoid cycles when normalizing `#[repr(transparent)]` types to their interior, but runs afoul of this pattern used in `self_cell`: ``` struct X<T> { x: u8, p: PhantomData<T>, } #[repr(transparent)] struct Y(X<Y>); ``` When attempting to normalize Y, it will still cycle indefinitely. By using a types-visited list, this will instead get expanded exactly one layer deep to X<Y>, and then stop, not attempting to normalize `Y` any further. This PR was split off from rust-lang#121962 as part of fixing the larger vtable compatibility issues. r? ``@workingjubilee``

…, r=workingjubilee CFI: Support self_cell-like recursion Current `transform_ty` attempts to avoid cycles when normalizing `#[repr(transparent)]` types to their interior, but runs afoul of this pattern used in `self_cell`: ``` struct X<T> { x: u8, p: PhantomData<T>, } #[repr(transparent)] struct Y(X<Y>); ``` When attempting to normalize Y, it will still cycle indefinitely. By using a types-visited list, this will instead get expanded exactly one layer deep to X<Y>, and then stop, not attempting to normalize `Y` any further. This PR was split off from rust-lang#121962 as part of fixing the larger vtable compatibility issues. r? ```@workingjubilee```

…, r=workingjubilee CFI: Support self_cell-like recursion Current `transform_ty` attempts to avoid cycles when normalizing `#[repr(transparent)]` types to their interior, but runs afoul of this pattern used in `self_cell`: ``` struct X<T> { x: u8, p: PhantomData<T>, } #[repr(transparent)] struct Y(X<Y>); ``` When attempting to normalize Y, it will still cycle indefinitely. By using a types-visited list, this will instead get expanded exactly one layer deep to X<Y>, and then stop, not attempting to normalize `Y` any further. This PR was split off from rust-lang#121962 as part of fixing the larger vtable compatibility issues. r? ````@workingjubilee````

…, r=workingjubilee CFI: Support self_cell-like recursion Current `transform_ty` attempts to avoid cycles when normalizing `#[repr(transparent)]` types to their interior, but runs afoul of this pattern used in `self_cell`: ``` struct X<T> { x: u8, p: PhantomData<T>, } #[repr(transparent)] struct Y(X<Y>); ``` When attempting to normalize Y, it will still cycle indefinitely. By using a types-visited list, this will instead get expanded exactly one layer deep to X<Y>, and then stop, not attempting to normalize `Y` any further. This PR was split off from rust-lang#121962 as part of fixing the larger vtable compatibility issues. r? `````@workingjubilee`````

…, r=workingjubilee CFI: Support self_cell-like recursion Current `transform_ty` attempts to avoid cycles when normalizing `#[repr(transparent)]` types to their interior, but runs afoul of this pattern used in `self_cell`: ``` struct X<T> { x: u8, p: PhantomData<T>, } #[repr(transparent)] struct Y(X<Y>); ``` When attempting to normalize Y, it will still cycle indefinitely. By using a types-visited list, this will instead get expanded exactly one layer deep to X<Y>, and then stop, not attempting to normalize `Y` any further. This PR was split off from rust-lang#121962 as part of fixing the larger vtable compatibility issues. r? ``````@workingjubilee``````

Rollup merge of rust-lang#122875 - maurer:cfi-transparent-termination, r=workingjubilee CFI: Support self_cell-like recursion Current `transform_ty` attempts to avoid cycles when normalizing `#[repr(transparent)]` types to their interior, but runs afoul of this pattern used in `self_cell`: ``` struct X<T> { x: u8, p: PhantomData<T>, } #[repr(transparent)] struct Y(X<Y>); ``` When attempting to normalize Y, it will still cycle indefinitely. By using a types-visited list, this will instead get expanded exactly one layer deep to X<Y>, and then stop, not attempting to normalize `Y` any further. This PR was split off from rust-lang#121962 as part of fixing the larger vtable compatibility issues. r? ``````@workingjubilee``````

…, r=workingjubilee CFI: Support self_cell-like recursion Current `transform_ty` attempts to avoid cycles when normalizing `#[repr(transparent)]` types to their interior, but runs afoul of this pattern used in `self_cell`: ``` struct X<T> { x: u8, p: PhantomData<T>, } #[repr(transparent)] struct Y(X<Y>); ``` When attempting to normalize Y, it will still cycle indefinitely. By using a types-visited list, this will instead get expanded exactly one layer deep to X<Y>, and then stop, not attempting to normalize `Y` any further. This PR was split off from rust-lang#121962 as part of fixing the larger vtable compatibility issues. r? ``````@workingjubilee``````

workingjubilee · 2024-03-25T19:51:41Z

Well, this obviously would need a rebase, but also it's not clear that we're going to take exactly this route since #123012 so!

@rustbot author

maurer · 2024-03-25T19:57:17Z

I'm closing this PR as I'm taking an approach much closer to the type removal one. The branch will still be there for reference until we finish fixing the overall vtable issues in case we need to crib solutions from it.

Since we're now using an approach which is a variant of rust-lang#123082 (that transforms closures into dynamic Fn traits but isolating it to the Fn call methods only) instead of rust-lang#121962 or rust-lang#122573, skipping non-passed arguments shouldn't be necessary KCFI anymore and we can claim back the reduced granularity. This reverts commit f2f0d25.

Since we're now using an approach which is a variant of rust-lang#123082 (that transforms closures into dynamic Fn traits but isolating it to the Fn call methods only) instead of rust-lang#121962 or rust-lang#122573, skipping non-passed arguments shouldn't be necessary for KCFI anymore and we can claim back the reduced granularity. This reverts commit f2f0d25.

rustbot assigned petrochenkov Mar 4, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 4, 2024

This comment has been minimized.

Sign in to view

maurer force-pushed the staging branch from c7df23d to 73b583c Compare March 4, 2024 05:57

ojeda mentioned this pull request Mar 4, 2024

rustc wanted features & bugfixes Rust-for-Linux/linux#355

Open

47 tasks

rustbot assigned compiler-errors and unassigned petrochenkov Mar 4, 2024

RalfJung reviewed Mar 4, 2024

View reviewed changes

rcvalle added the PG-exploit-mitigations Project group: Exploit mitigations label Mar 4, 2024

maurer mentioned this pull request Mar 13, 2024

CFI: Skip non-passed arguments #122456

Merged

maurer mentioned this pull request Mar 15, 2024

CFI: Repair vtables without altering types #122573

Closed

rcvalle mentioned this pull request Mar 20, 2024

CFI: Fix fn items, closures, and Fn trait objects, and more #116404

Closed

maurer mentioned this pull request Mar 22, 2024

CFI: Support self_cell-like recursion #122875

Merged

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 25, 2024

maurer closed this Mar 25, 2024

rcvalle mentioned this pull request Mar 27, 2024

CFI: Abstract Closures and Coroutines #123106

Merged

rcvalle mentioned this pull request Mar 29, 2024

Revert "CFI: Skip non-passed arguments" #123205

Draft

rcvalle mentioned this pull request Apr 4, 2024

Tracking Issue for Kernel Control Flow Integrity (KCFI) Support for Rust #123479

Open

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

CFI: Fix many vtable-related problems #121962

CFI: Fix many vtable-related problems #121962

maurer commented Mar 4, 2024

rustbot commented Mar 4, 2024

rustbot commented Mar 4, 2024

This comment has been minimized.

petrochenkov commented Mar 4, 2024

workingjubilee commented Mar 4, 2024 •

edited

Loading

RalfJung commented Mar 4, 2024 •

edited

Loading

RalfJung Mar 4, 2024 •

edited

Loading

maurer Mar 4, 2024

RalfJung Mar 4, 2024

maurer Mar 4, 2024 •

edited

Loading

maurer commented Mar 4, 2024

RalfJung commented Mar 4, 2024 •

edited

Loading

workingjubilee commented Mar 4, 2024

maurer commented Mar 4, 2024

maurer commented Mar 4, 2024

RalfJung commented Mar 4, 2024 •

edited

Loading

workingjubilee commented Mar 4, 2024

maurer commented Mar 4, 2024

maurer commented Mar 4, 2024 •

edited

Loading

workingjubilee commented Mar 4, 2024 •

edited

Loading

rcvalle commented Mar 4, 2024

workingjubilee commented Mar 4, 2024

bjorn3 commented Mar 4, 2024

maurer commented Mar 4, 2024

bjorn3 commented Mar 4, 2024

rcvalle commented Mar 14, 2024 •

edited

Loading

workingjubilee commented Mar 14, 2024 •

edited

Loading

rcvalle commented Mar 20, 2024

workingjubilee commented Mar 25, 2024

maurer commented Mar 25, 2024

CFI: Fix many vtable-related problems #121962

CFI: Fix many vtable-related problems #121962

Conversation

maurer commented Mar 4, 2024

rustbot commented Mar 4, 2024

rustbot commented Mar 4, 2024

This comment has been minimized.

petrochenkov commented Mar 4, 2024

workingjubilee commented Mar 4, 2024 • edited Loading

RalfJung commented Mar 4, 2024 • edited Loading

RalfJung Mar 4, 2024 • edited Loading

Choose a reason for hiding this comment

maurer Mar 4, 2024

Choose a reason for hiding this comment

RalfJung Mar 4, 2024

Choose a reason for hiding this comment

maurer Mar 4, 2024 • edited Loading

Choose a reason for hiding this comment

maurer commented Mar 4, 2024

RalfJung commented Mar 4, 2024 • edited Loading

workingjubilee commented Mar 4, 2024

maurer commented Mar 4, 2024

maurer commented Mar 4, 2024

RalfJung commented Mar 4, 2024 • edited Loading

workingjubilee commented Mar 4, 2024

maurer commented Mar 4, 2024

maurer commented Mar 4, 2024 • edited Loading

workingjubilee commented Mar 4, 2024 • edited Loading

rcvalle commented Mar 4, 2024

workingjubilee commented Mar 4, 2024

bjorn3 commented Mar 4, 2024

maurer commented Mar 4, 2024

bjorn3 commented Mar 4, 2024

rcvalle commented Mar 14, 2024 • edited Loading

Footnotes

workingjubilee commented Mar 14, 2024 • edited Loading

rcvalle commented Mar 20, 2024

workingjubilee commented Mar 25, 2024

maurer commented Mar 25, 2024

workingjubilee commented Mar 4, 2024 •

edited

Loading

RalfJung commented Mar 4, 2024 •

edited

Loading

RalfJung Mar 4, 2024 •

edited

Loading

maurer Mar 4, 2024 •

edited

Loading

RalfJung commented Mar 4, 2024 •

edited

Loading

RalfJung commented Mar 4, 2024 •

edited

Loading

maurer commented Mar 4, 2024 •

edited

Loading

workingjubilee commented Mar 4, 2024 •

edited

Loading

rcvalle commented Mar 14, 2024 •

edited

Loading

workingjubilee commented Mar 14, 2024 •

edited

Loading