Messages in this thread Patch in this message | | | Subject | Re: [RFC] kasan stack overflow warnings again: READ_ONCE(), typecheck() | From | Andrey Ryabinin <> | Date | Wed, 15 Feb 2017 17:06:53 +0300 |
| |
On 02/15/2017 02:03 AM, Arnd Bergmann wrote: > 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,
I think the following patch on top of yours should fix that for READ_ONCE(). WRITE_ONCE() probably can be fixed in a similar way, but I didn't try it.
diff --git a/include/linux/compiler.h b/include/linux/compiler.h index 10bca12..5d9dd63 100644 --- a/include/linux/compiler.h +++ b/include/linux/compiler.h @@ -301,10 +301,10 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s */ #define __READ_ONCE(x, check) \ - __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), \ + __builtin_choose_expr(sizeof(x) == 1, (typeof(x))(__u64)*(volatile __u8 *)&(x), \ + __builtin_choose_expr(sizeof(x) == 2, (typeof(x))(__u64)*(volatile __u16 *)&(x), \ + __builtin_choose_expr(sizeof(x) == 4, (typeof(x))(__u64)*(volatile __u32 *)&(x), \ + __builtin_choose_expr(sizeof(x) == 8, (typeof(x))(__u64)*(volatile __u64 *)&(x), \ ({union { typeof(x) __val; char __c[1]; } __u; \ if (check) \ __read_once_size(&(x), __u.__c, sizeof(x)); \
> 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/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; \
This should be under "if (check)" otherwise it breaks READ_ONCE_NOCHECK() > 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) >
| |