From patchwork Mon Jul 31 09:50:38 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Arnd Bergmann X-Patchwork-Id: 109019 Delivered-To: patch@linaro.org Received: by 10.140.101.6 with SMTP id t6csp175287qge; Mon, 31 Jul 2017 05:41:55 -0700 (PDT) X-Received: by 10.84.225.129 with SMTP id u1mr16799897plj.255.1501504915163; Mon, 31 Jul 2017 05:41:55 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1501504915; cv=none; d=google.com; s=arc-20160816; b=Uu1OB3b3v7jSOjbSEwDLAk0S0G8mQXHUhKDHEa8C8RVuzvP3jUzfMVzOPtTKaoNw8p Tm28DbSm7fJBVT0tVp3VuNDvAVejzKi75eXJBlFnL7kS9+kGcuxql2YXCVTqzuuqnvHM LoWWBjeQNfFVLOBZaDC6anwl1kmymHIllyTdYskStPaDdVQ6VGSHMH5e7RDrYufQSvCK 8aBJuaSowwmjYWEjfCg71xieWKpXfAbVZo83Rrswp8f2BS/rmH3cbjflY8C5n/SDBHGR XN8UH4M2P+aSJecFjhgrCv6YjQ00y4DOcQk4qqfRfyabEb/tbLmghhAgcnj7mLGAdCRg FvRg== 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=sFJY2H9bqEwRWmfAQvkfrkIoiice3DHAJnBCHErOr6vfdAgZ74M0cytnJW5WFx8GT7 xrdX6LcOALsbmMv5DD23wsaqzFqq9dj27CULMWv+x/wkn7T6ObhrZDnhGy9VH2cDKHU4 o62HuDgL0C7JiiT97NUS6FBX55KdAxaAX9Yqj64v4QPX3ZiuoSeirBOqMeOUoXOWfubP Juz3rGSytl/0vTokb1LS38e8W0Wjm23/ACOa6jVo0uEt9Iv2iMkkk8mC5NxlmExYv41D 4o5h0ovCU3MmS5Odd7yo8HVeXzS786htdz8sJstBjfTityTDCcaXXPZchVbWc8IVy63f 58oA== 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 o61si17186007pld.425.2017.07.31.05.41.54; Mon, 31 Jul 2017 05:41:55 -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 S1752168AbdGaMlw (ORCPT + 26 others); Mon, 31 Jul 2017 08:41:52 -0400 Received: from mout.kundenserver.de ([217.72.192.75]:56930 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750923AbdGaMlu (ORCPT ); Mon, 31 Jul 2017 08:41:50 -0400 Received: from wuerfel.lan ([78.43.238.10]) by mrelayeu.kundenserver.de (mreue103 [212.227.15.145]) with ESMTPA (Nemesis) id 0MV2eD-1d3qKU0Cx3-00YQfw; Mon, 31 Jul 2017 14:41:39 +0200 From: Arnd Bergmann To: Alexander Viro Cc: Arnd Bergmann , Ingo Molnar , Andrew Morton , Kees Cook , Frederic Weisbecker , 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:50:38 +0200 Message-Id: <20170731124128.461001-1-arnd@arndb.de> X-Mailer: git-send-email 2.9.0 X-Provags-ID: V03:K0:gqpwyrVfPDi+KJxd+P80sxLxKiB45psQo/HUClUXCWsQ9ZBpqmy XqVBw04wrC4PKdBlWUKfdcFhiXw4eCrNK93b9qo6vr8mdIP4O5SLv31KDdXk8zTr3lp9q/U NrvEAB5IzmPjuMzBrnbwYb/N5oFT/AooXG8XW+R9AvVdNjAg1QwietFhFP41NImYw/r4f9B ax9sq+JiHDqFicYyB7nFg== X-UI-Out-Filterresults: notjunk:1; V01:K0:rhYNhbfPpQI=:V9TTISDTWCNr2H+ozo/hFw jiGSN4v1sZSQw5s4TMDimkoetTfewOgvR26kfVpcD561dMn55WEPxOzujQRM7d468XNhp0w8f cV3QcFyVhQT3RF2CfYnNnR2pxg+AItZZmP6K6hW6X8HsRnZTgvOs3ZspuyD+/GylwtAlc9nz7 0ZcUs0PlQhg/CtuqZq6YkxcoPhINYTBcrlQ83PfgbOKNrRiSs5R5pFHJoWv+lJPGQScx9qJSw KkemAx+wenVOFq+BSC8iDIjtCJ0L0VVYNFXTQmndH0yCorvynEMPl4HotMCwcjVK5xBbqo877 0M1QxTqK8Ge3yLkbxDEbUSa+cm/ID5Agjh+BtQT1NpZwRBp9bHgolbb8uH2yQjR/YS3H+9k06 vpdEeSgoLXSlA/yfU69XXohHrCsjou7nbFCv2IfF37fWaPrrGbPJlkWrQEU9xcYUg9wO+HxLt WlDV3HVaY2R7tEC9CqlWim83ZqTSjzWda74e/YiYHk+awXXXrgNXOMeQAY1OOi5810sGs1l3g cGf/QbQ8B4BaXCqf6Tc6+e8AZeAuClKBIiVAk8WBKahR6dK6Y/mYNk3Wnwe5+Db/Y33mZNLWf abnGVbviKCOMkf0jjJ4vXxEB03HV++ELkQoOe+q5zkGOswC7PE9kkpPtIBgnKqgbRMV5wQDzM AaXTp+DX/YXUVzMtJt1hZfF9mdj8ldXMlShh2b2pH451155Cg5JHcztNrfFgBakAELopeqyrw dF5azl7SxEnHPT894ho6wvlFcGw9Mk3UGGTD6Q== 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; }