Page MenuHomePhabricator

[analyzer] MIGChecker: A checker for Mach Interface Generator calling convention.
ClosedPublic

Authored by NoQ on Jan 31 2019, 5:38 PM.

Details

Summary

This is a fairly common source of use-after-free exploits in the XNU kernel, so it kinda makes sense to attempt to catch these statically. It makes even more sense because these errors occur on error paths that are hard to test dynamically.

MIG is an autogenerator for C headers (and, partially, implementations) of processes that want to communicate via the Mach IPC. It allows processes to pass relatively arbitrary C structures to each other in a relatively transparent manner. These headers are generated for both the IPC client to include in order to be able to call MIG routines and for the IPC server to implement MIG routines it provides as callback functions written in C. Sometimes resources passed through MIG need to be "deallocated" on the other side within the actual body of the routine. Such resources include virtual memory page addresses and Mach ports. These resources are called "out-of-line" ("OOL") resources and they can be thought of as pointers that make sense in kernel address space. When an OOL resource is passed from a client to the server, the following calling convention applies:

  • If a server routine returns error code KERNEL_SUCCESS or MIG_NO_REPLY as its return value, it shall deallocate all OOL data manually.
  • If a server routine returns any other error code (i.e., fails with an error), OOL arguments shall be deallocated by the client.

In particular, if a server routine deallocates an OOL object and then returns an error, this is always a use-after-free. This is exactly what the checker is supposed to warn about.


It's a trivial initial commit for now - the checker in this shape passes just one test and it's not listing any APIs apart from vm_deallocate() and detects MIG routine code heuristically rather than relying on a better source information such as source-level annotations.

The checker is path-sensitive and it detects the following sequence of events on the execution paths Static Analyzer explores: first an OOL argument is deallocated, then an error is returned to the caller. It is sufficient to track a single bit in the program state in order to issue the warning: "was at least one OOL argument deallocated?".

The checker believes that all MIG routines are analyzed as top-level functions rather than inlined from other functions. This is the right thing to do for now, but if we introduce annotations, we might as well allow them on arbitrary functions that suddenly want to follow the same calling convention (which is usually a bad idea because this calling convention is fairly hard to follow, which is exactly why we ended up in this situation, but if it's about documenting an existing behavior, than it kinda makes sense), and then we'll have to drop the "inTopFrame" check and probably introduce more complexity.

The checker lacks a bug visitor for now, hence alpha.

Diff Detail

Repository
rC Clang

Event Timeline

NoQ created this revision.Jan 31 2019, 5:38 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 31 2019, 5:38 PM
NoQ edited the summary of this revision. (Show Details)Jan 31 2019, 5:40 PM
NoQ updated this revision to Diff 187294.Feb 18 2019, 8:27 PM

Clean up a bit. Fix kern_return_t accidentally defined as unsigned. Expand one comment.

NoQ updated this revision to Diff 187495.Feb 19 2019, 7:26 PM

Add a test that demonstrates that we correctly handle unknown or under-constrained return values. I.e., it would fail if we replace

!State->isNonNull(V).isConstrainedTrue()

with

!State->isNonNull(V).isConstrainedTrue()

Rename the test file to *.mm because Objective-C tests are going to be added eventually.

LGTM with a minor grammatical nits inline. It is great to see a checker for this!

clang/include/clang/StaticAnalyzer/Checkers/Checkers.td
817 ↗(On Diff #187495)

Minor grammar nit: "Find violations of *the* Mach Interface Generator calling convention"

clang/lib/StaticAnalyzer/Checkers/MIGChecker.cpp
131 ↗(On Diff #187495)

Minor wording nit: "This is *a* use-after-free vulnerability because *the* caller will try to deallocate it again"

dcoughlin accepted this revision.Feb 19 2019, 8:34 PM
This revision is now accepted and ready to land.Feb 19 2019, 8:34 PM
NoQ updated this revision to Diff 187870.Feb 21 2019, 3:00 PM
NoQ marked 2 inline comments as done.

Fxd! Also addressed D58368#1404768.

This revision was automatically updated to reflect the committed changes.