-
Notifications
You must be signed in to change notification settings - Fork 12k
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
[msan] Unpoison indirect outputs for userspace when -msan-handle-asm-conservative is specified #77393
Conversation
Created using spr 1.3.4
@llvm/pr-subscribers-compiler-rt-sanitizer @llvm/pr-subscribers-llvm-transforms Author: Fangrui Song (MaskRay) ChangesKMSAN defaults to unsigned f() {
unsigned v;
// __msan_instrument_asm_store unpoisons v before invoking the asm.
asm("movl $1,%0" : "=m"(v));
return v;
} Extend the mechanism to userspace, but require explicit As Link: google/sanitizers#192 Full diff: https://github.com/llvm/llvm-project/pull/77393.diff 2 Files Affected:
diff --git a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
index 94af63da38c82c..38270716bc6996 100644
--- a/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
+++ b/llvm/lib/Transforms/Instrumentation/MemorySanitizer.cpp
@@ -4103,7 +4103,11 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
// do the usual thing: check argument shadow and mark all outputs as
// clean. Note that any side effects of the inline asm that are not
// immediately visible in its constraints are not handled.
- if (ClHandleAsmConservative && MS.CompileKernel)
+ // For now, handle inline asm by default for KMSAN.
+ bool HandleAsm = ClHandleAsmConservative.getNumOccurrences()
+ ? ClHandleAsmConservative
+ : MS.CompileKernel;
+ if (HandleAsm)
visitAsmInstruction(CB);
else
visitInstruction(CB);
@@ -4557,7 +4561,13 @@ struct MemorySanitizerVisitor : public InstVisitor<MemorySanitizerVisitor> {
return;
Value *SizeVal =
IRB.CreateTypeSize(MS.IntptrTy, DL.getTypeStoreSize(ElemTy));
- IRB.CreateCall(MS.MsanInstrumentAsmStoreFn, {Operand, SizeVal});
+ if (MS.CompileKernel) {
+ IRB.CreateCall(MS.MsanInstrumentAsmStoreFn, {Operand, SizeVal});
+ } else {
+ auto [ShadowPtr, _] =
+ getShadowOriginPtrUserspace(Operand, IRB, IRB.getInt8Ty(), Align(1));
+ IRB.CreateAlignedStore(getCleanShadow(ElemTy), ShadowPtr, Align(1));
+ }
}
/// Get the number of output arguments returned by pointers.
diff --git a/llvm/test/Instrumentation/MemorySanitizer/msan_asm_conservative.ll b/llvm/test/Instrumentation/MemorySanitizer/msan_asm_conservative.ll
index 1239698f3ac326..9a501ee6954c9c 100644
--- a/llvm/test/Instrumentation/MemorySanitizer/msan_asm_conservative.ll
+++ b/llvm/test/Instrumentation/MemorySanitizer/msan_asm_conservative.ll
@@ -1,10 +1,14 @@
; Test for handling of asm constraints in MSan instrumentation.
+; RUN: opt < %s -msan-check-access-address=0 -msan-handle-asm-conservative=0 -S -passes=msan 2>&1 | \
+; RUN: FileCheck %s
+; RUN: opt < %s -msan-check-access-address=0 -msan-handle-asm-conservative=1 -S -passes=msan 2>&1 | \
+; RUN: FileCheck --check-prefixes=CHECK,USER-CONS %s
; RUN: opt < %s -msan-kernel=1 -msan-check-access-address=0 \
-; RUN: -msan-handle-asm-conservative=0 -S -passes=msan 2>&1 | FileCheck \
-; RUN: "-check-prefix=CHECK" %s
+; RUN: -msan-handle-asm-conservative=0 -S -passes=msan 2>&1 | FileCheck \
+; RUN: --check-prefixes=CHECK,KMSAN %s
; RUN: opt < %s -msan-kernel=1 -msan-check-access-address=0 \
-; RUN: -msan-handle-asm-conservative=1 -S -passes=msan 2>&1 | FileCheck \
-; RUN: "-check-prefixes=CHECK,CHECK-CONS" %s
+; RUN: -msan-handle-asm-conservative=1 -S -passes=msan 2>&1 | FileCheck \
+; RUN: --check-prefixes=CHECK,KMSAN,CHECK-CONS %s
target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
target triple = "x86_64-unknown-linux-gnu"
@@ -46,9 +50,9 @@ entry:
; CHECK: [[IS1_F1:%.*]] = load i32, ptr @is1, align 4
; CHECK: call void @__msan_warning
; CHECK: call i32 asm "",{{.*}}(i32 [[IS1_F1]])
-; CHECK: [[PACK1_F1:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id1{{.*}})
-; CHECK: [[EXT1_F1:%.*]] = extractvalue { ptr, ptr } [[PACK1_F1]], 0
-; CHECK: store i32 0, ptr [[EXT1_F1]]
+; KMSAN: [[PACK1_F1:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id1{{.*}})
+; KMSAN: [[EXT1_F1:%.*]] = extractvalue { ptr, ptr } [[PACK1_F1]], 0
+; KMSAN: store i32 0, ptr [[EXT1_F1]]
; Two input registers, two output registers:
@@ -69,14 +73,14 @@ entry:
; CHECK: [[IS1_F2:%.*]] = load i32, ptr @is1, align 4
; CHECK: [[IS2_F2:%.*]] = load i32, ptr @is2, align 4
; CHECK: call void @__msan_warning
-; CHECK: call void @__msan_warning
+; KMSAN: call void @__msan_warning
; CHECK: call { i32, i32 } asm "",{{.*}}(i32 [[IS1_F2]], i32 [[IS2_F2]])
-; CHECK: [[PACK1_F2:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id1{{.*}})
-; CHECK: [[EXT1_F2:%.*]] = extractvalue { ptr, ptr } [[PACK1_F2]], 0
-; CHECK: store i32 0, ptr [[EXT1_F2]]
-; CHECK: [[PACK2_F2:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id2{{.*}})
-; CHECK: [[EXT2_F2:%.*]] = extractvalue { ptr, ptr } [[PACK2_F2]], 0
-; CHECK: store i32 0, ptr [[EXT2_F2]]
+; KMSAN: [[PACK1_F2:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id1{{.*}})
+; KMSAN: [[EXT1_F2:%.*]] = extractvalue { ptr, ptr } [[PACK1_F2]], 0
+; KMSAN: store i32 0, ptr [[EXT1_F2]]
+; KMSAN: [[PACK2_F2:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id2{{.*}})
+; KMSAN: [[EXT2_F2:%.*]] = extractvalue { ptr, ptr } [[PACK2_F2]], 0
+; KMSAN: store i32 0, ptr [[EXT2_F2]]
; Input same as output, used twice:
; asm("" : "=r" (id1), "=r" (id2) : "r" (id1), "r" (id2));
@@ -96,14 +100,14 @@ entry:
; CHECK: [[ID1_F3:%.*]] = load i32, ptr @id1, align 4
; CHECK: [[ID2_F3:%.*]] = load i32, ptr @id2, align 4
; CHECK: call void @__msan_warning
-; CHECK: call void @__msan_warning
+; KMSAN: call void @__msan_warning
; CHECK: call { i32, i32 } asm "",{{.*}}(i32 [[ID1_F3]], i32 [[ID2_F3]])
-; CHECK: [[PACK1_F3:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id1{{.*}})
-; CHECK: [[EXT1_F3:%.*]] = extractvalue { ptr, ptr } [[PACK1_F3]], 0
-; CHECK: store i32 0, ptr [[EXT1_F3]]
-; CHECK: [[PACK2_F3:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id2{{.*}})
-; CHECK: [[EXT2_F3:%.*]] = extractvalue { ptr, ptr } [[PACK2_F3]], 0
-; CHECK: store i32 0, ptr [[EXT2_F3]]
+; KMSAN: [[PACK1_F3:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id1{{.*}})
+; KMSAN: [[EXT1_F3:%.*]] = extractvalue { ptr, ptr } [[PACK1_F3]], 0
+; KMSAN: store i32 0, ptr [[EXT1_F3]]
+; KMSAN: [[PACK2_F3:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id2{{.*}})
+; KMSAN: [[EXT2_F3:%.*]] = extractvalue { ptr, ptr } [[PACK2_F3]], 0
+; KMSAN: store i32 0, ptr [[EXT2_F3]]
; One of the input registers is also an output:
@@ -124,14 +128,14 @@ entry:
; CHECK: [[ID1_F4:%.*]] = load i32, ptr @id1, align 4
; CHECK: [[IS1_F4:%.*]] = load i32, ptr @is1, align 4
; CHECK: call void @__msan_warning
-; CHECK: call void @__msan_warning
+; KMSAN: call void @__msan_warning
; CHECK: call { i32, i32 } asm "",{{.*}}(i32 [[ID1_F4]], i32 [[IS1_F4]])
-; CHECK: [[PACK1_F4:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id1{{.*}})
-; CHECK: [[EXT1_F4:%.*]] = extractvalue { ptr, ptr } [[PACK1_F4]], 0
-; CHECK: store i32 0, ptr [[EXT1_F4]]
-; CHECK: [[PACK2_F4:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id2{{.*}})
-; CHECK: [[EXT2_F4:%.*]] = extractvalue { ptr, ptr } [[PACK2_F4]], 0
-; CHECK: store i32 0, ptr [[EXT2_F4]]
+; KMSAN: [[PACK1_F4:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id1{{.*}})
+; KMSAN: [[EXT1_F4:%.*]] = extractvalue { ptr, ptr } [[PACK1_F4]], 0
+; KMSAN: store i32 0, ptr [[EXT1_F4]]
+; KMSAN: [[PACK2_F4:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id2{{.*}})
+; KMSAN: [[EXT2_F4:%.*]] = extractvalue { ptr, ptr } [[PACK2_F4]], 0
+; KMSAN: store i32 0, ptr [[EXT2_F4]]
; One input register, three output registers:
@@ -153,15 +157,15 @@ entry:
; CHECK: [[IS1_F5:%.*]] = load i32, ptr @is1, align 4
; CHECK: call void @__msan_warning
; CHECK: call { i32, i32, i32 } asm "",{{.*}}(i32 [[IS1_F5]])
-; CHECK: [[PACK1_F5:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id1{{.*}})
-; CHECK: [[EXT1_F5:%.*]] = extractvalue { ptr, ptr } [[PACK1_F5]], 0
-; CHECK: store i32 0, ptr [[EXT1_F5]]
-; CHECK: [[PACK2_F5:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id2{{.*}})
-; CHECK: [[EXT2_F5:%.*]] = extractvalue { ptr, ptr } [[PACK2_F5]], 0
-; CHECK: store i32 0, ptr [[EXT2_F5]]
-; CHECK: [[PACK3_F5:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id3{{.*}})
-; CHECK: [[EXT3_F5:%.*]] = extractvalue { ptr, ptr } [[PACK3_F5]], 0
-; CHECK: store i32 0, ptr [[EXT3_F5]]
+; KMSAN: [[PACK1_F5:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id1{{.*}})
+; KMSAN: [[EXT1_F5:%.*]] = extractvalue { ptr, ptr } [[PACK1_F5]], 0
+; KMSAN: store i32 0, ptr [[EXT1_F5]]
+; KMSAN: [[PACK2_F5:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id2{{.*}})
+; KMSAN: [[EXT2_F5:%.*]] = extractvalue { ptr, ptr } [[PACK2_F5]], 0
+; KMSAN: store i32 0, ptr [[EXT2_F5]]
+; KMSAN: [[PACK3_F5:%.*]] = call {{.*}} @__msan_metadata_ptr_for_store_4({{.*}}@id3{{.*}})
+; KMSAN: [[EXT3_F5:%.*]] = extractvalue { ptr, ptr } [[PACK3_F5]], 0
+; KMSAN: store i32 0, ptr [[EXT3_F5]]
; 2 input memory args, 2 output memory args:
@@ -173,6 +177,8 @@ entry:
}
; CHECK-LABEL: @f_2i_2o_mem
+; USER-CONS: store i32 0, ptr inttoptr (i64 xor (i64 ptrtoint (ptr @id1 to i64), i64 87960930222080) to ptr), align 1
+; USER-CONS: store i32 0, ptr inttoptr (i64 xor (i64 ptrtoint (ptr @id2 to i64), i64 87960930222080) to ptr), align 1
; CHECK-CONS: call void @__msan_instrument_asm_store({{.*}}@id1{{.*}}, i64 4)
; CHECK-CONS: call void @__msan_instrument_asm_store({{.*}}@id2{{.*}}, i64 4)
; CHECK: call void asm "", "=*m,=*m,*m,*m,~{dirflag},~{fpsr},~{flags}"(ptr elementtype(i32) @id1, ptr elementtype(i32) @id2, ptr elementtype(i32) @is1, ptr elementtype(i32) @is2)
@@ -190,6 +196,7 @@ entry:
; CHECK-LABEL: @f_1i_1o_memreg
; CHECK: [[IS1_F7:%.*]] = load i32, ptr @is1, align 4
+; USER-CONS: store i32 0, ptr inttoptr (i64 xor (i64 ptrtoint (ptr @id1 to i64), i64 87960930222080) to ptr), align 1
; CHECK-CONS: call void @__msan_instrument_asm_store({{.*}}@id1{{.*}}, i64 4)
; CHECK: call void @__msan_warning
; CHECK: call i32 asm "", "=r,=*m,r,*m,~{dirflag},~{fpsr},~{flags}"(ptr elementtype(i32) @id1, i32 [[IS1_F7]], ptr elementtype(i32) @is1)
@@ -208,6 +215,7 @@ entry:
}
; CHECK-LABEL: @f_3o_reg_mem_reg
+; USER-CONS: store i32 0, ptr inttoptr (i64 xor (i64 ptrtoint (ptr @id2 to i64), i64 87960930222080) to ptr), align 1
; CHECK-CONS: call void @__msan_instrument_asm_store(ptr @id2, i64 4)
; CHECK: call { i32, i32 } asm "", "=r,=*m,=r,~{dirflag},~{fpsr},~{flags}"(ptr elementtype(i32) @id2)
@@ -232,10 +240,11 @@ entry:
; CHECK: [[PAIR1_F9:%.*]] = load {{.*}} @pair1
; CHECK: [[C1_F9:%.*]] = load {{.*}} @c1
; CHECK: [[MEMCPY_S1_F9:%.*]] = load {{.*}} @memcpy_s1
+; USER-CONS: store { i32, i32 } zeroinitializer, ptr inttoptr (i64 xor (i64 ptrtoint (ptr @pair2 to i64), i64 87960930222080) to ptr), align 1
; CHECK-CONS: call void @__msan_instrument_asm_store({{.*}}@pair2{{.*}}, i64 8)
; CHECK: call void @__msan_warning
-; CHECK: call void @__msan_warning
-; CHECK: call void @__msan_warning
+; KMSAN: call void @__msan_warning
+; KMSAN: call void @__msan_warning
; CHECK: call { i8, ptr } asm "", "=*r,=r,=r,r,r,r,~{dirflag},~{fpsr},~{flags}"(ptr elementtype(%struct.pair) @pair2, {{.*}}[[PAIR1_F9]], i8 [[C1_F9]], {{.*}} [[MEMCPY_S1_F9]])
; Three inputs and three outputs of different types: a pair, a char, a function pointer.
@@ -248,9 +257,12 @@ entry:
}
; CHECK-LABEL: @f_3i_3o_complex_mem
-; CHECK-CONS: call void @__msan_instrument_asm_store({{.*}}@pair2{{.*}}, i64 8)
-; CHECK-CONS: call void @__msan_instrument_asm_store({{.*}}@c2{{.*}}, i64 1)
-; CHECK-CONS: call void @__msan_instrument_asm_store({{.*}}@memcpy_d1{{.*}}, i64 8)
+; USER-CONS: store { i32, i32 } zeroinitializer, ptr inttoptr (i64 xor (i64 ptrtoint (ptr @pair2 to i64), i64 87960930222080) to ptr), align 1
+; USER-CONS-NEXT: store i8 0, ptr inttoptr (i64 xor (i64 ptrtoint (ptr @c2 to i64), i64 87960930222080) to ptr), align 1
+; USER-CONS-NEXT: store i64 0, ptr inttoptr (i64 xor (i64 ptrtoint (ptr @memcpy_d1 to i64), i64 87960930222080) to ptr), align 1
+; CHECK-CONS: call void @__msan_instrument_asm_store({{.*}}@pair2{{.*}}, i64 8)
+; CHECK-CONS: call void @__msan_instrument_asm_store({{.*}}@c2{{.*}}, i64 1)
+; CHECK-CONS: call void @__msan_instrument_asm_store({{.*}}@memcpy_d1{{.*}}, i64 8)
; CHECK: call void asm "", "=*m,=*m,=*m,*m,*m,*m,~{dirflag},~{fpsr},~{flags}"(ptr elementtype(%struct.pair) @pair2, ptr elementtype(i8) @c2, ptr elementtype(ptr) @memcpy_d1, ptr elementtype(%struct.pair) @pair1, ptr elementtype(i8) @c1, ptr elementtype(ptr) @memcpy_s1)
@@ -278,8 +290,8 @@ cleanup: ; preds = %entry, %skip_label
}
; CHECK-LABEL: @asm_goto
-; CHECK: [[LOAD_ARG:%.*]] = load {{.*}} %_msarg
-; CHECK: [[CMP:%.*]] = icmp ne {{.*}} [[LOAD_ARG]], 0
-; CHECK: br {{.*}} [[CMP]], label %[[LABEL:.*]], label
-; CHECK: [[LABEL]]:
-; CHECK-NEXT: call void @__msan_warning
+; KMSAN: [[LOAD_ARG:%.*]] = load {{.*}} %_msarg
+; KMSAN: [[CMP:%.*]] = icmp ne {{.*}} [[LOAD_ARG]], 0
+; KMSAN: br {{.*}} [[CMP]], label %[[LABEL:.*]], label
+; KMSAN: [[LABEL]]:
+; KMSAN-NEXT: call void @__msan_warning
|
[cpuid-x86.h] Add MSAN annotations to mark memory as initialized To be used with clang -fsanitize=memory (which currently requires additional flags, see llvm/llvm-project#77393)
Ping:) |
} else { | ||
auto [ShadowPtr, _] = | ||
getShadowOriginPtrUserspace(Operand, IRB, IRB.getInt8Ty(), Align(1)); | ||
IRB.CreateAlignedStore(getCleanShadow(ElemTy), ShadowPtr, Align(1)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason we use the the Aligned variant? Align(1) should be the same as unspecified alignment, so we can just use CreateStore
, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ElemTy
utilizes elementtype
(ptr elementtype(i32) @id1
=> i32, which does not encode the alignment).
An unspecified alignment uses the default, ElemTy
's alignment, which is 4 in the i32 case.
However, if the element type is actually unaligned, CreateStore
-generated align 4
shadow access could cause a misalignment failure on certain architectures using -mstrict-align
.
The tests specifically check , align 1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you add this as a comment? This is quite subtle and very likely lost on cursory readers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done:)
Created using spr 1.3.4
… operands (#79924) Modify #77393 to clear shadow memory using `llvm.memset.*` when the size is large, similar to `shouldUseBZeroPlusStoresToInitialize` in clang for `-ftrivial-auto-var-init=`. The intrinsic, if lowered to libcall, will use the msan interceptor. The instruction selector lowers a `StoreInst` to multiple stores, not utilizing `memset`. When the size is large (e.g. `store { [100 x i32] } zeroinitializer, ptr %12, align 1`), the generated code will be long (and `CodeGenPrepare::optimizeInst` will even crash for a huge size). ``` // Test stack size template <class T> void DoNotOptimize(const T& var) { // deprecated by google/benchmark#1493 asm volatile("" : "+m"(const_cast<T&>(var))); } int main() { using LargeArray = std::array<int, 1000000>; auto large_stack = []() { DoNotOptimize(LargeArray()); }; /////// CodeGenPrepare::optimizeInst triggers an assertion failure when creating an integer type with a bit width>2**23 large_stack(); } ```
KMSAN defaults to
msan-handle-asm-conservative
, which inserts__msan_instrument_asm_store
calls to unpoison indirect outputs ininline assembly (e.g.
=m
constraints in source).Extend the mechanism to userspace, but require explicit
-mllvm -msan-handle-asm-conservative
for experiments for now.As
https://docs.kernel.org/dev-tools/kmsan.html#inline-assembly-instrumentation
says, this approach may mask certain errors (an indirect output may not
actually be initialized), but it also helps to avoid a lot of false
positives.
Link: google/sanitizers#192