CFI: core and std have explict CFI violations

[rcvalle]

Even though the user can now rebuild both core and std with CFI enabled (see #90546) using Cargo build-std feature (which is recommended), both have explicit CFI violations that prevent the compiled program from functioning with CFI enabled.

So far, I've identified three CFI violations:

1. std::sys::unix:thread_local_dtor::register_dtor weakly links __cxa_thread_atexit_impl and and the Rust compiler currently omits weakly function definitions and its metadata from LLVM IR.
2. core::fmt::rt::Argument transmuting formatter in new and indirectly branching to/calling it in fmt.
3. Rust's "try catch" construct (i.e., std::panicking::r#try) use of FnOnce explicitly violating CFI .
4. std::sys::unix::weak::syscall macro weakly links functions and the Rust compiler currently omits weakly function definitions and its metadata from LLVM IR.

I'm not sure if those are all CFI violations, but all core and std tests pass after disabling CFI in those locations with the no_sanitize attribute.

[workingjubilee]

I'm concerned about transmuting to a more refined type being rejected by LLVM's type-directed CFI. Specifically,

• from a function that accepts "thin-pointers to anything", i.e.
  • fn(*mut c_void) aka void(void*)
• to a function that accepts "thin-pointers to something specific" e.g.
  • fn(*mut u8) aka void(unsigned char*)

Intuitively, because void* must have an identical representation as char*, even though it is technically not a "compatible type" according to C (as void* is an incomplete type), refining the function's argument types should be permitted. And indeed casting a pointer from AnyOneThing* to void* and then back to AnyOneThing* should work.

I realize it may be unpleasant, but the reason why Rust does this?

https://www.youtube.com/watch?v=Y-Elr5K2Vuo

Rust's std puns between u8 and c_void like this because of LLVM insisting that a C void* is equivalent to an LLVMIR i8*. Thus Rust adopted this idiom of treating a byte pointer as interchangeable with a void pointer, because they effectively are, and because it was necessary to convince LLVM to recognize certain special functions.

[ChrisDenton]

Related:

• c_void implementation
• c_void docs

[rcvalle]

Thank you for all the information, @workingjubilee and @ChrisDenton! LLVM CFI does support those patterns (as C and C++ also use those), the casts/transmutes just have to be adjusted at the indirect call sites. You can see what I just did for (1) in the PR. This is not a characteristic of LLVM CFI itself, but many fine-grained type-based CFI implementations that perform function pointer type testing at the indirect call site.

It's actually common for large code bases to have those CFI violations when CFI support is initially implemented. For example, the Linux kernel had many. I'm actually surprised I've found only three so far for core and std, and this is actually very good.

[workingjubilee]

I am slightly less concerned with "large codebases" with small armies of experts to support them. Many of those experts are well-paid for their time, and looking after those who are not is a complex task.

Rather, I care about the ones maintained by some Jane Doe who makes a chat program that is a hodgepodge of miscellaneous C that is exposed to the network and eminently exploitable, and who decides to add Rust to handle the most brutally-exposed network-facing parser. I care about the volunteer maintainers of some obscure distro I've never heard of rebuilding it and trying to add compiler-driven hardening and running into complications. Not because any one such case is terribly important in security terms, but because there are millions of such cases in a very long tail.

So I am attentive to when CFI violations trigger on, frankly, trivialities, ones that require intimate knowledge of C99 or LLVM to explain. They should care more for intuitively obvious violations, not for pointers being coerced to/from the "anything-pointer". And to be more precise, c_void wasn't a stable std type until Rust 1.30, so there is an unfortunate amount of code that didn't specify using it. Including, er, some in std!

[rcvalle]

I hear your concerns and I wish fine-grained type-based CFI was an enable and forget mitigation as many others because I want as many users to use it as possible, but unfortunately it does require some code care and maintenance for use.

The violation in (1) is actually an uncommon case of a weakly-linked C or C++ callback/function pointer being called from Rust across the FFI boundary where the function signature it was being cast to at the call site was different from the function definition of the function linked.

Let me know if you have any suggestions of how this could be improved and I'll be happy to take a look at it.

[workingjubilee]

I mean, my question is, really, "What bug or exploit, exactly, are we preventing by prohibiting an argument of type uint8_t* from being passed to a parameter of type void*?"

[workingjubilee]

I should note, here: It feels intuitive and obvious to me what bugs we're preventing by prohibiting, say, i16 from being passed to a parameter expecting u32. Clear as day, no question, etc. And I feel that even if passing *mut i16 to a parameter expecting *mut u32 is more understandable, here again: obvious layout/alignment disagreement in the pointee, so all sorts of trouble if you actually act on it.

But in C, the main difference between char* and void* is... well, you can dereference one, and do pointer arithmetic on it without a C extension? But both are supposed to have the same repr, and thus be equally... well, meaningless.

[rcvalle]

Besides assisting with the bugs you mentioned, for example, not allowing an attacker to redirect the control flow to functions with void * and char * arguments interchangeably makes the difference in the attacker being able to exploit a vulnerability where they could just redirect the control flow to uninteresting(const void *) vs system(const char *). This is just an example, there could be more depending on the program and the vulnerability.

It's not that char * arguments couldn't be passed, they just have to be properly cast to demonstrate intent at the call site.

If the user wants to trade maintenance for a coarser-grained protection, they can use the -Zsanitizer-cfi-generalize-pointers that would allow for that.

[workingjubilee]

Except they cannot, can they? Your code comments in that PR allude to the problem: People building the code may have to decide how they bind against other code that may have already been built with various CFI options. This makes such choices virtually irrelevant on any distros which are not fully source-based, at least, for anyone but the head maintainers. And you, of course. Meanwhile, -Zsanitizer-cfi-generalize-pointers erases the typechecking capabilities of LLVM's Itanium C++ CFI with respect to pointers effectively entirely. Instead of allowing punning pointee types along a few specific legal transitions, essentially all pointers get punned without concern.

Relying on configuration flags during the build to ameliorate this problem causes de facto ecosystem and dialect splits which we have struggled mightily in the past to prevent. If I understand correctly, one has to link and build the entire program with this option to benefit from it, and it effectively prevents anyone from benefiting unless all their dependees also comply.

[workingjubilee]

Besides, it's also eminently the case that, having done a mem::transmute on the function pointer itself, that the author of that code did demonstrate intent. That is, it is not clear to me that this was not, in fact, a "proper cast".

[rcvalle]

Generalized pointer metadata is always included by Clang in C or C++ -compiled code regardless if the pointer generalization option is specified (as does the Rust compiler).

[workingjubilee]

??? Is LLVM going to treat the fact that some kinds of normalization must defer to other compilation units while others do not as a bug?

[RalfJung]

core::fmt::rt::Argument transmuting formatter in new and indirectly branching to/calling it in fmt.

I was unable to find this in the current code. Including a permalink to the relevant code would make it much easier for others to look into what is happening here. :)
Is this even still an issue?

Rust's "try catch" construct (i.e., std::panicking::r#try) use of FnOnce explicitly violating CFI .

Same here, what is this about? There is no fn try in the current standard library.

[RalfJung]

Same here, what is this about? There is no fn try in the current standard library.

Oh, yes there is. But I don't see any transmute of function pointers here so I don't see the CFI violation.

[Jules-Bertholet]

@rustbot label A-sanitizers