[PATCH] D119816: Fix not working attribute no_sanitize bounds that affects linux kernel

Tue Feb 15 01:14:04 PST 2022

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D119816.408742.patch
Type: text/x-patch
Size: 5054 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20220215/2d340357/attachment.bin>