From patchwork Thu Dec 21 18:59:14 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Adhemerval Zanella Netto X-Patchwork-Id: 757026 Delivered-To: patch@linaro.org Received: by 2002:a5d:67c6:0:b0:336:6142:bf13 with SMTP id n6csp1053791wrw; Thu, 21 Dec 2023 10:59:51 -0800 (PST) X-Google-Smtp-Source: AGHT+IGDNb51QRUoeA+4u1J2b9LG2T7VJowQUe7nzUR3iMwUi7JAGRgal/30ye+7mtj5eaOAyzBN X-Received: by 2002:a05:620a:1a8d:b0:77f:948:8a09 with SMTP id bl13-20020a05620a1a8d00b0077f09488a09mr283495qkb.114.1703185190901; Thu, 21 Dec 2023 10:59:50 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1703185190; cv=pass; d=google.com; s=arc-20160816; b=NH2Xqyy4uofXXd6ghNtflY7Vt/NpAkZxu/dNu6Gx7K1sEbottwI/bqgBIYK8bBm4OS /oxmgY468D6Sq2jVr2BYW3ieO8oXYE+KPT9mYI1vlOUayubv5W658JFZu+dVQ7lTlycq SMa/XpBnAHHK0+cPQe8cJuuZm1N6v3w3STkIlYDtfP/l+rRYMVRvlW9itLJS72Gc/AIH f6gKn+0zxsq0E7uci7seiEQjhHWQTbkkCT4Fl27jUj7hvcdVfUCpo6vdn1aB2CRy0Eev KIulb6E2z/4eKSjf+zHThCQ5hnDgGGDahNTn50ljDVfotXGIZg4d/wNtq64vlKz/00am 8Kig== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=errors-to:list-subscribe:list-help:list-post:list-archive :list-unsubscribe:list-id:precedence:content-transfer-encoding :mime-version:message-id:date:subject:to:from:dkim-signature :arc-filter:dmarc-filter:delivered-to; bh=CMtChY6AD4egiDfYR9dcKWO+9kmiI8DmrCIMapsfjvU=; fh=JNN3k7BRNI1OnWdIBK9jlpNeitGd8uBm02dHI75AGcg=; b=yKbPNg9gCAGtWsPGE+u4fyLYPZZvJbeKSDvHW0oXD41M9ePKJ4Zd4i53soWJIpbohb 1BMGZgcI+rytP7Pl1GxLzW+nM/ttgr3zLJzWloLniAtx41Ngn9Eb7VaNihXHrz/tGFDa dafVXkFIM8+sfm6NCYDPO+pOOpiY9IQ8rPEth952ji7afXxJqF6w1Rt9d24N7jUlI4qf 31YZ/n/VGzfRvKrY8EqawWpPmhT57FmJU4jN+05plOlWyrV+kYDNL9V8aur3i1L1EU0z cIVi2a+uq7LM5SbQPdCpiafIRTrD8p7T5OrC3J9KR4hWNS5Z8RL8BkKpRPGg1t9AuDP2 LIRw== ARC-Authentication-Results: i=2; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=SfFEf1Jt; arc=pass (i=1); spf=pass (google.com: domain of libc-alpha-bounces+patch=linaro.org@sourceware.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="libc-alpha-bounces+patch=linaro.org@sourceware.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from server2.sourceware.org (server2.sourceware.org. [8.43.85.97]) by mx.google.com with ESMTPS id ay43-20020a05620a17ab00b0077d59d88b25si2804580qkb.273.2023.12.21.10.59.50 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 21 Dec 2023 10:59:50 -0800 (PST) Received-SPF: pass (google.com: domain of libc-alpha-bounces+patch=linaro.org@sourceware.org designates 8.43.85.97 as permitted sender) client-ip=8.43.85.97; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=SfFEf1Jt; arc=pass (i=1); spf=pass (google.com: domain of libc-alpha-bounces+patch=linaro.org@sourceware.org designates 8.43.85.97 as permitted sender) smtp.mailfrom="libc-alpha-bounces+patch=linaro.org@sourceware.org"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 84D39386189D for ; Thu, 21 Dec 2023 18:59:50 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from mail-oo1-xc33.google.com (mail-oo1-xc33.google.com [IPv6:2607:f8b0:4864:20::c33]) by sourceware.org (Postfix) with ESMTPS id D75573858030 for ; Thu, 21 Dec 2023 18:59:43 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D75573858030 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org D75573858030 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::c33 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1703185186; cv=none; b=Uxh07ECXVCKKP3st8vK2iLp2HBFgjn+uqPFWnVshmSatoyCaP/bPjM2Ll7/0I3e2cUphoxoa96CR0ubtB/WoaGE4m7zm529BlnpKAhZRNbF1HoaUyKXouflTKPhFOu9bFKcPt4CNgJwvINdP2JAEar++c7xXn+3Oo9+GIV1ih10= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1703185186; c=relaxed/simple; bh=KuXr4zN61nvoLk5ser8RXGT0CS/TmJmV6A4IIcC76p4=; h=DKIM-Signature:From:To:Subject:Date:Message-Id:MIME-Version; b=U/wZi99jN6/Cf9zGGN9GqglPdOkVwdxPwmg6FxzoSDWqi5kr06BCtBes8Hxpf2Nf7uv4LbXNpboAFhFZguLEP3QfIClbkl/Vw94V78K+mzFthgVUoEpkYnNSdmf7YroKSJSfitVe9MUhd/+jPygP7U8TOMxAkPVvzqWiApCu/to= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-oo1-xc33.google.com with SMTP id 006d021491bc7-5941782f8a3so674218eaf.2 for ; Thu, 21 Dec 2023 10:59:43 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1703185182; x=1703789982; darn=sourceware.org; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:from:to:cc:subject:date:message-id:reply-to; bh=CMtChY6AD4egiDfYR9dcKWO+9kmiI8DmrCIMapsfjvU=; b=SfFEf1JtHDEYBFjESsFD37yPgzfOOuJza/wplUcRIhMBaZiSE9OskehDSPAEClanIM 9QMpYtIjXGNNvM9xw0XP7uvga02nTyo3+0SCmWFbYliBQxeySGsdeR/y346bRNUqhRKs LYVx5Dhikj3oJddTQqDzLTDzUENWxZ8KK8f51gBH54MG1J0K/B2j3LoSfHLrrrgb1QGS ZN0F5AHnD1jwB423BMBfh2pSn0FevQDxCijdqBJnz4CxWyr2fi9f/nZ0BGofY1fHPtjJ bd8TPslXir19i6KsIXG4e3dqs1hCB3sMXRBrh50zHuBaG5hUXC0BZe0559CoIiznDeAy kWKg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1703185182; x=1703789982; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=CMtChY6AD4egiDfYR9dcKWO+9kmiI8DmrCIMapsfjvU=; b=aettnrq3aFuY58GVtLbca2VlqqsEb21lFHuN5VFk5v3JE4jiltDQD0ZUN2Qm3rlFKZ H5ASOllLZ94Vc8xASdU3BMO/nnli5GzHR7FIwYrWKrX2poQZZn0CcQpW/e2OILzcP++u Qj/OOrQEB0F3iR0S9CV++P0kB5z5iGJy+OdeeQ75WRHfLAihrS8Bmk/KLzk/WIO0omY8 sNbJ9yNySwrzSvBbMlAkIsMdlRWEjSUBXK7fbgrQSvnxW4l11hf6Qu6Aii/ajIbAomL1 0+Wjd5K0bwfBoCQpcKvkNr6ZTUW1SnDBC8mrhHQiD8IBLJ/La07CQPLjV7m50HkiGkx5 7SJw== X-Gm-Message-State: AOJu0YyxW+3Pw4fDWRcSiACmuwhKA1Og+O/9aFKpve2Th202V9Q3zLCp PXnavOle8J1rSny5IxNxmT7qD7vNom0+af/pH6N4GzFh0Ws= X-Received: by 2002:a05:6358:9faa:b0:170:b0fe:13a2 with SMTP id fy42-20020a0563589faa00b00170b0fe13a2mr235930rwb.17.1703185182241; Thu, 21 Dec 2023 10:59:42 -0800 (PST) Received: from mandiga.. ([2804:1b3:a7c0:8192:ecd7:d327:bea0:14dc]) by smtp.gmail.com with ESMTPSA id a9-20020a63e409000000b005cdbebd61d8sm1946165pgi.9.2023.12.21.10.59.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 21 Dec 2023 10:59:41 -0800 (PST) From: Adhemerval Zanella To: libc-alpha@sourceware.org, Siddhesh Poyarekar Subject: [PATCH 00/15] Improve fortify support with clang Date: Thu, 21 Dec 2023 15:59:14 -0300 Message-Id: <20231221185929.1307116-1-adhemerval.zanella@linaro.org> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 X-Spam-Status: No, score=-5.4 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces+patch=linaro.org@sourceware.org When using clang, the fortify wrappers show less coverage on both compile and runtime. For instance, a snippet from tst-fortify.c: $ cat t.c #include #include const char *str1 = "JIHGFEDCBA"; int main (int argc, char *argv[]) { #define O 0 struct A { char buf1[9]; char buf2[1]; } a; strcpy (a.buf1 + (O + 4), str1 + 5); return 0; } $ gcc -O2 t.c -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=2 -o t $ ./t *** buffer overflow detected ***: terminated Aborted (core dumped) $ clang -O2 t.c -Wp,-U_FORTIFY_SOURCE,-D_FORTIFY_SOURCE=2 -o t $ ./t $ Although clang does support __builtin___strcpy_chk (which correctly lowers to __strcpy_chk), the __builtin_object_size passed as third the argument is not fully correct and thus limits the possible runtime checks. This limitation was already being raised some years ago [1], but the work has been staled. However, a similar patch has been used for ChromeOS for some time [2]. Bionic libc also uses a similar approach to enable fortified wrappers. Improve its support with clang, requires defining the fortified wrapper differently. For instance, the read wrapper is currently expanded as: extern __inline __attribute__((__always_inline__)) __attribute__((__artificial__)) __attribute__((__warn_unused_result__)) ssize_t read (int __fd, void *__buf, size_t __nbytes) { return __glibc_safe_or_unknown_len (__nbytes, sizeof (char), __glibc_objsize0 (__buf)) ? __read_alias (__fd, __buf, __nbytes) : __glibc_unsafe_len (__nbytes, sizeof (char), __glibc_objsize0 (__buf)) ? __read_chk_warn (__fd, __buf, __nbytes, __builtin_object_size (__buf, 0)) : __read_chk (__fd, __buf, __nbytes, __builtin_object_size (__buf, 0)); } The wrapper relies on __builtin_object_size call lowers to a constant at Compile time and many other operations in the wrapper depend on having a single, known value for parameters. Because this is impossible to have for function parameters, the wrapper depends heavily on inlining to work and While this is an entirely viable approach on GCC is not fully reliable on clang. This is because by the time llvm gets to inlining and optimizing, there is a minimal reliable source and type-level information available (more information on a more deep explanation on how to fortify wrapper works on clang [4]). To allow the wrapper to work reliably and with the same functionality as with GCC, clang requires a different approach: * __attribute__((diagnose_if(c, “str”, “warning”))) which is a * function level attribute; if the compiler can determine that 'c' is true at compile-time, it will emit a warning with the text 'str1'. If it would be better to emit an error, the wrapper can use "error" instead of "warning". * __attribute__((overloadable)) which is also a function-level attribute; and it allows C++-style overloading to occur on C functions. * __attribute__((pass_object_size(n))) which is a parameter-level attribute; and it makes the compiler evaluate __builtin_object_size(param, n) at each call site of the function that has the parameter and passes it in as a hidden parameter. This attribute has two side effects that are key to how FORTIFY works: 1. It can overload solely on pass_object_size (e.g. there are two overloads of foo in void foo(char * __attribute__((pass_object_size(0))) c); void foo(char *); (The one with pass_object_size attribute has preceded the default one). 2. A function with at least one pass_object_size parameter can never have its address taken (and overload resolution respects this). Thus the read wrapper can be implemented as follows, without hindering any fortify coverage compile and runtime: Thus the read wrapper can be implemented as follows, without hindering any fortify coverage compile and runtime: extern __inline __attribute__((__always_inline__)) __attribute__((__artificial__)) __attribute__((__overloadable__)) __attribute__((__warn_unused_result__)) ssize_t read (int __fd, void *const __attribute__((pass_object_size (0))) __buf, size_t __nbytes) __attribute__((__diagnose_if__ ((((__builtin_object_size (__buf, 0)) != -1ULL && (__nbytes) > (__builtin_object_size (__buf, 0)) / (1))), "read called with bigger length than the size of the destination buffer", "warning"))) { return (__builtin_object_size (__buf, 0) == (size_t) -1) ? __read_alias (__fd, __buf, __nbytes) : __read_chk (__fd, __buf, __nbytes, __builtin_object_size (__buf, 0)); } To avoid changing the current semantic for GCC, a set of macros is defined to enable the clang required attributes, along with some changes on internal macros to avoid the need to issue the symbol_chk symbols (which are done through the __diagnose_if__ attribute for clang). The read wrapper can be simplified as: __fortify_function __attribute_overloadable__ __wur ssize_t read (int __fd, __fortify_clang_overload_arg0 (void *, ,__buf), size_t __nbytes) __fortify_clang_warning_only_if_bos0_lt (__nbytes, __buf, "read called with bigger length than " "size of the destination buffer") { return __glibc_fortify (read, __nbytes, sizeof (char), __glibc_objsize0 (__buf), __fd, __buf, __nbytes); } There is no expected semantic or code change when using GCC. Also, clang does not support __va_arg_pack, so variadic functions are expanded to call va_arg implementations. The error function must not have bodies (address takes are expanded to nonfortified calls), and with the __fortify_function compiler might still create a body with the C++ mangling name (due to the overload attribute). In this case, the function is defined with __fortify_function_error_function macro instead. To fully test it, I used my clang branch [4] which allowed me to fully build all fortify tests with clang. With this patchset, there is no regressions anymore. [1] https://sourceware.org/legacy-ml/libc-alpha/2017-09/msg00434.html [2] https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/refs/heads/main/sys-libs/glibc/files/local/glibc-2.35/0006-glibc-add-clang-style-FORTIFY.patch [3] https://docs.google.com/document/d/1DFfZDICTbL7RqS74wJVIJ-YnjQOj1SaoqfhbgddFYSM/edit [4] https://sourceware.org/git/?p=glibc.git;a=shortlog;h=refs/heads/azanella/clang Adhemerval Zanella (15): debug: Adapt fortify tests to libsupport debug: Increase tst-fortify checks for compiler without __va_arg_pack support debug: Add fortify dprintf tests debug: Add fortify syslog tests debug: Add fortify wprintf tests cdefs.h: Add clang fortify directives libio: Improve fortify with clang string: Improve fortify with clang stdlib: Improve fortify with clang unistd: Improve fortify with clang socket: Improve fortify with clang syslog: Improve fortify with clang wcsmbs: Improve fortify with clang debug: Improve fcntl.h fortify warnings with clang debug: Improve mqueue.h fortify warnings with clang debug/Makefile | 7 ++ debug/test-stpcpy_chk.c | 2 +- debug/test-strcpy_chk.c | 2 +- debug/tst-fortify-syslog.c | 128 +++++++++++++++++++++ debug/tst-fortify-wide.c | 104 +++++++++++++++++ debug/tst-fortify.c | 83 +++++++++----- debug/tst-longjmp_chk.c | 8 +- debug/tst-longjmp_chk2.c | 6 +- debug/tst-longjmp_chk3.c | 6 +- io/bits/fcntl2.h | 92 +++++++++++++++ io/bits/poll2.h | 29 +++-- io/fcntl.h | 3 +- libio/bits/stdio2.h | 173 ++++++++++++++++++++++++---- misc/bits/syslog.h | 14 ++- misc/sys/cdefs.h | 201 ++++++++++++++++++++++++++++----- posix/bits/unistd.h | 110 +++++++++++++----- rt/bits/mqueue2.h | 29 +++++ rt/mqueue.h | 3 +- socket/bits/socket2.h | 20 +++- stdlib/bits/stdlib.h | 40 +++++-- string/bits/string_fortified.h | 57 ++++++---- wcsmbs/bits/wchar2.h | 167 +++++++++++++++++++-------- 22 files changed, 1064 insertions(+), 220 deletions(-) create mode 100644 debug/tst-fortify-syslog.c create mode 100644 debug/tst-fortify-wide.c