Can CFI be made compatible with type erasure schemes?

[RalfJung]

There is a kind of common type erasure scheme used in Rust that goes something like this:

/// Invariant: there exists some type `T` such that `data` is actually a `&'a T` and `op`
/// is actually a `fn(&T)`.
struct ErasedTypeAndOp<'a> {
  data: *const (),
  op: fn(*const ()),
  _phantom: PhantomData<&'a ()>,
}

impl<'a> ErasedTypeAndOp<'a> {
  pub fn new<T>(data: &'a T, op: fn(&'a T)) {
    Self {
      data: data as *const T,
      op: unsafe { std::mem::transmute(op) },
      _phantom: PhantomData,
    }
  }

  pub fn call_op(&self) {
    (self.op)(self.data)
  }
}

This is used, in particular, in the standard library for type-erased fmt arguments.

Unfortunately, CFI is not happy with this since the function pointer op was transmuted, and is not invoked at its original type. Our documented ABI compatibility rules are fine with this (and Miri also won't complain), but CFI is actually more restrictive than those rules and (at least in some configurations) rejects this call since caller and callee do not agree on the type of the argument.

This is not good: ideally we could just tell people they can turn on CFI in any Rust program and expect it to work. In other words, ideally CFI would only reject programs that we consider buggy (in an official Rust lang/opsem sense), since they have either UB or erroneous behavior.

I am not sure what is the best way to fix this. I also know very little about what CFI can and cannot do (and I understand there's actually a bunch of CFI implementations that differ in their capabilities). My completely naive first idea would be to say that we have a new magic primitive type Erased and then declare ErasedTypeAndOp as follows:

struct ErasedTypeAndOp<'a> {
  data: *const Erased,
  op: fn(*const Erased),
  _phantom: PhantomData<&'a ()>,
}

Then we say that if caller and callee use different pointer types for their arguments, that is erroneous behavior unless one of them uses the special Erased pointee type, in which case the call is permitted. (Or maybe only the call site is allowed to use Erased like that?)

Is that something CFI can do -- basically have pointee type checking "turned off" if the call site uses *const Erased? If yes, would this be an acceptable trade-off between still rejecting accidental type mismatches (and catching as many attacks as possible) while accepting legitimate type erasure patterns?

Cc @rust-lang/opsem @maurer @rcvalle

[VorpalBlade]

Then we say that if caller and callee use different pointer types for their arguments, that is erroneous behavior unless one of them uses the special Erased pointee type, in which case the call is permitted. (Or maybe only the call site is allowed to use Erased like that?)

Wouldn't this be a breaking change? And it isn't a soundness or security fix, so I don't see h it being allowed by that rule.

[RalfJung]

Yeah we'd have to evaluate whether making the ecosystem CFI-compatible is possible without undue breakage. OTOH if we don't do this CFI will always be a second-class citizen as a sanitizer, since it will keep rejecting completely valid code.

[VorpalBlade]

The other option is to make CFI Rust compatible. Why is that not being considered as a solution? I see no reason why Rust and C/C++ should use exactly the same variant of CFI, since they are different languages with different UB/EB.

For the same reason I would expect yet other languages to need other variations of CFI as well (though I don't know them in enough detail to come up with any specific examples).

[RalfJung]

The other option is to make CFI Rust compatible. Why is that not being considered as a solution?

It is being considered. I am talking to various stakeholders and the CFI folks are telling me that accepting arbitrary pointee type mismatches makes CFI "essentially worthless". I'll let them explain why that is the case.

[ChayimFriedman2]

It is possible to fix the example without fn transmutes (although I don't know if this will be good enough for CFI) by introducing a shim:

use std::marker::PhantomData;

/// Invariant: there exists some type `T` such that `data` is actually a `&'a T` and `op`
/// is actually a `fn(&T)`.
struct ErasedTypeAndOp<'a> {
    data: *const (),
    op: fn(*const ()),
    _phantom: PhantomData<&'a ()>,
}

impl<'a> ErasedTypeAndOp<'a> {
    pub fn new<F: Fn(&'a T), T>(data: &'a T, _op: F) -> Self {
        const { assert!(std::mem::size_of::<F>() == 0) };
        fn op_shim<'a, T: 'a, F: Fn(&'a T)>(data: *const ()) {
            unsafe {
                let op = std::mem::zeroed::<F>();
                op(&*data.cast::<T>())
            }
        }
        Self {
            data: data as *const T as *const (),
            op: op_shim::<T, F>,
            _phantom: PhantomData,
        }
    }

    pub fn call_op(&self) {
        (self.op)(self.data)
    }
}

There are other variants of this, such as introducing a trait.

This isn't as flexible as transmuting the fn pointer, but I think it fits most of the cases. So if we can break compatibility (especially considering that the docs about ABI compatibility weren't stable for a long time, only since 1.76.0) we can tell people to use that approach.

It seems a somewhat-safe wrapper (providing unsafe fn(*const ())) for this pattern can be made into std, but maybe this will require variadics to be fully generic.

[RalfJung]

Ah right, if you take the function item as input you can do this without extra allocations. You could try this approach with std::fmt to see how it does in practice.

(especially considering that the docs about ABI compatibility weren't stable for a long time, only since 1.76.0)

We didn't document anything before, but this has been de-facto relied upon since Rust 1.0 by std itself and hence likely was picked up by other crates during the years.

Also, CFI is not a super widely used sanitizer technique (to my knowledge), so I am hesitant to force everyone to adopt such quirky patterns... it would be better if the sanitizer could be improved to incur less collateral damage on legit code.

[ChayimFriedman2]

We didn't document anything before, but this has been de-facto relied upon since Rust 1.0 by std itself and hence likely was picked up by other crates during the years.

Personally I have always used a shim until this was documented. std can use things that are not guaranteed sound. But I can understand others were relying on this.

What about declaring it is not UB, but recommending developers to switch to alternatives (perhaps adding something like Erased too)?

Also, is there any way we can automatically detect crates using such techniques and lint against them when trying to apply CFI?