Skip to content

[PAC] Authenticate function pointers in UBSan type checks #99590

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.

Sign up for GitHub

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
Jul 19, 2024
Merged

[PAC] Authenticate function pointers in UBSan type checks #99590

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion clang/lib/CodeGen/Address.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,9 @@ class Address {
/// Return the pointer contained in this class after authenticating it and
/// adding offset to it if necessary.
llvm::Value *emitRawPointer(CodeGenFunction &CGF) const {
return getBasePointer();
if (!isSigned())
return getBasePointer();
return emitRawPointerSlow(CGF);
}

/// Return address with different pointer, but same element type and
Expand Down
9 changes: 9 additions & 0 deletions clang/lib/CodeGen/CGExpr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5837,6 +5837,15 @@ RValue CodeGenFunction::EmitCall(QualType CalleeType, const CGCallee &OrigCallee
CGM.getLLVMContext(), {PrefixSigType, Int32Ty}, /*isPacked=*/true);

llvm::Value *CalleePtr = Callee.getFunctionPointer();
if (CGM.getCodeGenOpts().PointerAuth.FunctionPointers) {
// Use raw pointer since we are using the callee pointer as data here.
Address Addr =
Address(CalleePtr, CalleePtr->getType(),
CharUnits::fromQuantity(
CalleePtr->getPointerAlignment(CGM.getDataLayout())),
Callee.getPointerAuthInfo(), nullptr);
CalleePtr = Addr.emitRawPointer(*this);
}

// On 32-bit Arm, the low bit of a function pointer indicates whether
// it's using the Arm or Thumb instruction set. The actual first
Expand Down
4 changes: 4 additions & 0 deletions clang/lib/CodeGen/CGPointerAuth.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,10 @@ Address Address::getResignedAddress(const CGPointerAuthInfo &NewInfo,
/*Offset=*/nullptr, isKnownNonNull());
}

llvm::Value *Address::emitRawPointerSlow(CodeGenFunction &CGF) const {
return CGF.getAsNaturalPointerTo(*this, QualType());
}

llvm::Value *LValue::getPointer(CodeGenFunction &CGF) const {
assert(isSimple());
return emitResignedPointer(getType(), CGF);
Expand Down
4 changes: 4 additions & 0 deletions clang/test/CodeGen/ubsan-function.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
// RUN: %clang_cc1 -triple aarch64_be-linux-gnu -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s --check-prefixes=CHECK,GNU,64
// RUN: %clang_cc1 -triple arm-none-eabi -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all | FileCheck %s --check-prefixes=CHECK,ARM,GNU,32

// RUN: %clang_cc1 -triple arm64e-apple-ios -emit-llvm -o - %s -fsanitize=function -fno-sanitize-recover=all -fptrauth-calls | FileCheck %s --check-prefixes=CHECK,GNU,64,64e

// GNU: define{{.*}} void @_Z3funv() #0 !func_sanitize ![[FUNCSAN:.*]] {
// MSVC: define{{.*}} void @"?fun@@YAXXZ"() #0 !func_sanitize ![[FUNCSAN:.*]] {
void fun() {}
Expand All @@ -13,6 +15,8 @@ void fun() {}
// ARM: ptrtoint ptr {{.*}} to i32, !nosanitize !5
// ARM: and i32 {{.*}}, -2, !nosanitize !5
// ARM: inttoptr i32 {{.*}} to ptr, !nosanitize !5
// 64e: %[[STRIPPED:.*]] = ptrtoint ptr {{.*}} to i64, !nosanitize
// 64e: call i64 @llvm.ptrauth.auth(i64 %[[STRIPPED]], i32 0, i64 0), !nosanitize
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sadly we are not checking full sequence here :( But this is not a problem of this patch, it is just underlying test done this way

// CHECK: getelementptr <{ i32, i32 }>, ptr {{.*}}, i32 -1, i32 0, !nosanitize
// CHECK: load i32, ptr {{.*}}, align {{.*}}, !nosanitize
// CHECK: icmp eq i32 {{.*}}, -1056584962, !nosanitize
Expand Down
Loading