ztong0001 created this revision.
Herald added subscribers: ormris, dexonsmith, jdoerfert, steven_wu, hiraditya.
ztong0001 requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.
Adding __attribute__((no_sanitize("bounds"))) is not really disabling bounds
checking, i.e. disable -fsanitize=array-bounds -fsanitize=local-bounds. or
-fsanitize=bounds.
We have a potential user in Linux kennel which is currently affected by
-fsanitize=local-bounds.
Linux kernel(5.17) has a function pskb_expand_head (net/core/skbuff.c)
currently experiencing false positive when CONFIG_UBSAN_LOCAL_BOUNDS is enabled
(i.e. supply -fsanitize=array-bounds -fsanitize=local-bounds).
The aforementioned function does the following which will create a false
positive.
char* p = kmalloc(M); // allocate buffer, size is M
int actual_size = ksize(p); // return actual allocated size in the slab
allocator, this will guaranteed to be larger than M + N
p[actual_size - N+1] = 0; //Out of bound access
LLVM assume the size of the buffer is M, thus the later p[actual_size-N+1]
access will be treated as out of bound access.
However, this function deliberately use the actual allocated size instead of M.
Kernel will be unhappy as bounds check fail and it crash hard with the
following message:
[ 0.060423] PTP clock support registered
[ 0.060820] invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
[ 0.061232] CPU: 0 PID: 1 Comm: swapper/0 Not tainted
5.17.0-rc3-00247-g83e396641110-dirty #102
[ 0.061418] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
1.13.0-1ubuntu1.1 04/01/2014
[ 0.061418] RIP: 0010:pskb_expand_head+0x38b/0x3b0
[ 0.061418] Code: 84 fc fd ff ff be 01 00 00 00 b8 01 00 00 00 3e 0f c1 47
18 85 c0 74 14 8d 48 01 0
9 c1 0f 89 de fd ff ff eb 0c 67 0f b9 40 12 <0f> 0b be 02 00 00 00 48 83 c7 18
e8 25 b7 cc ff e9 c2 fd
ff ff 0f
[ 0.061418] RSP: 0018:ffffc9000000bb60 EFLAGS: 00010287
[ 0.061418] RAX: 00000000000006c0 RBX: ffff888002e92000 RCX: 000000000000ffff
[ 0.061418] RDX: 00000000000006c0 RSI: ffff888002cebef0 RDI: 000000000002c800
[ 0.061418] RBP: ffff888002cebec0 R08: ffff888002ceb000 R09: ffffffff8174ed1f
[ 0.061418] R10 <https://reviews.llvm.org/source/svn-test-suite/>:
ffffea00000b3a00 R11 <https://reviews.llvm.org/source/libunwind/>:
0000000000000000 R12 <https://reviews.llvm.org/source/lldb/>: ffff888002ceb000
[ 0.061418] R13 <https://reviews.llvm.org/source/pstl/>: 0000000000000000
R14 <https://reviews.llvm.org/source/llvm-github/>: 0000000000012a20 R15
<https://reviews.llvm.org/source/zorg/>: ffff888002e0f000
[ 0.061418] FS: 0000000000000000(0000) GS:ffff88803ec00000(0000)
knlGS:0000000000000000
[ 0.061418] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 0.061418] CR2: ffff888002a01000 CR3: 000000000220c000 CR4: 0000000000350ef0
[ 0.061418] Call Trace:
[ 0.061418] <TASK>
[ 0.061418] netlink_trim+0x83/0xa0
[ 0.061418] netlink_broadcast+0x2a/0x5a0
[ 0.061418] ? nla_put+0x72/0x90
[ 0.061418] genlmsg_multicast_allns+0xcc/0x120
[ 0.061418] genl_ctrl_event+0x219/0x370
[ 0.061418] genl_register_family+0x5b8/0x660
[ 0.061418] ? rtnl_register_internal+0x14d/0x180
[ 0.061418] ? tca_get_fill+0x120/0x120
[ 0.061418] ? tc_ctl_action+0x5b0/0x5b0
[ 0.061418] ? genl_init+0x31/0x31
[ 0.061418] ethnl_init+0xd/0x50
[ 0.061418] do_one_initcall+0xa5/0x290
[ 0.061418] ? alloc_page_interleave+0x5e/0x80
[ 0.061418] ? preempt_count_add+0x5e/0xa0
[ 0.061418] ? ida_alloc_range+0x3fc/0x440
[ 0.061418] ? parse_args+0x254/0x390
[ 0.061418] do_initcall_level+0xb4/0x131
[ 0.061418] do_initcalls+0x44/0x6b
[ 0.061418] kernel_init_freeable+0xd8/0x138
[ 0.061418] ? rest_init+0xc0/0xc0
[ 0.061418] kernel_init+0x11/0x1a0
[ 0.061418] ret_from_fork+0x1f/0x30
The solution here is to add __attribute__((no_sanitize("bounds"))) to this
function to avoid local bounds checking altogether.
This patch adds __attribute__((no_sanitize("bounds"))) support to LLVM, and in
linux kernel, we will add this attribute to the function definition.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D119816
Files:
clang/lib/CodeGen/CodeGenFunction.cpp
clang/test/CodeGen/sanitize-coverage.c
llvm/include/llvm/AsmParser/LLToken.h
llvm/include/llvm/Bitcode/LLVMBitCodes.h
llvm/include/llvm/IR/Attributes.td
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
llvm/lib/Transforms/Utils/CodeExtractor.cpp
Index: llvm/lib/Transforms/Utils/CodeExtractor.cpp
===================================================================
--- llvm/lib/Transforms/Utils/CodeExtractor.cpp
+++ llvm/lib/Transforms/Utils/CodeExtractor.cpp
@@ -938,6 +938,7 @@
case Attribute::NonLazyBind:
case Attribute::NoRedZone:
case Attribute::NoUnwind:
+ case Attribute::NoSanitizeBounds:
case Attribute::NoSanitizeCoverage:
case Attribute::NullPointerIsValid:
case Attribute::OptForFuzzing:
Index: llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
===================================================================
--- llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
+++ llvm/lib/Transforms/Instrumentation/BoundsChecking.cpp
@@ -142,6 +142,9 @@
static bool addBoundsChecking(Function &F, TargetLibraryInfo &TLI,
ScalarEvolution &SE) {
+ if (F.hasFnAttribute(Attribute::NoSanitizeBounds))
+ return false;
+
const DataLayout &DL = F.getParent()->getDataLayout();
ObjectSizeOpts EvalOpts;
EvalOpts.RoundToAlign = true;
Index: llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
===================================================================
--- llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
+++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
@@ -690,6 +690,8 @@
return bitc::ATTR_KIND_NO_UNWIND;
case Attribute::NoSanitizeCoverage:
return bitc::ATTR_KIND_NO_SANITIZE_COVERAGE;
+ case Attribute::NoSanitizeBounds:
+ return bitc::ATTR_KIND_NO_SANITIZE_BOUNDS;
case Attribute::NullPointerIsValid:
return bitc::ATTR_KIND_NULL_POINTER_IS_VALID;
case Attribute::OptForFuzzing:
Index: llvm/lib/Bitcode/Reader/BitcodeReader.cpp
===================================================================
--- llvm/lib/Bitcode/Reader/BitcodeReader.cpp
+++ llvm/lib/Bitcode/Reader/BitcodeReader.cpp
@@ -1453,6 +1453,8 @@
return Attribute::NoUnwind;
case bitc::ATTR_KIND_NO_SANITIZE_COVERAGE:
return Attribute::NoSanitizeCoverage;
+ case bitc::ATTR_KIND_NO_SANITIZE_BOUNDS:
+ return Attribute::NoSanitizeBounds;
case bitc::ATTR_KIND_NULL_POINTER_IS_VALID:
return Attribute::NullPointerIsValid;
case bitc::ATTR_KIND_OPT_FOR_FUZZING:
Index: llvm/include/llvm/IR/Attributes.td
===================================================================
--- llvm/include/llvm/IR/Attributes.td
+++ llvm/include/llvm/IR/Attributes.td
@@ -178,6 +178,9 @@
/// No SanitizeCoverage instrumentation.
def NoSanitizeCoverage : EnumAttr<"nosanitize_coverage", [FnAttr]>;
+/// No SanitizeBounds instrumentation.
+def NoSanitizeBounds : EnumAttr<"nosanitize_bounds", [FnAttr]>;
+
/// Null pointer in address space zero is valid.
def NullPointerIsValid : EnumAttr<"null_pointer_is_valid", [FnAttr]>;
Index: llvm/include/llvm/Bitcode/LLVMBitCodes.h
===================================================================
--- llvm/include/llvm/Bitcode/LLVMBitCodes.h
+++ llvm/include/llvm/Bitcode/LLVMBitCodes.h
@@ -677,6 +677,7 @@
ATTR_KIND_NO_SANITIZE_COVERAGE = 76,
ATTR_KIND_ELEMENTTYPE = 77,
ATTR_KIND_DISABLE_SANITIZER_INSTRUMENTATION = 78,
+ ATTR_KIND_NO_SANITIZE_BOUNDS = 79,
};
enum ComdatSelectionKindCodes {
Index: llvm/include/llvm/AsmParser/LLToken.h
===================================================================
--- llvm/include/llvm/AsmParser/LLToken.h
+++ llvm/include/llvm/AsmParser/LLToken.h
@@ -219,6 +219,7 @@
kw_nosync,
kw_nocf_check,
kw_nounwind,
+ kw_nosanitize_bounds,
kw_nosanitize_coverage,
kw_null_pointer_is_valid,
kw_optforfuzzing,
Index: clang/test/CodeGen/sanitize-coverage.c
===================================================================
--- clang/test/CodeGen/sanitize-coverage.c
+++ clang/test/CodeGen/sanitize-coverage.c
@@ -53,6 +53,7 @@
// ASAN-NOT: call void @__asan_report_store
// MSAN-NOT: call void @__msan_warning
// BOUNDS-NOT: call void @__ubsan_handle_out_of_bounds
+ // BOUNDS-NOT: call void @llvm.trap()
// TSAN-NOT: call void @__tsan_func_entry
// UBSAN-NOT: call void @__ubsan_handle
if (n)
@@ -72,6 +73,7 @@
// ASAN-NOT: call void @__asan_report_store
// MSAN-NOT: call void @__msan_warning
// BOUNDS-NOT: call void @__ubsan_handle_out_of_bounds
+ // BOUNDS-NOT: call void @llvm.trap()
// TSAN-NOT: call void @__tsan_func_entry
// UBSAN-NOT: call void @__ubsan_handle
if (n)
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===================================================================
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -757,6 +757,10 @@
SanOpts.set(SanitizerKind::KernelHWAddress, false);
if (mask & SanitizerKind::KernelHWAddress)
SanOpts.set(SanitizerKind::HWAddress, false);
+ if (mask & SanitizerKind::LocalBounds)
+ Fn->addFnAttr(llvm::Attribute::NoSanitizeBounds);
+ if (mask & SanitizerKind::ArrayBounds)
+ Fn->addFnAttr(llvm::Attribute::NoSanitizeBounds);
// SanitizeCoverage is not handled by SanOpts.
if (Attr->hasCoverage())
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
