From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752160AbdGaMlw (ORCPT ); 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 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 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(-) 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; } -- 2.9.0