From patchwork Thu May 4 14:24:35 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Daniel Micay X-Patchwork-Id: 9712077 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 843EA60362 for ; Thu, 4 May 2017 14:25:59 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 717692041F for ; Thu, 4 May 2017 14:25:59 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 6464828601; Thu, 4 May 2017 14:25:59 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.1 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_MED, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from mother.openwall.net (mother.openwall.net [195.42.179.200]) by mail.wl.linuxfoundation.org (Postfix) with SMTP id D15AC2041F for ; Thu, 4 May 2017 14:25:57 +0000 (UTC) Received: (qmail 3714 invoked by uid 550); 4 May 2017 14:25:55 -0000 Mailing-List: contact kernel-hardening-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Delivered-To: mailing list kernel-hardening@lists.openwall.com Received: (qmail 3646 invoked from network); 4 May 2017 14:25:52 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id; bh=gDWr4tt6i61yK1gHg7PkRhBUPJTKq7T3TICPf/VewGg=; b=MBb+NvGFWsd0RzY7JlAJLda3kfntXq9kbA1DNJiUrraJxjImQIqw/RCY5+5YRdpWSH bU98fGQL9Lw+gvMBwuboR7QWV3w71h9VvJd+f8JsAEyMllwfD/wYxCMXeDDywMMqhZIW DVoxd90Ys7f1CH+qjpEIWWr5Q5nwr5WC83OtsC/WisvUt/BBcQJCmqYZxd96LadCBGu9 vSJBr3dhTr93atXCAItbJZEK3OIndC0uyRwT5xAufDgOon7voPvRuch6Rgqvy/IoUagc 5x8pgTb38S3yY3CuUbOtlcDl0H9SkN9sEnnkyuzm2PqYZY7wY2JouYyGzu2CJnrkKVoe oHsA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=gDWr4tt6i61yK1gHg7PkRhBUPJTKq7T3TICPf/VewGg=; b=SqY8exzHywuuyif5f4iFHyjRHLvh7fzvss92w7aSvUEU/eDwVxIxCNGXAY/4+SgAkM 2h2Pv5MADS51ioOj03RkGgryjoBMGUhRx1OEu78eRAPgpwv89ICvyNHmLNLDM/LSQI8j nk1eA7WErEJ2rfTge1zn6TBZN70USZkF60JE1mDHhtDsxz5CVbS21QQaDIOLNvtKFrrq FGkSe7Gyz75xhF+qxkwsfoeSU6yz1phWcJ+vVU61XHl0j9LblwIH8qOWTaCKZtAI7yCY GKC/nA2uB9yJWEW/43/NX3iaLsgBCfZia+N3QrDO4Y1OoVQfo8XYq4Wd4I3MGymht9y+ dhtw== X-Gm-Message-State: AN3rC/4JQeIrBqT12hlarqlBSutt8JMYoiZLAy3bDuiPcU0okRB9rfBO 6+TfMB/A5/3VWw== X-Received: by 10.107.17.220 with SMTP id 89mr38631461ior.177.1493907940585; Thu, 04 May 2017 07:25:40 -0700 (PDT) From: Daniel Micay To: Kees Cook , kernel-hardening@lists.openwall.com Cc: Daniel Micay Date: Thu, 4 May 2017 10:24:35 -0400 Message-Id: <20170504142435.10175-1-danielmicay@gmail.com> X-Mailer: git-send-email 2.12.2 Subject: [kernel-hardening] [PATCH] add the option of fortified string.h functions X-Virus-Scanned: ClamAV using ClamSMTP This adds support for compiling with a rough equivalent to the glibc _FORTIFY_SOURCE=1 feature, providing compile-time and runtime buffer overflow checks for string.h functions when the compiler determines the size of the source or destination buffer at compile-time. Unlike glibc, it covers buffer reads in addition to writes. GNU C __builtin_*_chk intrinsics are avoided because they're only designed to detect write overflows and are overly complex. A single inline branch works for everything but strncat while those intrinsics would force the creation of a bunch of extra non-inline wrappers that aren't able to receive the detected source buffer size. This detects various undefined uses of memcpy, etc. at compile-time outside of non-core kernel code, and in the arm64 vdso code, so there will need to be a series of fixes applied (mainly memcpy -> strncpy for copying string constants to avoid copying past the end of the source). It isn't particularly bad, but there are likely some issues that occur during regular use at runtime (none found so far). Future improvements left out of initial implementation for simplicity, as it's all quite optional and can be done incrementally: The fortified string functions should place a limit on reads from the source. For strcat/strcpy, these could just be a variant of strlcat / strlcpy using the size limit as a bound on both the source and destination, with the return value only reporting whether truncation occurred rather than providing the source length. It would be an easier API for developers to use too and not that it really matters but it would be more efficient for the case where truncation is intended. It should be possible to optionally use __builtin_object_size(x, 1) for some functions (C strings) to detect intra-object overflows (like glibc's _FORTIFY_SOURCE=2), but for now this takes the conservative approach to avoid likely compatibility issues. The error reporting could be made friendlier by splitting up the compile-time error for reads and writes. The runtime error could also directly report the buffer and copy sizes. It may make sense to have the compile-time checks in a separate configuration option since they wouldn't have a runtime performance cost if there was an ifdef for the runtime check. However, the runtime cost of these checks is already quite low (much lower than SSP) and as long as some people are using it with allyesconfig, the issues will be found for any in-tree code. Signed-off-by: Daniel Micay --- arch/x86/boot/compressed/misc.c | 5 ++ include/linux/string.h | 161 ++++++++++++++++++++++++++++++++++++++++ lib/string.c | 8 ++ security/Kconfig | 6 ++ 4 files changed, 180 insertions(+) diff --git a/arch/x86/boot/compressed/misc.c b/arch/x86/boot/compressed/misc.c index b3c5a5f030ce..43691238a21d 100644 --- a/arch/x86/boot/compressed/misc.c +++ b/arch/x86/boot/compressed/misc.c @@ -409,3 +409,8 @@ asmlinkage __visible void *extract_kernel(void *rmode, memptr heap, debug_putstr("done.\nBooting the kernel.\n"); return output; } + +void fortify_panic(const char *name) +{ + error("detected buffer overflow"); +} diff --git a/include/linux/string.h b/include/linux/string.h index 26b6f6a66f83..3bd429c9593a 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -169,4 +169,165 @@ static inline const char *kbasename(const char *path) return tail ? tail + 1 : path; } +#define __FORTIFY_INLINE extern __always_inline __attribute__((gnu_inline)) +#define __RENAME(x) __asm__(#x) + +void fortify_panic(const char *name) __noreturn __cold; +void __buffer_overflow(void) __compiletime_error("buffer overflow"); + +#if !defined(__NO_FORTIFY) && defined(__OPTIMIZE__) && defined(CONFIG_FORTIFY_SOURCE) +__FORTIFY_INLINE char *strcpy(char *p, const char *q) +{ + size_t p_size = __builtin_object_size(p, 0); + if (p_size == (size_t)-1) + return __builtin_strcpy(p, q); + if (strlcpy(p, q, p_size) >= p_size) + fortify_panic(__func__); + return p; +} + +__FORTIFY_INLINE char *strncpy(char *p, const char *q, __kernel_size_t size) +{ + size_t p_size = __builtin_object_size(p, 0); + if (__builtin_constant_p(size) && p_size < size) + __buffer_overflow(); + if (p_size < size) + fortify_panic(__func__); + return __builtin_strncpy(p, q, size); +} + +__FORTIFY_INLINE char *strcat(char *p, const char *q) +{ + size_t p_size = __builtin_object_size(p, 0); + if (p_size == (size_t)-1) + return __builtin_strcat(p, q); + if (strlcat(p, q, p_size) >= p_size) + fortify_panic(__func__); + return p; +} + +__FORTIFY_INLINE char *strncat(char *p, const char *q, __kernel_size_t count) +{ + size_t p_len, copy_len; + size_t p_size = __builtin_object_size(p, 0); + if (p_size == (size_t)-1) + return __builtin_strncat(p, q, count); + p_len = __builtin_strlen(p); + copy_len = strnlen(q, count); + if (p_size < p_len + copy_len + 1) + fortify_panic(__func__); + __builtin_memcpy(p + p_len, q, copy_len); + p[p_len + copy_len] = '\0'; + return p; +} + +__FORTIFY_INLINE __kernel_size_t strlen(const char *p) +{ + __kernel_size_t ret; + size_t p_size = __builtin_object_size(p, 0); + if (p_size == (size_t)-1) + return __builtin_strlen(p); + ret = strnlen(p, p_size); + if (p_size <= ret) + fortify_panic(__func__); + return ret; +} + +extern __kernel_size_t __real_strnlen(const char *, __kernel_size_t) __RENAME(strnlen); +__FORTIFY_INLINE __kernel_size_t strnlen(const char *p, __kernel_size_t maxlen) +{ + size_t p_size = __builtin_object_size(p, 0); + __kernel_size_t ret = __real_strnlen(p, maxlen < p_size ? maxlen : p_size); + if (p_size <= ret) + fortify_panic(__func__); + return ret; +} + +__FORTIFY_INLINE void *memset(void *p, int c, __kernel_size_t size) +{ + size_t p_size = __builtin_object_size(p, 0); + if (__builtin_constant_p(size) && p_size < size) + __buffer_overflow(); + if (p_size < size) + fortify_panic(__func__); + return __builtin_memset(p, c, size); +} + +__FORTIFY_INLINE void *memcpy(void *p, const void *q, __kernel_size_t size) +{ + size_t p_size = __builtin_object_size(p, 0); + size_t q_size = __builtin_object_size(q, 0); + if (__builtin_constant_p(size) && (p_size < size || q_size < size)) + __buffer_overflow(); + if (p_size < size || q_size < size) + fortify_panic(__func__); + return __builtin_memcpy(p, q, size); +} + +__FORTIFY_INLINE void *memmove(void *p, const void *q, __kernel_size_t size) +{ + size_t p_size = __builtin_object_size(p, 0); + size_t q_size = __builtin_object_size(q, 0); + if (__builtin_constant_p(size) && (p_size < size || q_size < size)) + __buffer_overflow(); + if (p_size < size || q_size < size) + fortify_panic(__func__); + return __builtin_memmove(p, q, size); +} + +extern void *__real_memscan(void *, int, __kernel_size_t) __RENAME(memscan); +__FORTIFY_INLINE void *memscan(void *p, int c, __kernel_size_t size) +{ + size_t p_size = __builtin_object_size(p, 0); + if (__builtin_constant_p(size) && p_size < size) + __buffer_overflow(); + if (p_size < size) + fortify_panic(__func__); + return __real_memscan(p, c, size); +} + +__FORTIFY_INLINE int memcmp(const void *p, const void *q, __kernel_size_t size) +{ + size_t p_size = __builtin_object_size(p, 0); + size_t q_size = __builtin_object_size(q, 0); + if (__builtin_constant_p(size) && (p_size < size || q_size < size)) + __buffer_overflow(); + if (p_size < size || q_size < size) + fortify_panic(__func__); + return __builtin_memcmp(p, q, size); +} + +__FORTIFY_INLINE void *memchr(const void *p, int c, __kernel_size_t size) +{ + size_t p_size = __builtin_object_size(p, 0); + if (__builtin_constant_p(size) && p_size < size) + __buffer_overflow(); + if (p_size < size) + fortify_panic(__func__); + return __builtin_memchr(p, c, size); +} + +void *__real_memchr_inv(const void *s, int c, size_t n) __RENAME(memchr_inv); +__FORTIFY_INLINE void *memchr_inv(const void *p, int c, size_t size) +{ + size_t p_size = __builtin_object_size(p, 0); + if (__builtin_constant_p(size) && p_size < size) + __buffer_overflow(); + if (p_size < size) + fortify_panic(__func__); + return __real_memchr_inv(p, c, size); +} + +extern void *__real_kmemdup(const void *src, size_t len, gfp_t gfp) __RENAME(kmemdup); +__FORTIFY_INLINE void *kmemdup(const void *p, size_t size, gfp_t gfp) +{ + size_t p_size = __builtin_object_size(p, 0); + if (__builtin_constant_p(size) && p_size < size) + __buffer_overflow(); + if (p_size < size) + fortify_panic(__func__); + return __real_kmemdup(p, size, gfp); +} +#endif + #endif /* _LINUX_STRING_H_ */ diff --git a/lib/string.c b/lib/string.c index b5c9a1168d3a..ca356e537e24 100644 --- a/lib/string.c +++ b/lib/string.c @@ -19,6 +19,8 @@ * - Kissed strtok() goodbye */ +#define __NO_FORTIFY + #include #include #include @@ -952,3 +954,9 @@ char *strreplace(char *s, char old, char new) return s; } EXPORT_SYMBOL(strreplace); + +void fortify_panic(const char *name) +{ + panic("detected buffer overflow in %s", name); +} +EXPORT_SYMBOL(fortify_panic); diff --git a/security/Kconfig b/security/Kconfig index 93027fdf47d1..0e5035d720ce 100644 --- a/security/Kconfig +++ b/security/Kconfig @@ -154,6 +154,12 @@ config HARDENED_USERCOPY_PAGESPAN been removed. This config is intended to be used only while trying to find such users. +config FORTIFY_SOURCE + bool "Harden common functions against buffer overflows" + help + Detect overflows of buffers in common functions where the compiler + can determine the buffer size. + config STATIC_USERMODEHELPER bool "Force all usermode helper calls through a single binary" help