From patchwork Mon Aug 7 11:15:03 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 109550 Delivered-To: patch@linaro.org Received: by 10.140.95.78 with SMTP id h72csp1466525qge; Mon, 7 Aug 2017 04:16:35 -0700 (PDT) X-Received: by 10.99.119.130 with SMTP id s124mr129017pgc.281.1502104595607; Mon, 07 Aug 2017 04:16:35 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1502104595; cv=none; d=google.com; s=arc-20160816; b=nAhZ5QhOV5j6HEwCLCvkNTbGL0N3fRGN4NBSvEuzGBO798U1ZkN3kb6MWSOG+HaePv esRoT3TVdaKFyOVBCciueArlUlNWk5zn6bFsAYOVonI8AohIvWluE0+D7QpyNYTiJ0em kAHIZb3hr26JFCAXRmGCYOcezP8eAasBxcnuLptj2xZXX/sX0B5eQE9DGq5udrfRetJt qAKrkUADDs7SJgzLpFUzWAWSZJqCVopqBYPKTMV6KgLQWw3DFzldAXvimG/NOthZmla8 2wN3A4Ba6lK2v5+gfW8u9PF4GxMktT+PEuOkN+6EY4yN8/bmf5y0ecBAMQEtrZuauMBJ shiw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:message-id:date:subject:cc:to:from :arc-authentication-results; bh=vyiQP9DL1ivcYzk4NSwAeM8aogOj7tEMsZ1+UQP7ars=; b=g0l2azOJQIXlfWleIsTXC9l4J+/zF8o+q6ElqRUT+hacDfeFJyP+MMSmRkyUX7ryXz YgCyLdig4vXSbKJssCS/xgmwng0qR9MbRqwdry7k8geNDUbgmuOAP5qxlJqfEnsTgaTk 1q3Y7Ta1LpFdyT4LcX+D3VEezK7hLabMfm6rtSxRiBadmv8jtUU1Kmyhh47UQTqUmpb8 Kp2wROV7eDWeyM6JtP6b8lgFk3Zz49SwP3MRftjlS/9crIc70yXOA6hChDs1XXSEi1Iq pBno1Z1m94j5e/TJliw/hW4y+96yhal7k4pEzL2XOQ+VvIz7bkVcEuzOGVfFpVnVxjnP YsAg== ARC-Authentication-Results: i=1; mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id o26si4505956pge.34.2017.08.07.04.16.35; Mon, 07 Aug 2017 04:16:35 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752935AbdHGLQb (ORCPT + 25 others); Mon, 7 Aug 2017 07:16:31 -0400 Received: from mout.kundenserver.de ([217.72.192.74]:59169 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752786AbdHGLQ2 (ORCPT ); Mon, 7 Aug 2017 07:16:28 -0400 Received: from wuerfel.lan ([78.43.238.10]) by mrelayeu.kundenserver.de (mreue104 [212.227.15.145]) with ESMTPA (Nemesis) id 0MQfKn-1e4CjM2mBJ-00U2lV; Mon, 07 Aug 2017 13:16:17 +0200 From: Arnd Bergmann To: Kees Cook , Andrew Morton , Daniel Micay Cc: Arnd Bergmann , Dan Williams , David Howells , linux-kernel@vger.kernel.org Subject: [PATCH] string.h: work around __builtin_constant_p quirk Date: Mon, 7 Aug 2017 13:15:03 +0200 Message-Id: <20170807111615.4187078-1-arnd@arndb.de> X-Mailer: git-send-email 2.9.0 X-Provags-ID: V03:K0:rkIHnAGb5qSJh8CvfNcmZ6NYw0bAAaVfF2NZdKxGBGk0gt1OFvI BsapPF/YqT8llR2KYa05VdZvV6mq91+WXypzcoaROuxEZxAikO/NhDk/PDReNuvxmpO90qB iU3VgZRuitlg+2aNkyqBeGeGEAdvsSlnv7I/x82OGW0O8QdY7+SAZDtstbRlysARqrFSTQp NiDYv3qz5ZJ78ZWefGy7w== X-UI-Out-Filterresults: notjunk:1; V01:K0:MtwtmVmlUN0=:N7vyIYKtxgnsqlcqHZIdw2 +JCumIySBszHtWSuOjXfIqOFEFwXkgwtw9Vv1Td+kZgAWbKnvqY4U68Evwc+sTYZhCc7AiTN8 nq+7QTJx2o0vEkHkcuj/W4dIqdHd5RNTcvukXIuHVMmlbO+29tez4b7FvqKUD03X0CjtizEJl ReDEM4RuAp/nKtJb5h4tSNYZR7HG4p+mDrmnraIUalcqvGkr6GUHtSIPmuY9VM1X+LTH1sCjl lIyeTZsHe8y4bTyaeFjY2gg0QJFH2Hs/Rt4fs66hxuamSr26xHssW1VwdQ0n/IUTLom/X4z4V DErewU0yWm111EotKvr34NUGKAmW/7tTtvW27a7W47T5biVkfqEok87qfgjYco4lRHXKz0TUc QIW4wvwXNfx3KhOobRWeJ+YvVTrlryb25NbkgA85FF01m1bnIAKnK+btH8m/drgJPAUo8gGTk d1jaIPyKdXhSjmmv/p5Ec5RTIZgxKzPR6G8iB+hb2Dx2CU21D+18WKb8RwVYM85P1kZWw3qEo WkCuoSD56BoCNmSJvifScSkFbJ7Xhs7+Z2wOkONIkrO7XqMrLL9HAy2q4EXjHtUvD1ypGwvFV 9aySsNO9DdGJ9WNEUj2X0J4UKkQYHqv5mRidamwIN1zHuoi9hRnN/BzcEG+LipopyXvzaaQ/4 vn0xDbtpbAWYJPKdJv+acbCjSghQoC1TKlE2clAzAfN1E2yoCrYFqgEjQVSF2oW94KbeuhdGk X717DxcgF37HVL/81aQEsuNLyzE3UrIfvyk0og== Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org The compile-time check in the hardened memcpy() triggered a build error in code that should not have: In function 'memcpy', inlined from '__adfs_dir_put' at fs/adfs/dir_f.c:318:2, inlined from 'adfs_f_update' at fs/adfs/dir_f.c:403:2: include/linux/string.h:305:4: error: call to '__read_overflow2' declared with attribute error: detected read beyond size of object passed as 2nd parameter __read_overflow2(); ^~~~~~~~~~~~~~~~~~ My understanding is that in the __adfs_dir_put() function, the gcc uses a specialization for the common 'thissize = 26', which due to jump threading leads to a case where it has shown that 'thissize' can be constant, but at the same time another case exists where it may have a negative value (for sb->s_blocksize=0) that could lead to overflowing the local 'adfs_direntry de' variable. The bug was hidden before patch "fortify: use WARN instead of BUG for now", which apparently dropped the compile-time checks due to the following code being marked as '__unreachable'. This reworks the hardened string functions to avoid some branches, and introduces a macro for checking whether the argument is a compile-time constant. Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72785 Fixes: mmotm ("fortify: use WARN instead of BUG for now") Fixes: 6974f0c4555e ("include/linux/string.h: add the option of fortified string.h functions") Signed-off-by: Arnd Bergmann --- include/linux/string.h | 62 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 25 deletions(-) -- 2.9.0 diff --git a/include/linux/string.h b/include/linux/string.h index 25f47e07c4c6..3ba29007a942 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -203,10 +203,19 @@ void __read_overflow2(void) __compiletime_error("detected read beyond size of ob void __write_overflow(void) __compiletime_error("detected write beyond size of object passed as 1st parameter"); #if !defined(__NO_FORTIFY) && defined(__OPTIMIZE__) && defined(CONFIG_FORTIFY_SOURCE) + +/* + * a more reliable check for constant arguments, see + * https://gcc.gnu.org/bugzilla/show_bug.cgi?id=72785 + */ +#define __constant_argument(arg) \ + __builtin_choose_expr(__builtin_constant_p(arg), (arg), 0) + __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) + size_t constsize = __constant_argument(size); + if (p_size < constsize) __write_overflow(); if (p_size < size) fortify_overflow(__func__); @@ -257,7 +266,8 @@ __FORTIFY_INLINE size_t strlcpy(char *p, const char *q, size_t size) ret = strlen(q); if (size) { size_t len = (ret >= size) ? size - 1 : ret; - if (__builtin_constant_p(len) && len >= p_size) + size_t constlen = __constant_argument(len); + if (constlen >= p_size) __write_overflow(); if (len >= p_size) fortify_overflow(__func__); @@ -287,7 +297,8 @@ __FORTIFY_INLINE char *strncat(char *p, const char *q, __kernel_size_t count) __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) + size_t constsize = __constant_argument(size); + if (p_size < constsize) __write_overflow(); if (p_size < size) fortify_overflow(__func__); @@ -298,12 +309,11 @@ __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)) { - if (p_size < size) - __write_overflow(); - if (q_size < size) - __read_overflow2(); - } + size_t constsize = __constant_argument(size); + if (p_size < constsize) + __write_overflow(); + if (q_size < constsize) + __read_overflow2(); if (p_size < size || q_size < size) fortify_overflow(__func__); return __builtin_memcpy(p, q, size); @@ -313,12 +323,11 @@ __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)) { - if (p_size < size) - __write_overflow(); - if (q_size < size) - __read_overflow2(); - } + size_t constsize = __constant_argument(size); + if (p_size < constsize) + __write_overflow(); + if (q_size < constsize) + __read_overflow2(); if (p_size < size || q_size < size) fortify_overflow(__func__); return __builtin_memmove(p, q, size); @@ -328,7 +337,8 @@ 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) + size_t constsize = __constant_argument(size); + if (p_size < constsize) __read_overflow(); if (p_size < size) fortify_overflow(__func__); @@ -339,12 +349,11 @@ __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)) { - if (p_size < size) - __read_overflow(); - if (q_size < size) - __read_overflow2(); - } + size_t constsize = __constant_argument(size); + if (p_size < constsize) + __read_overflow(); + if (q_size < constsize) + __read_overflow2(); if (p_size < size || q_size < size) fortify_overflow(__func__); return __builtin_memcmp(p, q, size); @@ -353,7 +362,8 @@ __FORTIFY_INLINE int memcmp(const void *p, const void *q, __kernel_size_t 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) + size_t constsize = __constant_argument(size); + if (p_size < constsize) __read_overflow(); if (p_size < size) fortify_overflow(__func__); @@ -364,7 +374,8 @@ 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) + size_t constsize = __constant_argument(size); + if (p_size < constsize) __read_overflow(); if (p_size < size) fortify_overflow(__func__); @@ -375,7 +386,8 @@ extern void *__real_kmemdup(const void *src, size_t len, gfp_t gfp) __RENAME(kme __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) + size_t constsize = __constant_argument(size); + if (p_size < constsize) __read_overflow(); if (p_size < size) fortify_overflow(__func__);