A new LLVM CFI implementation [LWN.net]

A new LLVM CFI implementation

Posted Jun 19, 2022 14:24 UTC (Sun) by atnot (subscriber, #124910) [Link] (5 responses)

Yes, this is to be expected as it is only designed to protect function pointers from being overwritten with completely arbitrary addresses. Only being able to call something that is actually intended to be a target of a function pointer is much less useful than, say, being able to call into the middle of a function that changes you uid. Since function pointers are used when you want to be able to change what function is called at runtime, checking the name would not make a lot of sense.

A new LLVM CFI implementation

Posted Jun 19, 2022 14:29 UTC (Sun) by willy (subscriber, #9762) [Link]

It'd be interesting to have a function be able to declare "I am an implementation of aops->dirty_folio", and abort if it's called through get_block_t. Should be possible with enough ELF magic and __attribute__((...))

A new LLVM CFI implementation

Posted Jun 19, 2022 14:29 UTC (Sun) by mss (subscriber, #138799) [Link] (3 responses)

> Since function pointers are used when you want to be able to change what function is called at runtime, checking the name would not make a lot of sense.

I assume having a hash table of allowed call targets could work here (it would be incompatible with out-of-tree kernel modules, however).

But this sounds a bit like the previous CFI design that this new implementation seeks to replace.

A new LLVM CFI implementation

Posted Jun 19, 2022 20:58 UTC (Sun) by willy (subscriber, #9762) [Link] (2 responses)

No, this is COME FROM, not GO TO.

A new LLVM CFI implementation

Posted Jun 19, 2022 21:32 UTC (Sun) by mss (subscriber, #138799) [Link] (1 responses)

> No, this is COME FROM, not GO TO.

I don't quite understand your analogy here, the article says:
> When an indirect call is made, extra code is emitted to fetch and check this hash value prior to emitting the call itself

So the checking happens prior to the actual call: in the caller, not in the callee.

A new LLVM CFI implementation

Posted Jun 19, 2022 21:41 UTC (Sun) by willy (subscriber, #9762) [Link]

I was perhaps overly eager to make an INTERCAL reference.

But I wasn't referring to the implementation; rather the concept is that the function declares who can call it. That's done by type here, but could also be done by saying "I am an implementation of get_block_t"

A new LLVM CFI implementation

Posted Jun 19, 2022 18:49 UTC (Sun) by iabervon (subscriber, #722) [Link]

The hash computation could presumably be changed pretty easily, since it only has to agree for all the files in a build of the kernel. Once the enforcement mechanism based on hashes is stable, the hash calculation should be easy enough to make more restrictive. Probably the ideal choice would be to hash the name of the typedef used to declare the function pointer, but then you need to change the code to annotate the functions with this typedef name. Getting the compiler to use this information for something useful is probably a necessary first step in getting people to make this kind of code change, so you want to start with using the information that is already available to do something that's good but could be improved.

Checking the name

Posted Jun 19, 2022 21:45 UTC (Sun) by corbet (editor, #1) [Link] (7 responses)

I don't quite understand what you are asking for here. The name of the called function isn't known at compile time, that's why it's an indirect call. So what name would you check against?

Checking the name

Posted Jun 19, 2022 21:58 UTC (Sun) by mss (subscriber, #138799) [Link] (5 responses)

> The name of the called function isn't known at compile time, that's why it's an indirect call

I think that the set of functions implementing particular callback in the kernel should be known at compile time, either via manual annotations (as willy has suggested above) or maybe even automatically by a sufficiently smart compiler.

This probably would be incompatible with out-of-tree kernel modules, however.

Checking the name

Posted Jun 19, 2022 22:40 UTC (Sun) by NYKevin (subscriber, #129325) [Link] (3 responses)

You can do that now. Just declare your functions as taking an extra (unused) argument of type struct { char[0]; char[0]; ...} with a different number of empty arrays for each possible callback. They have zero size and even at O0, no code should be emitted for them, but they still change the signature of the function and (should!) therefore change the C++ name mangling. The problem is, you also need to actually pass an argument, and unlike C++, C doesn't let you get away with just writing "{}" for that, so you probably need some sort of macro. It's a shame the standard doesn't let you write something like "void" for a zero-size argument.

Checking the name

Posted Jun 20, 2022 13:51 UTC (Mon) by khim (subscriber, #9252) [Link] (2 responses)

> It's a shame the standard doesn't let you write something like "void" for a zero-size argument.

Well… zero-sized arguments don't exist in standard C, they are GNU extension which means you can try to supply the patch which will support what you want to Clang and GCC.

Once you left the standard it's kind of hard to expect to see such non-standard constructs supported in said standard, don't you think?

Checking the name

Posted Jun 20, 2022 17:39 UTC (Mon) by nybble41 (subscriber, #55106) [Link] (1 responses)

I think the point was that the standard should have allowed zero-sized (void type) arguments, variables, and fields from the beginning, for symmetry with expressions and function return types, so it wouldn't require a GNU extension. Likewise for empty structures and unions & zero-length arrays (including array variables, not just fields).

A keyword for the void constructor might be nice, but "(void)0" would serve well enough. This could be made into a standard VOID macro, like NULL for "(void*)0".

Checking the name

Posted Jun 20, 2022 20:56 UTC (Mon) by wahern (subscriber, #37304) [Link]

The standard does have empty struct members using 0-length bit fields which have been in the standard from the beginning.[1] They're not quite empty as they're intended to serve as markers instructing the compiler to shift subsequent bit fields to a new word. 0-length arrays actually work similarly--fundamentally they're a mechanism for controlling alignment and can also introduce padding where there wouldn't have been any in their absence. (In practice, though, they can only introduce tail padding--at the tail end of a preceding bit field, if any, for the former, or tail end of the struct for the latter.) Empty bit fields seem a little more useful, though, as aside from being standard they can be (must be since C11) unnamed: `struct { int a:3; int:0; int b:1; };` is a valid definition. You can use them along with the GNU empty struct extension to get an empty type without any names: both `sizeof (struct { int:0; })` and `sizeof (struct { int:0; int:0; })` are valid and evaluate to 0 (tested clang 3.8 and GCC 9.4).

Also, at least based on a straight-forwarding reading of the standard, sequentially declared 0-length bit fields should collapse (i.e. not unspecified or undefined behavior), so that they introduce only one word of padding at most, if any; and this is indeed the behavior I see from clang and GCC. And while maybe more susceptible to disagreement, the language of the standard does seem to specify that a 0-length bit field not succeeding another bit field should not introduce any padding. I see the same behaviors for 0-length arrays, but the GCC documentation seemed much more ambiguous on both points.[2]

I am curious why I haven't seen void (the true "nothing") type semantics extended elsewhere in the grammar. Maybe 0-length bit and array fields were sufficient, if not ideal, for the most pressing scenarios. But perhaps the language (inclusive of extensions) is finally moving in a direction where the old hacks are insufficient, and void might see some more attention.

[1] See $ 3.5.2.1 at http://port70.net/~nsz/c/c89/c89-draft.txt

[2] Putting it all together after having double checked my assertions with the standard, it does seem that the only use for 0-length arrays has been almost entirely subsumed by flexible array members, except that the former make indexing notation easier (no offsetof verbiage). For anything else (mostly in relation to extensions, like 0-length structures), 0-length bit field notation seems sufficient. Maybe the situation is different with C++.

Checking the name

Posted Jun 19, 2022 23:29 UTC (Sun) by corbet (editor, #1) [Link]

That is essentially what the existing implementation does - that's what the jump tables contain. It requires link-time optimization to work, though, and I'm not sure what it buys over verification of the prototype.

Checking the name

Posted Jun 20, 2022 2:38 UTC (Mon) by comex (subscriber, #71521) [Link]

Maybe they are thinking of the function pointers in C++ vtables, where you know not just the expected signature, but the name of the virtual method that is being overridden. If you include that in the hash, you can prevent the attacker from replacing it with an unrelated function or method that happens to have the same signature.

Of course, that doesn't apply to C. But it may be possible to get a similar effect with manual annotations of some sort...