Tracking Issue for LLVM AllocToken and Heap Partitioning Support for Rust

[rcvalle]

View all comments

This is a tracking issue for the LLVM AllocToken and Heap Partitioning Support for Rust project (see the design document draft).

Steps

• Add option to the Rust compiler driver and frontend (i.e., -Zsanitizer=alloc-token and related options).
• Add support for emitting allocation token function attributes, module flags, and allocation token hint metadata to the Rust compiler code generation.
• Implement the default pointer-split heap partitioning scheme in the rustc_sanitizers crate.
• Define allocation token classifications and type name encodings for cross-language LLVM AllocToken and heap partitioning support.
• Change the initial implementation to use the defined classifications and type name encodings for cross-language LLVM AllocToken and heap partitioning support.
• Add typed allocation paths and allocation token hints to the Rust standard library.
• Add token-enabled allocator interface support to the Rust standard library and to the Rust compiler allocator shim generation.
• Add the alloc_token_infer intrinsic.
• Create documentation and tests.

Unresolved Questions

None.

Implementation history

• Add LLVM AllocToken and Heap Partitioning Support to the Rust compiler. #160298

[bjorn3]

Left a couple of review comments on rcvalle@41bf600

[bjorn3]

I see you reacted to my comments, but github just doesn't show any commit comments at all anymore. The file sidebar shows comment counts, but nothing is shown inside the actual diff: rcvalle@41bf600

[rcvalle]

Yes, it's happening to me as well. I had to expand all the files to be able to see your comments, but even doing so the third comment didn't show up--I wanted to reply I fixed it as well and I couldn't. Let me open a draft PR sometime later this week and we can continue the discussions and review there.

[nikic]

usize/isize should not be classified as pointer-containing types.

[rcvalle]

usize/isize should not be classified as pointer-containing types.

It's a bit complicated (see the Classification compatibility for the pointer-split heap partitioning scheme section in the design doc). The tl;dr is that the classification is conservative¹ and consistent with how Clang classifies uintptr_t and intptr_t, which end up as usize and isize across the FFI boundary in "mixed binaries"--for cross-language LLVM AllocToken and heap partitioning support--and because pointers may be stored as usize (e.g., via ptr::expose_provenance and ptr::with_exposed_provenance, and AtomicUsize).

Footnotes

1. Misclassifying a type not containing pointers into the partition containing pointers only loses part of the separation benefit for that type, while misclassifying a type containing pointers into the partition not containing pointers breaks the guarantee of the partition not containing pointers. ↩

[RalfJung]

and because pointers may be stored as usize (e.g., via ptr::expose_provenance and ptr::with_exposed_provenance, and AtomicUsize).

No, pointers can't be stored in usize. usize cannot hold provenance. The address part of a pointer can be stored in usize, but that's not a whole pointer. And almost all uses of usize are not for pointers.

usize should be treated like size_t, not like uintptr_t.

[RalfJung]

Tracking issues are typically for already accepted proposals. Where was the prior discussion and which team has accepted this proposal using which process?

[rcvalle]

and because pointers may be stored as usize (e.g., via ptr::expose_provenance and ptr::with_exposed_provenance, and AtomicUsize).

No, pointers can't be stored in usize. usize cannot hold provenance. The address part of a pointer can be stored in usize, but that's not a whole pointer. And almost all uses of usize are not for pointers.

usize should be treated like size_t, not like uintptr_t.

Yes, an address is what an attacker would target in e.g., a heap-based out-of-bounds write, and that's what we want to hint the AllocTokenPass about the possibility of, and it doesn't change the language semantics or the program correctness in any way. Think of it as similar to a low-level optimization hint, and what a token-enabled memory allocator does with the token identifier derived from that (by the AllocTokenPass) is implementation specific.

[rcvalle]

Tracking issues are typically for already accepted proposals. Where was the prior discussion and which team has accepted this proposal using which process?

Sorry, almost all sanitizers support were done without an MCP or tracking issue (or a design document). I mostly use these tracking issues to track our Compiler team and PG exploit mitigations work (and for possibly future stabilization) and didn't know they were strictly for that so apologies. Happy to open an MCP or do as you suggested in the PR in this case if you think that would be helpful. Let me know.

[RalfJung]

Thanks for explaining. I haven't seen such tracking issues before so I was just a little surprised.

The public lib API change will definitely need an ACP. As for the MCP, the change seemed big enough (with new flags and new attributes) for me to warrant one; I assume @bjorn3 would second it fairly quickly but it'd be a chance for the rest of the team to chime in with feedback.

Yes, an address is what an attacker would target in e.g., a heap-based out-of-bounds write, and that's what we want to hint the AllocTokenPass about the possibility of, and it doesn't change the language semantics or the program correctness in any way. Think of it as similar to a low-level optimization hint, and what a token-enabled memory allocator does with the token identifier derived from that (by the AllocTokenPass) is implementation specific.

I see. Still seems odd to me given that most usize are not pointers so mixing them with pointers could be bad, but you are the mitigation expert here. :)

[rcvalle]

@RalfJung will do that as soon as I've some time. Thank you!