Skip to content

[PAC] Make __is_function_overridden pauth-aware on ELF platforms #107498

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

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

asl
Copy link
Collaborator

@asl asl commented Sep 6, 2024

Apparently, there are two almost identical implementations: one for MachO and another one for ELF. The ELF bits somehow slipped while #84573 was reviewed.

The particular implementation is identical to MachO case.

@asl asl requested a review from kovdan01 September 6, 2024 01:16
@asl asl requested a review from a team as a code owner September 6, 2024 01:16
@asl asl changed the title Make __is_function_overridden pauth-aware on ELF platforms [PAC] Make __is_function_overridden pauth-aware on ELF platforms Sep 6, 2024
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Sep 6, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 6, 2024

@llvm/pr-subscribers-libcxx

Author: Anton Korobeynikov (asl)

Changes

Apparently, there are two almost identical implementations: one for MachO and another one for ELF. The ELF bits somehow slipped while #84573 was reviewed.

The particular implementation is identical to MachO case.


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

1 Files Affected:

  • (modified) libcxx/src/include/overridable_function.h (+8)
diff --git a/libcxx/src/include/overridable_function.h b/libcxx/src/include/overridable_function.h
index e83ca7be7befaf..c56a00defbc56b 100644
--- a/libcxx/src/include/overridable_function.h
+++ b/libcxx/src/include/overridable_function.h
@@ -116,6 +116,14 @@ _LIBCPP_HIDE_FROM_ABI bool __is_function_overridden(_Ret (*__fptr)(_Args...)) no
   uintptr_t __end   = reinterpret_cast<uintptr_t>(&__stop___lcxx_override);
   uintptr_t __ptr   = reinterpret_cast<uintptr_t>(__fptr);
 
+#  if __has_feature(ptrauth_calls)
+  // We must pass a void* to ptrauth_strip since it only accepts a pointer type. Also, in particular,
+  // we must NOT pass a function pointer, otherwise we will strip the function pointer, and then attempt
+  // to authenticate and re-sign it when casting it to a uintptr_t again, which will fail because we just
+  // stripped the function pointer.
+  __ptr = reinterpret_cast<uintptr_t>(ptrauth_strip(reinterpret_cast<void*>(__ptr), ptrauth_key_function_pointer));
+#  endif
+
   return __ptr < __start || __ptr > __end;
 }
 _LIBCPP_END_NAMESPACE_STD

Copy link
Member

@ldionne ldionne left a comment

Choose a reason for hiding this comment

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

LGTM with a comment.

@asl asl force-pushed the users/asl/pauth-libcxx-overridable_function branch from bf26f91 to d2ef188 Compare September 9, 2024 19:30
@asl asl force-pushed the users/asl/pauth-libcxx-overridable_function branch from d2ef188 to c3c9611 Compare September 9, 2024 19:30
@asl asl merged commit 33c1325 into main Sep 9, 2024
63 of 65 checks passed
@asl asl deleted the users/asl/pauth-libcxx-overridable_function branch September 9, 2024 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants