From patchwork Mon Jul 31 09:52:15 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 108987 Delivered-To: patch@linaro.org Received: by 10.140.101.6 with SMTP id t6csp27192qge; Mon, 31 Jul 2017 02:52:50 -0700 (PDT) X-Received: by 10.84.232.8 with SMTP id h8mr16236187plk.252.1501494770234; Mon, 31 Jul 2017 02:52:50 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1501494770; cv=none; d=google.com; s=arc-20160816; b=Gk84TtI+NftqbsnduVAybskHFQpPeJ7/N6ROsfYxLbDqeC6xF8vks9VJgzJmjBr4iV mGY4k9P4cD0XZJo1F9dangnVN810xk/NU+5Guv91oVq4Gy6CgCnLamWAZpTY0zEfOsg8 g/CO0rQzQCmgTqXlPF/pNeBcyeZh/K0RwBZPmcPiBomk9IqCBeNLe46rHRxSsQ8SE3Mf xPQ1/f0QmEU549ljka+T7BixFHH9P7Bgc0BAH+KkQYZA/WD6gutD+RdAcf7F5OoGKBuh +WuX11MVrp+Q0bsqvsI6KgUXeteU52Ole6cJpyRtgX6sDZIHKfY5H3QVMLq4FFjQgsyM 6AtQ== 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=aEzuUocjYrZI7WJofI/ItfV6Bm971b9+/e0WVfMX5cY=; b=u4HcnTIV4fMBrdd+mF0JS51LTf5kHBx1MGjibh1k048M+OmVGJ/FfWY6zxZGt1g9Td TVVhxYHzYJ4ODRyqzcOJuTKOc6rCsINF4iSbmSasbewd7SgUnE3R3FHICTgWqPrdrY27 wsi7rDEZprGmGaIjHjcnOPbCfDTFWWe/KhWaTLFgZKz+vXTd8a2aDYUfJsqD572J+KTt ZIvBIngtTDb8FebQc1gpalK84M77/OjeMSUREX60EgLFJbTVWgdJuj5aCJx/GYNCVAJS UgEjwPVy5rl4/HXS/zVvVkGWRQTkB1Cny8H54iuwndBHC2L8YRHVsXEfgSksCxuK6UGP KgSA== 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 u194si10111805pgc.468.2017.07.31.02.52.49; Mon, 31 Jul 2017 02:52:50 -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 S1751958AbdGaJwq (ORCPT + 26 others); Mon, 31 Jul 2017 05:52:46 -0400 Received: from mout.kundenserver.de ([212.227.126.135]:54579 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750923AbdGaJwo (ORCPT ); Mon, 31 Jul 2017 05:52:44 -0400 Received: from wuerfel.lan ([78.43.238.10]) by mrelayeu.kundenserver.de (mreue005 [212.227.15.129]) with ESMTPA (Nemesis) id 0LmgTP-1eAu5s0pVr-00aD0t; Mon, 31 Jul 2017 11:52:31 +0200 From: Arnd Bergmann To: Alexander Viro Cc: Andrey Ryabinin , Arnd Bergmann , Ingo Molnar , Andrew Morton , Kees Cook , Frederic Weisbecker , Rik van Riel , Denys Vlasenko , Dmitry Safonov , linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH] [RFC] fs/binfmt_elf: work around bogus ubsan array-bounds warning Date: Mon, 31 Jul 2017 11:52:15 +0200 Message-Id: <20170731095228.461151-1-arnd@arndb.de> X-Mailer: git-send-email 2.9.0 X-Provags-ID: V03:K0:aTNM8Nz2tZFHGhxLmrmPjCDht8pBT/V7KUsqWzJTnnRO77LK0ZX zeE5E1QHa1c5nVYvzTTvXg67PdUj6CaMCYwo206+3y91pn1+on/qUpFqxVyXgWAwEbpoml7 6MzoqcmuaxzknyA+1+FHEa7v9mjYRBSoeT5wnKerHHMJgGHmX3s7uESJJJmubQK5fedRnSs L0KL51Twr57b4Jti54j3g== X-UI-Out-Filterresults: notjunk:1; V01:K0:u+PlVhNwwWY=:sdCTyaVkDNE1nR6qLLr6i8 4HhBrhWE2/HaaNiz5H6fEW7y8hOotqYuhD4PBrxUOp8DEext9TiD/kXrXJY49TQUE/4brxH3G i5Rtf86Plg1KhMCxewaqWCzpnUU7MWfB+bcJZSLjrYNF0V2zZayIcktX1weG+wGvb/k0L5bjJ DBl5JQLACoNeWa3n353NZKk4ZRH9hgBUAVl/n3lolLbIK+GdvcuGWOgugpYMxAAWMkQuE6C7f IeA4FSvuL6F9Kr+4RbROb2mVsZDGqYW8IX8+34qEHgxWymvq7V0HMulMOyB+QtQdhrOy7YRtj TjcCo6sq72kdN5LBOv6UxwUhf+LO4XmE0QZMPjTEpRiTmW76LFqhJYbq0X60jGoiXtCp8WVos M31EbDcgLAGZw4OQBaQ97YCOYj3mChkeHuc8zoFEamLBK2rJWn3AAa88fgQWrGM5rRYGtBewJ w+SIzpExtv98Klq1Izw59JKnpC4Q7NnL9a/15KbIywv0LIbwSzppw5hW9o71r78vP11Tvq2H9 jotVrxaENAoF5jFuMBFseSwjZj+xTpwjnQYpKJ/vq8Gr3q41WQVWfh7iu+mmWTIsbjN6KRvgu 8jOW5cBl7xl6+PblKdzf+yGZLKRZfQWymG1shJq7c8MFHJjwtFxqtz2W3gycY9YdhKYqSznVX 1yvAad1KjbY7bC/HJWlAJkxPSr1duXJBkar2A718hwx906QHT6Ju3u0MkeNMIb9muLL0OpQ1W UwHdEjqbT3IBowjUw2LCCOlGZGFKcliM0Eoiiw== Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org I don't really understand this warning, but it does seem to get triggered by the unusual pointer notation used in the x86_64 __copy_to_user() optimization. Here, the source buffer for __copy_to_user() is the constant expression '("x86_64")', which is seven bytes long and does not trigger the optimized fast-path that handles buffers of 1, 2, 4, 8, 10 and 16 bytes. I suspect this is another case of __builtin_constant_p() not doing exactly what we expect it to do: The compiler first decides that it knows the string length of the constant expression, but then it does not eliminate the 'case 10' and 'case 16' portions of the switch() statement before checking for a possible buffer overflow. The warning we now get is: In file included from include/linux/uaccess.h:13:0, from include/linux/highmem.h:8, from /home/arnd/arm-soc/fs/binfmt_elf.c:28: fs/binfmt_elf.c: In function 'create_elf_tables': arch/x86/include/asm/uaccess_64.h:143:20: error: array subscript is above array bounds [-Werror=array-bounds] __put_user_asm(4[(u16 *)src], 4 + (u16 __user *)dst, ^ arch/x86/include/asm/uaccess.h:473:16: note: in definition of macro '__put_user_asm' : ltype(x), "m" (__m(addr)), "i" (errret), "0" (err)) ^ arch/x86/include/asm/uaccess_64.h:143:20: error: array subscript is above array bounds [-Werror=array-bounds] __put_user_asm(4[(u16 *)src], 4 + (u16 __user *)dst, ^ arch/x86/include/asm/uaccess.h:473:16: note: in definition of macro '__put_user_asm' : ltype(x), "m" (__m(addr)), "i" (errret), "0" (err)) ^ arch/x86/include/asm/uaccess_64.h:154:20: error: array subscript is above array bounds [-Werror=array-bounds] __put_user_asm(1[(u64 *)src], 1 + (u64 __user *)dst, ^ arch/x86/include/asm/uaccess.h:473:16: note: in definition of macro '__put_user_asm' : ltype(x), "m" (__m(addr)), "i" (errret), "0" (err)) ^ arch/x86/include/asm/uaccess_64.h:154:20: error: array subscript is above array bounds [-Werror=array-bounds] __put_user_asm(1[(u64 *)src], 1 + (u64 __user *)dst, ^ arch/x86/include/asm/uaccess.h:473:16: note: in definition of macro '__put_user_asm' : ltype(x), "m" (__m(addr)), "i" (errret), "0" (err)) ^ As we know from other false-positive warnings that we get with UBSAN, the sanitizers disable a couple of optimization steps in the compiler that get in the way of sanitizing, but otherwise would let the compiler figure out that it doesn't need to warn. I considered turning off -Warray-bounds whenever UBSAN is enabled, however, I got exactly two such warnings in the entire kernel with UBSAN, and the other one turned out to be a real kernel stack overflow. Using copy_to_user instead of __copy_to_user shuts up the warning here and is harmless, but is otherwise a completely bogus change as the function is still using a mix of __copy_to_user and copy_to_user. I have not found out why create_elf_tables() uses the __copy_to_user version in the first place, and the right answer might be that it should simply use copy_to_user() and put_user() everywhere. Signed-off-by: Arnd Bergmann --- fs/binfmt_elf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.9.0 diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index 879ff9c7ffd0..f4fe681855ec 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -195,7 +195,7 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec, size_t len = strlen(k_platform) + 1; u_platform = (elf_addr_t __user *)STACK_ALLOC(p, len); - if (__copy_to_user(u_platform, k_platform, len)) + if (copy_to_user(u_platform, k_platform, len)) return -EFAULT; }