Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PAC][compiler-rt][UBSan] Strip signed vptr instead of authenticating it #100153

Merged
merged 1 commit into from
Jul 23, 2024

Conversation

ahatanak
Copy link
Collaborator

@ahatanak ahatanak commented Jul 23, 2024

vptr cannot be authenticated without knowing the class type if it was signed with type discrimination.

Co-authored-by: Oliver Hunt oliver@apple.com

vtpr cannot be authenticated without knowing the class type if it was
signed with type discrimination.

Co-authored-by: Oliver Hunt <oliver@apple.com>
@llvmbot
Copy link
Member

llvmbot commented Jul 23, 2024

@llvm/pr-subscribers-compiler-rt-sanitizer

Author: Akira Hatanaka (ahatanak)

Changes

vtpr cannot be authenticated without knowing the class type if it was signed with type discrimination.

Co-authored-by: Oliver Hunt <oliver@apple.com>


Full diff: https://github.com/llvm/llvm-project/pull/100153.diff

1 Files Affected:

  • (modified) compiler-rt/lib/ubsan/ubsan_type_hash_itanium.cpp (+1-1)
diff --git a/compiler-rt/lib/ubsan/ubsan_type_hash_itanium.cpp b/compiler-rt/lib/ubsan/ubsan_type_hash_itanium.cpp
index 468a8fcd603f0..15788574dd995 100644
--- a/compiler-rt/lib/ubsan/ubsan_type_hash_itanium.cpp
+++ b/compiler-rt/lib/ubsan/ubsan_type_hash_itanium.cpp
@@ -207,7 +207,7 @@ struct VtablePrefix {
   std::type_info *TypeInfo;
 };
 VtablePrefix *getVtablePrefix(void *Vtable) {
-  Vtable = ptrauth_auth_data(Vtable, ptrauth_key_cxx_vtable_pointer, 0);
+  Vtable = ptrauth_strip(Vtable, ptrauth_key_cxx_vtable_pointer);
   VtablePrefix *Vptr = reinterpret_cast<VtablePrefix*>(Vtable);
   VtablePrefix *Prefix = Vptr - 1;
   if (!IsAccessibleMemoryRange((uptr)Prefix, sizeof(VtablePrefix)))

@asl asl added this to the LLVM 19.X Release milestone Jul 23, 2024
Copy link
Contributor

@kovdan01 kovdan01 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

I suppose that in future preprocessor conditions against vtable discrimination enabled/disabled might be used, and if disabled, old code with ptrauth_auth_data and zero discriminator should be OK.

@ojhunt
Copy link
Contributor

ojhunt commented Jul 23, 2024

Thanks!

I suppose that in future preprocessor conditions against vtable discrimination enabled/disabled might be used, and if disabled, old code with ptrauth_auth_data and zero discriminator should be OK.

It can't know what the discriminator is unless it's explicitly provided, this particular function may not have abi constraints so it could in theory be updated to take the full discriminator as an argument.

For dynamic_cast we use ptrauth_strip, but have the codegen for dynamic cast perform a forced load+auth of the vtable pointer prior to the call. It's not perfect but is the only really ABI stable solution.

@asl asl merged commit 0a6a3c1 into llvm:main Jul 23, 2024
8 of 10 checks passed
@asl
Copy link
Collaborator

asl commented Jul 23, 2024

/cherry-pick 0a6a3c1

@asl asl modified the milestone: LLVM 19.X Release Jul 23, 2024
llvmbot pushed a commit to llvmbot/llvm-project that referenced this pull request Jul 23, 2024
… it (llvm#100153)

vptr cannot be authenticated without knowing the class type if it was
signed with type discrimination.

Co-authored-by: Oliver Hunt <oliver@apple.com>
(cherry picked from commit 0a6a3c1)
@llvmbot
Copy link
Member

llvmbot commented Jul 23, 2024

/pull-request #100219

@ahatanak ahatanak deleted the ptrauth-ubsan-vptr branch July 23, 2024 22:33
tru pushed a commit to llvmbot/llvm-project that referenced this pull request Jul 24, 2024
… it (llvm#100153)

vptr cannot be authenticated without knowing the class type if it was
signed with type discrimination.

Co-authored-by: Oliver Hunt <oliver@apple.com>
(cherry picked from commit 0a6a3c1)
yuxuanchen1997 pushed a commit that referenced this pull request Jul 25, 2024
… it (#100153)

vptr cannot be authenticated without knowing the class type if it was
signed with type discrimination.

Co-authored-by: Oliver Hunt <oliver@apple.com>
@ahatanak ahatanak requested review from efriedma-quic and removed request for efriedma-quic August 2, 2024 02:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants