From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751348AbdBNXEJ (ORCPT ); Tue, 14 Feb 2017 18:04:09 -0500 Received: from mout.kundenserver.de ([212.227.126.131]:54005 "EHLO mout.kundenserver.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750791AbdBNXEI (ORCPT ); Tue, 14 Feb 2017 18:04:08 -0500 From: Arnd Bergmann To: Dmitry Vyukov Cc: "kasan-dev@googlegroups.com" , Andrey Ryabinin , Alexander Potapenko , linux-kernel@vger.kernel.org, Christian Borntraeger Subject: [RFC] kasan stack overflow warnings again: READ_ONCE(), typecheck() Date: Wed, 15 Feb 2017 00:03:53 +0100 Message-ID: <1745339.Zp71hEDpdI@wuerfel> User-Agent: KMail/5.1.3 (Linux/4.8.0-34-generic; KDE/5.18.0; x86_64; ; ) MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" X-Provags-ID: V03:K0:kmZ0bDZjmxAtSkWTsv17mhlQaCtdXpfPS8TXn2lE0c7TZoAyOcx 9bHpLcZU1X+2QTy+XIoeWdfs7Gzoi8rg9qvr4l5MTsrpO2AVX0s2KLera1Tf5QSRpn4zHQe A7M1QxVgAm+Ci7OUVrN4YOxdARpx6due+7s+r2+P59cQHfDzx75bbGEYzCmRs1jkQCDltB5 n2Iuff3nlEX+8JfFQmdiQ== X-UI-Out-Filterresults: notjunk:1;V01:K0:yHcaT49TgaI=:104teJJ+r2xf/XbMqJdKV6 q06DtoJ3mhrElbFRumyGeE6EYJpXcEs4y3LPFeeZfJE5hbttzBk+6oBmn0Pkj2TO24v1V0rkX p6UCxBnDVUOIyak/lCRpzFbZuMRzmq3/C6SKoyVTvF1+QWK5bdZDf9FF4juGG986I+kJNZ/mX xLeEASnIHgf5dNlXHEbCQtLduI1sPtZBY60yaHls3kGsj/Kujgg1OVUAxhv5ePHUrGFZXBeZw T2rwodH5igxADxsvxHZiY/95PGl8kOAnUNDfJH/vF3VUqTBWwAsennD9LuJQq7ljNhkGeL/i8 z7RV/CwtWPSORtWIktCtfBwI9J1L3vGzRklP0vPp3bdUMxUj4wTPm2cJkUlha7/sRwszf6ohM bI813Ki+eFtIVQ/zr74fSCJKl8CURsT/xYzfVBW0rM+QyzYICuS5/UMI1KKq88wlGE0SwhNZW HMLw6ydGJfttPWEOeRJ+1nE+rBWH+7z9gAXkhO49E0/whVjfj4Khrd6aE0QHcjI5JuDuVWG2B Fq1Sgqm273GWp6Ej8WsViqumSfTRcGmK6pujjCbi6MJevC8eIyPalsGcUL2idqnytNpcL/amf srBJC5ZFlwadgiSC8/h/73XTt4EBN/EPb+RkfnoDW6+DIRLVjXe3A92aMZ65r6dvyVycp54Q6 wxhNyRkNlg/w/x2HCAvcMlSiKFZA+QuSfGb4YSCDtxIYUsNd9YXNvPe/6y7WbrbA31AsKf6+D aQnzoAmdKM2qyH7T Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Dmitry, I've come a little further on the stack size analysis after initially finding that gcc-7.0.1 is much better than the horrible stack frames we were seeing on gcc-7.0.0 (I found over 80kb in one case), I found more that remain with the newer compiler, but also analyzed the worst remaining ones down to two common helpers: typecheck() caused the worst remaining problem, but only in a few files like "drivers/net/wireless/ralink/rt2x00/rt2800lib.c:5068:1: error: the frame size of 23768 bytes is larger than 1024 bytes". I think my fix should be able to make it in, but I'd give it some more testing. It also seems here that gcc asan-stack isn't too smart, as it adds checks to local variables we never use except for comparing their addresses. This may also impact min() and max(), which have the same check. READ_ONCE()/WRITE_ONCE() are used for atomic_t and caused the next worst offender (12688 byte stacks in lib/atomic64_test.c) as well as a lot of other instances that were over 2048 byte stacks. This one is a lot trickier as my the version from my patch is most likely not safe for all callers, and the helper has been extended to cover a lot of corner cases that my version destroys (it doesn't force aggregates to be accessed as scalars for example, probably also causes sparse warnings, and maybe doesn't even guarantee the "ONCE" semantics). I see that Andrey also provided a READ_ONCE_NOCHECK() helper that would also take care of this if used consistently, but it seems wrong to use that for all atomics. Any other ideas? Arnd diff --git a/arch/x86/include/asm/switch_to.h b/arch/x86/include/asm/switch_to.h index fcc5cd387fd1..0c243dd569fe 100644 --- a/arch/x86/include/asm/switch_to.h +++ b/arch/x86/include/asm/switch_to.h @@ -30,7 +30,7 @@ static inline void prepare_switch_to(struct task_struct *prev, * * To minimize cache pollution, just follow the stack pointer. */ - READ_ONCE(*(unsigned char *)next->thread.sp); + (void)READ_ONCE(*(unsigned char *)next->thread.sp); #endif } diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c index 01157d6e8cfe..dc677c7c22be 100644 --- a/fs/overlayfs/util.c +++ b/fs/overlayfs/util.c @@ -222,8 +222,8 @@ void ovl_dentry_update(struct dentry *dentry, struct dentry *upperdentry) void ovl_inode_init(struct inode *inode, struct inode *realinode, bool is_upper) { - WRITE_ONCE(inode->i_private, (unsigned long) realinode | - (is_upper ? OVL_ISUPPER_MASK : 0)); + WRITE_ONCE(inode->i_private, (void *)((unsigned long) realinode | + (is_upper ? OVL_ISUPPER_MASK : 0))); } void ovl_inode_update(struct inode *inode, struct inode *upperinode) @@ -231,7 +231,7 @@ void ovl_inode_update(struct inode *inode, struct inode *upperinode) WARN_ON(!upperinode); WARN_ON(!inode_unhashed(inode)); WRITE_ONCE(inode->i_private, - (unsigned long) upperinode | OVL_ISUPPER_MASK); + (void *)((unsigned long) upperinode | OVL_ISUPPER_MASK)); if (!S_ISDIR(upperinode->i_mode)) __insert_inode_hash(inode, (unsigned long) upperinode); } diff --git a/include/linux/compiler.h b/include/linux/compiler.h index 416b17e03016..b8018eddd757 100644 --- a/include/linux/compiler.h +++ b/include/linux/compiler.h @@ -310,14 +310,17 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s */ #define __READ_ONCE(x, check) \ -({ \ - union { typeof(x) __val; char __c[1]; } __u; \ + __builtin_choose_expr(sizeof(x) == 1, *(volatile typeof(&(x)))&(x), \ + __builtin_choose_expr(sizeof(x) == 2, *(volatile typeof(&(x)))&(x), \ + __builtin_choose_expr(sizeof(x) == 4, *(volatile typeof(&(x)))&(x), \ + __builtin_choose_expr(sizeof(x) == sizeof(long), *(volatile typeof(&(x)))&(x), \ + ({union { typeof(x) __val; char __c[1]; } __u; \ if (check) \ __read_once_size(&(x), __u.__c, sizeof(x)); \ else \ __read_once_size_nocheck(&(x), __u.__c, sizeof(x)); \ __u.__val; \ -}) + }))))) #define READ_ONCE(x) __READ_ONCE(x, 1) /* @@ -326,13 +329,19 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s */ #define READ_ONCE_NOCHECK(x) __READ_ONCE(x, 0) -#define WRITE_ONCE(x, val) \ -({ \ - union { typeof(x) __val; char __c[1]; } __u = \ - { .__val = (__force typeof(x)) (val) }; \ - __write_once_size(&(x), __u.__c, sizeof(x)); \ - __u.__val; \ -}) +#define WRITE_ONCE(x, val) \ +( \ + __builtin_choose_expr(sizeof(x) == 1, *(volatile typeof(&(x)))&(x) = (val), \ + __builtin_choose_expr(sizeof(x) == 2, *(volatile typeof(&(x)))&(x) = (val), \ + __builtin_choose_expr(sizeof(x) == 4, *(volatile typeof(&(x)))&(x) = (val), \ + __builtin_choose_expr(sizeof(x) == sizeof(long), *(volatile typeof(&(x)))&(x) = (val), \ + ({ \ + typeof(x) __val = (val); \ + barrier(); \ + __builtin_memcpy((void *)&x, (void *)&__val, sizeof(__val)); \ + barrier(); \ + }))))) \ +) #endif /* __KERNEL__ */ diff --git a/include/linux/typecheck.h b/include/linux/typecheck.h index eb5b74a575be..adb1579fa5f0 100644 --- a/include/linux/typecheck.h +++ b/include/linux/typecheck.h @@ -5,12 +5,7 @@ * Check at compile time that something is of a particular type. * Always evaluates to 1 so you may use it easily in comparisons. */ -#define typecheck(type,x) \ -({ type __dummy; \ - typeof(x) __dummy2; \ - (void)(&__dummy == &__dummy2); \ - 1; \ -}) +#define typecheck(type,x) ({(void)((typeof(type) *)NULL == (typeof(x) *)NULL); 1;}) /* * Check at compile time that 'function' is a certain type, or is a pointer