In the Linux kernel, we have several instances of code exceeding the permitted stack frame limit (usually 1kb or 2kb per function) when asan-stack=1 is used. A typical pattern is something like extern void f_multiple(unsigned char *arg, unsigned long count); static inline void f_single(unsigned char arg) { f_multiple(&arg, 1); } void g(void) { f_single(0); f_single(1); f_single(2); /* ... */ f_single(100); } gcc -Wall -c test.c -O2 -fsanitize=kernel-address --param asan-stack=1 -Wframe-larger-than=0 test.c: In function ‘g’: test.c:15:1: warning: the frame size of 288 bytes is larger than 0 bytes [-Wframe-larger-than=] Here, each call to f_single() allocates a new stack location for its argument, and adds a 32-byte redzone before and after each byte to catch out-of-bounds access on the pointer. In comparison, clang/llvm appears to to better in two ways here: 1. As of https://github.com/llvm-mirror/llvm/commit/daa1bf3b74054#diff-a6f91f41a097bdf01d36783f8bec4ed6R43, the redzone size is dynamically adapted to the object size, using only 16 bytes for variables up to four bytes, but also using much larger redzones for large stack objects. 2. In some cases, the stack for the temporary objects gets reused between calls, leading to the stack usage to no longer scale linearly with the number of calls to the inline helper. I have a workaround for the kernel that marks all inline functions as __attribute__((noinline)) when I found a code path that has an excessive stack usage with asan-stack using an affected gcc version (gcc-4.9 and higher, I have not tried versions higher than 7.1.1). It would be good to improve the stack frame allocation here in a future gcc release so we can turn off that workaround again in the kernel when using newer compilers.
May I please ask you for pre-processed source file. Note that with GCC 7 the situation can be even worse as we added -fsanitize-address-use-after-scope. Having the example I can do some measurements and we'll think about improvements.
Created attachment 42178 [details] preprocessed linux/drivers/media/dvb-frontends/stv090x.c, compressed This is one of the typical files showing the behavior, with stack sizes up to 8 KB using gcc, but only a few hundred bytes using clang: $ /home/arnd/cross-gcc/bin/x86_64-linux-gcc-8.0.0 -c /tmp/stv090x.i -Werror=frame-larger-than=256 -O2 -fsanitize=kernel-address --param asan-stack=0 # no warnings $ /home/arnd/cross-gcc/bin/x86_64-linux-gcc-8.0.0 -c stv090x.i -Werror=frame-larger-than=128 -O2 -fsanitize=kernel-address --param asan-stack=1 /git/arm-soc/drivers/media/dvb-frontends/stv090x.c: In function 'stv090x_i2c_gate_ctrl': /git/arm-soc/drivers/media/dvb-frontends/stv090x.c:810:1: error: the frame size of 288 bytes is larger than 256 bytes [-Werror=frame-larger-than=] /git/arm-soc/drivers/media/dvb-frontends/stv090x.c: In function 'stv090x_blind_search': /git/arm-soc/drivers/media/dvb-frontends/stv090x.c:2067:1: error: the frame size of 8688 bytes is larger than 256 bytes [-Werror=frame-larger-than=] } ^ /git/arm-soc/drivers/media/dvb-frontends/stv090x.c: In function 'stv090x_optimize_track': /git/arm-soc/drivers/media/dvb-frontends/stv090x.c:3089:1: error: the frame size of 5872 bytes is larger than 256 bytes [-Werror=frame-larger-than=] } ^ /git/arm-soc/drivers/media/dvb-frontends/stv090x.c: In function 'stv090x_search': /git/arm-soc/drivers/media/dvb-frontends/stv090x.c:3505:1: error: the frame size of 8016 bytes is larger than 256 bytes [-Werror=frame-larger-than=] } ^ /git/arm-soc/drivers/media/dvb-frontends/stv090x.c: In function 'stv090x_attach': /git/arm-soc/drivers/media/dvb-frontends/stv090x.c:4998:1: error: the frame size of 1152 bytes is larger than 256 bytes [-Werror=frame-larger-than=] $ /home/arnd/cross-gcc/bin/clang -Wframe-larger-than=256 /tmp/stv090x.i -c -Wall -Wno-constant-logical-operand -O2 -Wno-unused-value -Wno-pointer-sign -fsanitize=kernel-address -mllvm -asan-stack=0 /git/arm-soc/drivers/media/dvb-frontends/stv090x.c:3173:34: warning: stack frame size of 264 bytes in function 'stv090x_algo' [-Wframe-larger-than=] static enum stv090x_signal_state stv090x_algo(struct stv090x_state *state) $ /home/arnd/cross-gcc/bin/clang -Wframe-larger-than=256 /tmp/stv090x.i -c -Wall -Wno-constant-logical-operand -O2 -Wno-unused-value -Wno-pointer-sign -fsanitize=kernel-address -mllvm -asan-stack=1 /git/arm-soc/drivers/media/dvb-frontends/stv090x.c:756:12: warning: stack frame size of 280 bytes in function 'stv090x_write_reg' [-Wframe-larger-than=] static int stv090x_write_reg(struct stv090x_state *state, unsigned int reg, u8 data) ^ /git/arm-soc/drivers/media/dvb-frontends/stv090x.c:3173:34: warning: stack frame size of 376 bytes in function 'stv090x_algo' [-Wframe-larger-than=] static enum stv090x_signal_state stv090x_algo(struct stv090x_state *state) ^
(In reply to Arnd Bergmann from comment #2) > Created attachment 42178 [details] > preprocessed linux/drivers/media/dvb-frontends/stv090x.c, compressed > > This is one of the typical files showing the behavior, with stack sizes up > to 8 KB using gcc, but only a few hundred bytes using clang: > Looking at it again, it was probably not the best example, in this case the difference between clang and gcc is that clang happens to inline stv090x_write_regs while gcc doesn't inline it. If I mark that function as __always_inline, gcc produces good code as well, but that is a little besides the point. In a lot of other examples the function that gets called is from another file, so we can't inline it. I can attach a few other examples if that helps, or you could just replace the function with an extern declaration for testing.
Created attachment 42195 [details] preprocessed net/wireless/nl80211.c, compressed This is another file that shows the problem, in fact we hit this with every allmodconfig build using gcc-5 through gcc-8.0.0: /home/arnd/cross-gcc/bin/x86_64-linux-gcc-5.4.1 -c nl80211.i -O2 --std=gnu89 -Wall -Wno-unused -Wno-pointer-sign -Wframe-larger-than=1024 -fno-strict-aliasing -fno-strict-overflow -fsanitize=kernel-address --param asan-stack=1 -fconserve-stack/git/arm-soc/net/wireless/nl80211.c: In function 'nl80211_add_commands_unsplit': /git/arm-soc/net/wireless/nl80211.c:1411:1: warning: the frame size of 2208 bytes is larger than 1024 bytes [-Wframe-larger-than=] } ^ /git/arm-soc/net/wireless/nl80211.c: In function 'nl80211_get_mesh_config': /git/arm-soc/net/wireless/nl80211.c:5779:1: warning: the frame size of 2048 bytes is larger than 1024 bytes [-Wframe-larger-than=] } ^ /git/arm-soc/net/wireless/nl80211.c: In function 'nl80211_send_wiphy': /git/arm-soc/net/wireless/nl80211.c:1905:1: warning: the frame size of 3856 bytes is larger than 1024 bytes [-Wframe-larger-than=] } ... /git/arm-soc/net/wireless/nl80211.c: In function 'nl80211_send_station.isra.47': /git/arm-soc/net/wireless/nl80211.c:4476:1: warning: the frame size of 2240 bytes is larger than 1024 bytes [-Wframe-larger-than=] } ^ /git/arm-soc/net/wireless/nl80211.c: In function 'nl80211_dump_station': /git/arm-soc/net/wireless/nl80211.c:4529:1: warning: the frame size of 1168 bytes is larger than 1024 bytes [-Wframe-larger-than=] } ^ /git/arm-soc/net/wireless/nl80211.c: In function 'nl80211_dump_scan': /git/arm-soc/net/wireless/nl80211.c:7837:1: warning: the frame size of 1280 bytes is larger than 1024 bytes [-Wframe-larger-than=] } In comparison: /home/arnd/cross-gcc/bin/clang -c nl80211.i -O2 --std=gnu89 -Wno-gnu -Wall -Wno-unused -Wno-pointer-sign -Wno-tautological-compare -Wno-constant-logical-operand -no-integrated-as -Wframe-larger-than=1024 -fsanitize=kernel-address -mllvm -asan-stack=1 /git/arm-soc/net/wireless/nl80211.c:1420:12: warning: stack frame size of 1816 bytes in function 'nl80211_send_wiphy' [-Wframe-larger-than=] static int nl80211_send_wiphy(struct cfg80211_registered_device *rdev, ^ /git/arm-soc/net/wireless/nl80211.c:14117:6: warning: stack frame size of 1112 bytes in function 'cfg80211_del_sta_sinfo' [-Wframe-larger-than=] void cfg80211_del_sta_sinfo(struct net_device *dev, const u8 *mac_addr, ^ /git/arm-soc/net/wireless/nl80211.c:9717:12: warning: stack frame size of 1144 bytes in function 'cfg80211_cqm_rssi_update' [-Wframe-larger-than=] static int cfg80211_cqm_rssi_update(struct cfg80211_registered_device *rdev, ^ /git/arm-soc/net/wireless/nl80211.c:4531:12: warning: stack frame size of 1112 bytes in function 'nl80211_get_station' [-Wframe-larger-than=] static int nl80211_get_station(struct sk_buff *skb, struct genl_info *info) ^ /git/arm-soc/net/wireless/nl80211.c:4478:12: warning: stack frame size of 1240 bytes in function 'nl80211_dump_station' [-Wframe-larger-than=] static int nl80211_dump_station(struct sk_buff *skb, The proposed workaround in the kernel is https://patchwork.kernel.org/patch/9787449/, but it would be good to not need this for future compilers.
I can confirm that for the biggest function 'nl80211_send_wiphy', it really contains majority of stack variables which are 4B large. Having an adaptive redzone mechanism, one can effectively save half of stack. That will still not be enough for 1024B. Anyway, I there's another example where one needs to limit (or disable ASAN) in kernel: https://patchwork.kernel.org/patch/9653417/ Jakub what's your opinion on this?
(In reply to Martin Liška from comment #5) > I can confirm that for the biggest function 'nl80211_send_wiphy', it really > contains majority of stack variables which are 4B large. Having an adaptive > redzone mechanism, one can effectively save half of stack. That will still > not be enough for 1024B. To clarify the limit: The test output was from a 64-bit build, which currently has a 2048 byte limit in the kernel, while 32-bit targets use a 1024 byte limit. My series that avoids the problem here and in other files by adding optional 'noinline' attributes has a follow-up series that lowers the 64-bit limit to 1536 bytes after addressing all the warnings, but that is not currently posted for inclusion. If we can get gcc-8 to stay below 2048 bytes for all files without the noinline annotation, we could still keep using that limit. > Anyway, I there's another example where one needs to limit (or disable ASAN) > in kernel: https://patchwork.kernel.org/patch/9653417/ If I remember correctly, that one only happens with -fsanitize-address-use-after-scope, and I had a workaround that avoids this by changing the kernel code.
Ok, I'm quite opened for changes that will make smaller red zones for smaller variables. However, in case of sanitization-aware inlining, it's probably too complicated and I would rather use no_inline attribute on places where needed.
(In reply to Martin Liška from comment #7) > Ok, I'm quite opened for changes that will make smaller red zones for > smaller variables. However, in case of sanitization-aware inlining, it's > probably too complicated and I would rather use no_inline attribute on > places where needed. Ok, makes sense. What do you think of a possible optimization of the way that the stack variables get allocated (regardless of asan-stack), to allow gcc to reuse the stack locations for multiple instances of inlining the same function? I guess we don't want to do that for -fsanitize-address-use-after-scope, but for all other cases it sounds like a useful optimization that would drastically reduce the frame size of nl80211_send_wiphy() and many other functions. When I looked at this in the past, I found that clang does this more often than gcc already, and it did not seem to be impacted by enabling or disabling -fsanitize=kernel-address or asan-stack=1.
Unless -fsanitize-user-after-scope (default for -fsanitize=address, but not for -fsanitize=kernel-address), GCC does reuse stack slots. Just try say: void foo (int *, int *, int *, int *, int *, int *); void bar () { { int a, b, c, d, e, f; foo (&a, &b, &c, &d, &e, &f); } { int g, b, c, d, e, f; foo (&g, &b, &c, &d, &e, &f); } { int h, b, c, d, e, f; foo (&h, &b, &c, &d, &e, &f); } { int i, b, c, d, e, f; foo (&i, &b, &c, &d, &e, &f); } { int a, b, c, d, e, f; foo (&a, &b, &c, &d, &e, &f); } { int a, b, c, d, e, f; foo (&a, &b, &c, &d, &e, &f); } { int a, b, c, d, e, f; foo (&a, &b, &c, &d, &e, &f); } { int a, b, c, d, e, f; foo (&a, &b, &c, &d, &e, &f); } } int baz (int *p) { return *p; }
As far as I can tell, gcc doesn't merge stack slots that came from inline functions, as in comment 1, or this example: void baz (int *, int *, int *, int *, int *, int *); static inline void foo (int a, int b, int c, int d, int e, int f) { baz (&a, &b, &c, &d, &e, &f); } void bar (int a, int b, int c, int d, int e, int f) { foo (a, b, c, d, e, f); foo (a, b, c, d, e, f); foo (a, b, c, d, e, f); foo (a, b, c, d, e, f); foo (a, b, c, d, e, f); foo (a, b, c, d, e, f); foo (a, b, c, d, e, f); foo (a, b, c, d, e, f); } The frame sizes I see here are gcc-8 -O2: 192 bytes gcc-8 -O2 -fsanitize=address: 3120 bytes gcc-8 -O2 -fsanitize=kernel-address: 192 bytes gcc-8 -O2 -fsanitize=address asan-stack=0: 192 bytes gcc-8 -O2 -fsanitize=kernel-address asan-stack=1: 3120 bytes clang -O2: 72 bytes clang -O2 -fsanitize=address: 88 bytes clang -O2 -fsanitize=kernel-address: 888 bytes clang -O2 -fsanitize=address asan-stack=0: 104 bytes clang -O2 -fsanitize=kernel-address asan-stack=0: 104 bytes (note: clang -fsanitize=kernel-address defaults to asan-stack=1, while gcc defaults to asan-stack=0. gcc-5 and gcc-8 have identical output).
(In reply to Arnd Bergmann from comment #10) > As far as I can tell, gcc doesn't merge stack slots that came from inline > functions, as in comment 1, or this example: > > void baz (int *, int *, int *, int *, int *, int *); > static inline void foo (int a, int b, int c, int d, int e, int f) > { > baz (&a, &b, &c, &d, &e, &f); > } > void > bar (int a, int b, int c, int d, int e, int f) > { > foo (a, b, c, d, e, f); > foo (a, b, c, d, e, f); > foo (a, b, c, d, e, f); > foo (a, b, c, d, e, f); > foo (a, b, c, d, e, f); > foo (a, b, c, d, e, f); > foo (a, b, c, d, e, f); > foo (a, b, c, d, e, f); > } > > The frame sizes I see here are > > gcc-8 -O2: 192 bytes > gcc-8 -O2 -fsanitize=address: 3120 bytes > gcc-8 -O2 -fsanitize=kernel-address: 192 bytes > gcc-8 -O2 -fsanitize=address asan-stack=0: 192 bytes > gcc-8 -O2 -fsanitize=kernel-address asan-stack=1: 3120 bytes > clang -O2: 72 bytes > clang -O2 -fsanitize=address: 88 bytes > clang -O2 -fsanitize=kernel-address: 888 bytes > clang -O2 -fsanitize=address asan-stack=0: 104 bytes > clang -O2 -fsanitize=kernel-address asan-stack=0: 104 bytes > > (note: clang -fsanitize=kernel-address defaults to asan-stack=1, while gcc > defaults to asan-stack=0. gcc-5 and gcc-8 have identical output). That is unrelated to sanitization, we don't merge them with just -O2 either. We do handle: void baz (int *, int *, int *, int *, int *, int *); static inline void foo (int a, int b, int c, int d, int e, int f) { int a2 = a, b2 = b, c2 = c, d2 = d, e2 = e, f2 = f; baz (&a2, &b2, &c2, &d2, &e2, &f2); } void bar (int a, int b, int c, int d, int e, int f) { foo (a, b, c, d, e, f); foo (a, b, c, d, e, f); foo (a, b, c, d, e, f); foo (a, b, c, d, e, f); foo (a, b, c, d, e, f); foo (a, b, c, d, e, f); foo (a, b, c, d, e, f); foo (a, b, c, d, e, f); } though; the problem is that while for this testcase we have CLOBBER stmts for the addressable variables, when inlining a function which has addressable arguments the inliner doesn't add CLOBBER stmts for the arguments turned into variables. Let me fix that.
Created attachment 42212 [details] gcc8-pr81715.patch Untested fix.
Tested the fix with an x86 allmodconfig kernel (linux-next, with -fsanitize-address-use-after-scope disabled manually). With an arbitrary limit of 1500 bytes (the default is no limit when -fsanitize=kernel-address is used), I get 46 warnings in 22 files without the fix, including the two files I attached earlier. With the patch applied, only six warnings remain, and they are all below 2048 bytes (which I would then suggest as the new warning limit for sanitized kernels): net/caif/cfctrl.c:555:1: error: the frame size of 1568 bytes is larger than 1500 bytes [-Werror=frame-larger-than=] drivers/crypto/qat/qat_common/qat_hal.c:963:1: error: the frame size of 1800 bytes is larger than 1500 bytes [-Werror=frame-larger-than=] drivers/input/mouse/cyapa_gen5.c:2434:1: error: the frame size of 1920 bytes is larger than 1500 bytes [-Werror=frame-larger-than=] drivers/isdn/hardware/eicon/message.c:5984:1: error: the frame size of 2016 bytes is larger than 1500 bytes [-Werror=frame-larger-than=] drivers/media/tuners/tda8290.c:310:1: error: the frame size of 1664 bytes is larger than 1500 bytes [-Werror=frame-larger-than=] drivers/staging/rtl8712/rtl871x_ioctl_linux.c:335:1: error: the frame size of 1712 bytes is larger than 1500 bytes [-Werror=frame-larger-than=] This is wonderful, it almost solves the entire problem that I have spent several weeks on finding workarounds for over the past year. I would still like to see the redzone size change we discussed earlier, to help with the remaining warnings (I'd have to look at the six files I mentioned to see if they would benefit, will open another PR if I see a third problem in there) and with the -fsanitize-address-use-after-scope case. I'll also try to work around unpatched gcc-5/6/7 compilers by using the local variable trick from comment 11 for the worst cases.
Note the patch failed bootstrap, but just the first hunk from it passed bootstrap and is being regtested right now. I'll need to debug what's wrong with the retval clobbers tomorrow.
Author: jakub Date: Thu Sep 21 12:26:34 2017 New Revision: 253065 URL: https://gcc.gnu.org/viewcvs?rev=253065&root=gcc&view=rev Log: PR sanitizer/81715 * tree-inline.c (expand_call_inline): Emit clobber stmts for VAR_DECLs to which addressable non-volatile parameters are mapped and for id->retvar after the return value assignment. Clear id->retval and id->retbnd after inlining. * g++.dg/tree-ssa/pr8781.C (noop): Change argument type from const predicate to const predicate & to avoid UB. * g++.dg/opt/pr81715.C: New test. Added: trunk/gcc/testsuite/g++.dg/opt/pr81715.C Modified: trunk/gcc/ChangeLog trunk/gcc/testsuite/ChangeLog trunk/gcc/testsuite/g++.dg/tree-ssa/pr8781.C trunk/gcc/tree-inline.c
Did this change help? Would it be beneficial to backport to 7.x or even 6.x? As is it is certainly too dangerous, but wonder about additionally guarding the 3 hunks either with (flag_sanitize & SANITIZE_ADDRESS) != 0 or even (flag_sanitize & SANITIZE_KERNEL_ADDRESS) != 0, so it would affect only people using -fsanitize={,kernel-}address or even just -fsanitize=kernel-address.
Author: jakub Date: Fri Oct 27 20:33:35 2017 New Revision: 254179 URL: https://gcc.gnu.org/viewcvs?rev=254179&root=gcc&view=rev Log: Backported from mainline 2017-09-21 Jakub Jelinek <jakub@redhat.com> PR sanitizer/81715 * tree-inline.c (expand_call_inline): Emit clobber stmts for VAR_DECLs to which addressable non-volatile parameters are mapped and for id->retvar after the return value assignment, though for -fsanitize=kernel-address only. Clear id->retval and id->retbnd after inlining. * g++.dg/asan/pr81715.C: New test. Added: branches/gcc-7-branch/gcc/testsuite/g++.dg/asan/pr81715.C Modified: branches/gcc-7-branch/gcc/ChangeLog branches/gcc-7-branch/gcc/testsuite/ChangeLog branches/gcc-7-branch/gcc/tree-inline.c
Author: jakub Date: Thu Jan 18 20:30:33 2018 New Revision: 256861 URL: https://gcc.gnu.org/viewcvs?rev=256861&root=gcc&view=rev Log: PR sanitizer/81715 PR testsuite/83882 * function.h (gimplify_parameters): Add gimple_seq * argument. * function.c: Include gimple.h and options.h. (gimplify_parameters): Add cleanup argument, add CLOBBER stmts for the added local temporaries if needed. * gimplify.c (gimplify_body): Adjust gimplify_parameters caller, if there are any parameter cleanups, wrap whole body into a try/finally with the cleanups. Modified: trunk/gcc/ChangeLog trunk/gcc/function.c trunk/gcc/function.h trunk/gcc/gimplify.c
As it's fixed on GCC-7 and currect trunk, can we Jakub close that?
I haven't heard any answer to #c16 whether it actually helped the kernel or not.
I see, let me unassign that.
(In reply to Jakub Jelinek from comment #20) > I haven't heard any answer to #c16 whether it actually helped the kernel or > not. Sorry about that. Yes, it definitely helped the kernel a lot. At this point, we also have localized workarounds to the same effect (using local variables instead of accessing inline function arguments) in all functions that exceeded the arbitrary 2048 byte stack size limit, and backported into the 4.4 kernel and later, but with the newer gcc releases, we also get a lower stack consumption for lots of other functions that were high but below that limit. I had hoped that we could also do this on gcc-7 branch without KASAN, as high stack consumption is always problematic for the kernel, and the same functions that got bumped over the warning limit with KASAN still suffer from wasted stack space on older compilers without KASAN. Since you consider that too invasive for the stable releases, my current workaround has to suffice. One side issue that is not solved at all by the patch is -fsanitize-address-use-after-scope, since that still leads to extreme stack usage in the kernel. The problem here is that it forces many local variables into separate stack slots even when they could get reused without -fsanitize-address-use-after-scope, making it still actively dangerous to run kernels built with this option. My workaround in the kernel is now to have that option disabled by default and only enabled when users explicitly turn it on. I still think it would be nice to address that in the way I originally suggested, by copying the behavior that LLVM uses with its variably sized redzone area.
> One side issue that is not solved at all by the patch is > -fsanitize-address-use-after-scope, since that still leads to extreme stack > usage in the kernel. The problem here is that it forces many local variables > into separate stack slots even when they could get reused without > -fsanitize-address-use-after-scope, making it still actively dangerous to > run kernels built with this option. Note that's crucial to have separate stack slots to properly catch usage of a stack variable that's our of scope. > My workaround in the kernel is now to have that option disabled by default > and only enabled when users explicitly turn it on. I still think it would be > nice to address that in the way I originally suggested, by copying the > behavior that LLVM uses with its variably sized redzone area. That's definitely possible for GCC 9. Question is whether such change will be sufficient for you. Do you expect it will reduce stack usage in the desired way?
(In reply to Martin Liška from comment #23) > That's definitely possible for GCC 9. Question is whether such change will > be sufficient for you. Do you expect it will reduce stack usage in the > desired way? I've recreated my original finding, comparing a clang-5 release against a recent gcc-8 snapshot. Building an x86 allmodconfig kernel with clang, I get 78 -fsanitize-address-use-after-scope warnings against a 2048 byte limit, the largest ones are: drivers/usb/misc/sisusbvga/sisusb.c:1880:12: warning: stack frame size of 6776 bytes in function 'sisusb_init_gfxcore' [-Wframe-larger-than=] drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c:1521:1: warning: stack frame size of 5176 bytes in function 'gk104_ram_new_' [-Wframe-larger-than=] drivers/usb/misc/sisusbvga/sisusb.c:1750:12: warning: stack frame size of 5112 bytes in function 'sisusb_set_default_mode' [-Wframe-larger-than=] drivers/net/wireless/atmel/atmel.c:1307:5: warning: stack frame size of 5016 bytes in function 'atmel_open' [-Wframe-larger-than=] net/core/ethtool.c:2549:5: warning: stack frame size of 4568 bytes in function 'dev_ethtool' [-Wframe-larger-than=] drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:19216:6: warning: stack frame size of 4312 bytes in function 'wlc_phy_init_nphy' [-Wframe-larger-than=] drivers/media/usb/em28xx/em28xx-dvb.c:1129:12: warning: stack frame size of 3992 bytes in function 'em28xx_dvb_init' [-Wframe-larger-than=] drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c:6802:24: warning: stack frame size of 3960 bytes in function 'load_capture_binaries' [-Wframe-larger-than=] drivers/staging/wlan-ng/cfg80211.c:454:12: warning: stack frame size of 3864 bytes in function 'prism2_connect' [-Wframe-larger-than=] drivers/staging/wilc1000/host_interface.c:2480:13: warning: stack frame size of 3704 bytes in function 'host_if_work' [-Wframe-larger-than=] With gcc-8, the same configuration has 179 warnings, including: drivers/net/wireless/ralink/rt2x00/rt2800lib.c:5650:1: warning: the frame size of 23632 bytes is larger than 2048 bytes [-Wframe-larger-than=] drivers/net/wireless/ralink/rt2x00/rt2800lib.c:4515:1: warning: the frame size of 14056 bytes is larger than 2048 bytes [-Wframe-larger-than=] drivers/net/wireless/ralink/rt2x00/rt2800lib.c:3879:1: warning: the frame size of 11504 bytes is larger than 2048 bytes [-Wframe-larger-than=] lib/atomic64_test.c:250:1: warning: the frame size of 11192 bytes is larger than 2048 bytes [-Wframe-larger-than=] lib/atomic64_test.c:148:1: warning: the frame size of 10360 bytes is larger than 2048 bytes [-Wframe-larger-than=] drivers/net/wireless/ralink/rt2x00/rt73usb.c:1294:1: warning: the frame size of 8680 bytes is larger than 2048 bytes [-Wframe-larger-than=] fs/fscache/stats.c:287:1: warning: the frame size of 6536 bytes is larger than 2048 bytes [-Wframe-larger-than=] drivers/net/wireless/ralink/rt2x00/rt2800lib.c:8655:1: warning: the frame size of 6456 bytes is larger than 2048 bytes [-Wframe-larger-than=] drivers/media/dvb-frontends/stv090x.c:3090:1: warning: the frame size of 5872 bytes is larger than 2048 bytes [-Wframe-larger-than=] drivers/net/wireless/ralink/rt2x00/rt61pci.c:1647:1: warning: the frame size of 5792 bytes is larger than 2048 bytes [-Wframe-larger-than=] drivers/media/dvb-frontends/stv090x.c:1595:1: warning: the frame size of 5304 bytes is larger than 2048 bytes [-Wframe-larger-than=] drivers/scsi/fnic/fnic_trace.c:451:1: warning: the frame size of 5000 bytes is larger than 2048 bytes [-Wframe-larger-than=] drivers/net/wireless/ralink/rt2x00/rt2800lib.c:2417:1: warning: the frame size of 4912 bytes is larger than 2048 bytes [-Wframe-larger-than=] drivers/media/dvb-frontends/stv090x.c:4265:1: warning: the frame size of 4840 bytes is larger than 2048 bytes [-Wframe-larger-than=] Comparing against a 3072 byte limit, I get 18 warnings for clang vs 54 for gcc-8. The detailed analysis of some of those warnings last year had shown that the difference can be traced almost entirely to simple scalar variables that use 64 bytes redzone with gcc but only 16 bytes with clang.
(In reply to Arnd Bergmann from comment #24) > (In reply to Martin Liška from comment #23) > > > That's definitely possible for GCC 9. Question is whether such change will > > be sufficient for you. Do you expect it will reduce stack usage in the > > desired way? > > I've recreated my original finding, comparing a clang-5 release against a > recent gcc-8 snapshot. Building an x86 allmodconfig kernel with clang, I get > 78 -fsanitize-address-use-after-scope warnings against a 2048 byte limit, > the largest ones are: > > drivers/usb/misc/sisusbvga/sisusb.c:1880:12: warning: stack frame size of > 6776 bytes in function 'sisusb_init_gfxcore' [-Wframe-larger-than=] > drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c:1521:1: warning: stack > frame size of 5176 bytes in function 'gk104_ram_new_' [-Wframe-larger-than=] > drivers/usb/misc/sisusbvga/sisusb.c:1750:12: warning: stack frame size of > 5112 bytes in function 'sisusb_set_default_mode' [-Wframe-larger-than=] > drivers/net/wireless/atmel/atmel.c:1307:5: warning: stack frame size of 5016 > bytes in function 'atmel_open' [-Wframe-larger-than=] > net/core/ethtool.c:2549:5: warning: stack frame size of 4568 bytes in > function 'dev_ethtool' [-Wframe-larger-than=] > drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:19216:6: > warning: stack frame size of 4312 bytes in function 'wlc_phy_init_nphy' > [-Wframe-larger-than=] > drivers/media/usb/em28xx/em28xx-dvb.c:1129:12: warning: stack frame size of > 3992 bytes in function 'em28xx_dvb_init' [-Wframe-larger-than=] > drivers/staging/media/atomisp/pci/atomisp2/css2400/sh_css.c:6802:24: > warning: stack frame size of 3960 bytes in function 'load_capture_binaries' > [-Wframe-larger-than=] > drivers/staging/wlan-ng/cfg80211.c:454:12: warning: stack frame size of 3864 > bytes in function 'prism2_connect' [-Wframe-larger-than=] > drivers/staging/wilc1000/host_interface.c:2480:13: warning: stack frame size > of 3704 bytes in function 'host_if_work' [-Wframe-larger-than=] > > > With gcc-8, the same configuration has 179 warnings, including: > > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:5650:1: warning: the frame > size of 23632 bytes is larger than 2048 bytes [-Wframe-larger-than=] > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:4515:1: warning: the frame > size of 14056 bytes is larger than 2048 bytes [-Wframe-larger-than=] > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:3879:1: warning: the frame > size of 11504 bytes is larger than 2048 bytes [-Wframe-larger-than=] > lib/atomic64_test.c:250:1: warning: the frame size of 11192 bytes is larger > than 2048 bytes [-Wframe-larger-than=] > lib/atomic64_test.c:148:1: warning: the frame size of 10360 bytes is larger > than 2048 bytes [-Wframe-larger-than=] > drivers/net/wireless/ralink/rt2x00/rt73usb.c:1294:1: warning: the frame size > of 8680 bytes is larger than 2048 bytes [-Wframe-larger-than=] > fs/fscache/stats.c:287:1: warning: the frame size of 6536 bytes is larger > than 2048 bytes [-Wframe-larger-than=] > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:8655:1: warning: the frame > size of 6456 bytes is larger than 2048 bytes [-Wframe-larger-than=] > drivers/media/dvb-frontends/stv090x.c:3090:1: warning: the frame size of > 5872 bytes is larger than 2048 bytes [-Wframe-larger-than=] > drivers/net/wireless/ralink/rt2x00/rt61pci.c:1647:1: warning: the frame size > of 5792 bytes is larger than 2048 bytes [-Wframe-larger-than=] > drivers/media/dvb-frontends/stv090x.c:1595:1: warning: the frame size of > 5304 bytes is larger than 2048 bytes [-Wframe-larger-than=] > drivers/scsi/fnic/fnic_trace.c:451:1: warning: the frame size of 5000 bytes > is larger than 2048 bytes [-Wframe-larger-than=] > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:2417:1: warning: the frame > size of 4912 bytes is larger than 2048 bytes [-Wframe-larger-than=] > drivers/media/dvb-frontends/stv090x.c:4265:1: warning: the frame size of > 4840 bytes is larger than 2048 bytes [-Wframe-larger-than=] > > Comparing against a 3072 byte limit, I get 18 warnings for clang vs 54 for > gcc-8. The detailed analysis of some of those warnings last year had shown > that the difference can be traced almost entirely to simple scalar variables > that use 64 bytes redzone with gcc but only 16 bytes with clang. Ok, I don't have problem to implement the similar behavior in GCC 9. Looks most of warnings are in drivers. That should not be problem as I guess KASAN build is mainly used in a qemu machine (with syzkaller)? Thus exotic drivers should not be needed? I have question about clang where they use for instance: else if (Size <= 128) Res = Size + 32; So: $ cat test.c int main (void) { char my_char[N]; char *ptr, *ptr2; { char my_char2[N]; ptr = &my_char[N - 1]; ptr2 = &my_char2[N - 1]; } return *(ptr2+8); } $ clang -v clang version 5.0.1 (tags/RELEASE_501/final 312548) $ clang test.c -fsanitize-address-use-after-scope -fsanitize=address -D N=128 && ./a.out ================================================================= ==18745==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fffffffdb27 at pc 0x0000004e7f29 bp 0x7fffffffd9d0 sp 0x7fffffffd9c8 READ of size 1 at 0x7fffffffdb27 thread T0 #0 0x4e7f28 in main (/tmp/a.out+0x4e7f28) #1 0x7ffff6eb1f49 in __libc_start_main (/lib64/libc.so.6+0x20f49) #2 0x41c199 in _start /home/abuild/rpmbuild/BUILD/glibc-2.26/csu/../sysdeps/x86_64/start.S:120 Address 0x7fffffffdb27 is located in stack of thread T0 at offset 327 in frame #0 0x4e7d9f in main (/tmp/a.out+0x4e7d9f) This frame has 2 object(s): [32, 160) 'my_char' [192, 320) 'my_char2' <== Memory access at offset 327 overflows this variable HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext (longjmp and C++ exceptions *are* supported) SUMMARY: AddressSanitizer: stack-buffer-overflow (/tmp/a.out+0x4e7f28) in main Shadow bytes around the buggy address: 0x10007fff7b10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7b20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7b30: 00 00 00 00 00 00 00 00 00 00 00 00 f1 f1 f1 f1 0x10007fff7b40: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7b50: f2 f2 f2 f2 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 =>0x10007fff7b60: f8 f8 f8 f8[f3]f3 f3 f3 00 00 00 00 00 00 00 00 0x10007fff7b70: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7b80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7b90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7ba0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 0x10007fff7bb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 The middle red zone is only 32B. So I don't understand why 'Size' not used for red zone calculation?
(In reply to Martin Liška from comment #25) > (In reply to Arnd Bergmann from comment #24) > > Ok, I don't have problem to implement the similar behavior in GCC 9. Looks > most > of warnings are in drivers. That should not be problem as I guess KASAN > build is > mainly used in a qemu machine (with syzkaller)? Thus exotic drivers should > not > be needed? I actually have no idea in what other ways it may be used, though I didn't think that running syzkaller was the only use case. It always feels like most bugs in the kernel are in obscure drivers, but then most of the kernel code consists of obscure drivers ;-) Here are some warnings in code that is actually being run. For the full output I see on linux-next, have a look at https://pastebin.com/CMJiUsuR. There are a couple of other warnings mixed in there as well that I'm working on addressing, but it's mainly the stack overflow once I turn on CONFIG_KASAN_EXTRA. arch/x86/kernel/cpu/mshyperv.c:261:1: warning: the frame size of 2704 bytes is larger than 2048 bytes [-Wframe-larger-than=] arch/x86/kvm/emulate.c:2552:1: warning: the frame size of 2128 bytes is larger than 2048 bytes [-Wframe-larger-than=] drivers/acpi/nfit/core.c:3168:1: warning: the frame size of 3952 bytes is larger than 2048 bytes [-Wframe-larger-than=] drivers/firmware/efi/test/efi_test.c:688:1: warning: the frame size of 2400 bytes is larger than 2048 bytes [-Wframe-larger-than=] drivers/gpu/drm/amd/amdgpu/../display/dc/bios/command_table.c:83:1: warning: the frame size of 3760 bytes is larger than 2048 bytes [-Wframe-larger-than=] drivers/md/md.c:8561:1: warning: the frame size of 2544 bytes is larger than 2048 bytes [-Wframe-larger-than=] drivers/net/bonding/bond_netlink.c:677:1: warning: the frame size of 2096 bytes is larger than 2048 bytes [-Wframe-larger-than=] fs/btrfs/relocation.c:1202:1: warning: the frame size of 4272 bytes is larger than 2048 bytes [-Wframe-larger-than=] fs/fscache/stats.c:287:1: warning: the frame size of 6536 bytes is larger than 2048 bytes [-Wframe-larger-than=] fs/jbd2/commit.c:1128:1: warning: the frame size of 3728 bytes is larger than 2048 bytes [-Wframe-larger-than=] fs/nfs/pnfs.c:1892:1: warning: the frame size of 2672 bytes is larger than 2048 bytes [-Wframe-larger-than=] fs/ntfs/mft.c:2756:1: warning: the frame size of 2352 bytes is larger than 2048 bytes [-Wframe-larger-than=] fs/userfaultfd.c:1824:1: warning: the frame size of 2256 bytes is larger than 2048 bytes [-Wframe-larger-than=] fs/xfs/libxfs/xfs_rmap.c:1334:1: warning: the frame size of 2384 bytes is larger than 2048 bytes [-Wframe-larger-than=] kernel/rcu/tree.c:2282:1: warning: the frame size of 3160 bytes is larger than 2048 bytes [-Wframe-larger-than=] lib/rbtree.c:481:1: warning: the frame size of 2520 bytes is larger than 2048 bytes [-Wframe-larger-than=] mm/khugepaged.c:1560:1: warning: the frame size of 2976 bytes is larger than 2048 bytes [-Wframe-larger-than=] mm/migrate.c:2129:1: warning: the frame size of 2104 bytes is larger than 2048 bytes [-Wframe-larger-than=] mm/page_alloc.c:3247:1: warning: the frame size of 4584 bytes is larger than 2048 bytes [-Wframe-larger-than=] mm/vmscan.c:1350:1: warning: the frame size of 5072 bytes is larger than 2048 bytes [-Wframe-larger-than=] net/bridge/br_netlink.c:1446:1: warning: the frame size of 2592 bytes is larger than 2048 bytes [-Wframe-larger-than=] net/core/ethtool.c:2832:1: warning: the frame size of 3376 bytes is larger than 2048 bytes [-Wframe-larger-than=] net/core/rtnetlink.c:1631:1: warning: the frame size of 2272 bytes is larger than 2048 bytes [-Wframe-larger-than=] net/mac80211/util.c:2188:1: warning: the frame size of 2464 bytes is larger than 2048 bytes [-Wframe-larger-than=] net/rxrpc/recvmsg.c:603:1: warning: the frame size of 2424 bytes is larger than 2048 bytes [-Wframe-larger-than=] net/sctp/socket.c:7271:1: warning: the frame size of 2704 bytes is larger than 2048 bytes [-Wframe-larger-than=] net/wireless/nl80211.c:1938:1: warning: the frame size of 4248 bytes is larger than 2048 bytes [-Wframe-larger-than=] > The middle red zone is only 32B. So I don't understand why 'Size' not used > for red zone > calculation? No idea.
Let me decrypt how clang generates the red zones. I can probably quickly come up with a patch that will do the dynamic red zone size allocation. Having that you'll be able to rebuild kernel and catch remaining issues.
(In reply to Martin Liška from comment #27) > Let me decrypt how clang generates the red zones. I can probably quickly > come up with a patch that will do the dynamic red zone size allocation. > Having that you'll be able to rebuild kernel and catch remaining issues. So the functions looks as follows: static size_t VarAndRedzoneSize(size_t Size, size_t Granularity, size_t Alignment) { size_t Res = 0; if (Size <= 4) Res = 16; else if (Size <= 16) Res = 32; else if (Size <= 128) Res = Size + 32; else if (Size <= 512) Res = Size + 64; else if (Size <= 4096) Res = Size + 128; else Res = Size + 256; return alignTo(std::max(Res, 2 * Granularity), Alignment); } It's bit confusing as it's size of variable plus red zone in shadow memory. So to calculate just the size of redzone one has to subtract Size. And there's some rounding up according to alignment. I've prepared table with size of variable and corresponding middle end red zone size: +---------------+-----+------+---------+ | Variable size | GCC | LLVM | ratio | +---------------+-----+------+---------+ | 1 | 63 | 15 | 23.81% | | 4 | 60 | 12 | 20.00% | | 8 | 56 | 24 | 42.86% | | 12 | 52 | 20 | 38.46% | | 16 | 48 | 16 | 33.33% | | 32 | 32 | 32 | 100.00% | | 40 | 56 | 40 | 71.43% | | 50 | 46 | 46 | 100.00% | | 64 | 32 | 32 | 100.00% | | 96 | 32 | 32 | 100.00% | | 128 | 32 | 32 | 100.00% | | 129 | 63 | 79 | 125.40% | | 196 | 60 | 76 | 126.67% | | 256 | 32 | 64 | 200.00% | | 257 | 63 | 79 | 125.40% | | 511 | 33 | 65 | 196.97% | | 512 | 32 | 64 | 200.00% | | 1024 | 32 | 128 | 400.00% | | 1025 | 63 | 143 | 226.98% | | 2048 | 32 | 128 | 400.00% | | 4096 | 32 | 128 | 400.00% | | 8192 | 32 | 256 | 800.00% | +---------------+-----+------+---------+ As seen (and mentioned by you), for small variables we have bigger red zones.
I'm got a patch candidate for which I did testing of allmodconfig configuration. Sorting all violations against 2KB of stack memory: Before: TOTAL warnings: 185 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:5653:1: 23624 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:4518:1: 14144 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:3882:1: 11504 lib/atomic64_test.c:250:1: 11192 lib/atomic64_test.c:148:1: 10352 drivers/net/wireless/ralink/rt2x00/rt73usb.c:1294:1: 8680 drivers/net/wireless/ralink/rt2x00/rt2500pci.c:1047:1: 7712 drivers/net/wireless/ralink/rt2x00/rt2500usb.c:891:1: 7592 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:2717:1: 7472 drivers/media/dvb-frontends/stv090x.c:3431:1: 6808 mm/vmscan.c:1497:1: 6688 fs/fscache/stats.c:286:1: 6536 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:8659:1: 6456 drivers/media/dvb-frontends/stv090x.c:3090:1: 5880 lib/test_overflow.c:483:1: 5856 drivers/net/wireless/ralink/rt2x00/rt61pci.c:1647:1: 5792 drivers/media/dvb-frontends/stv090x.c:1595:1: 5304 drivers/scsi/fnic/fnic_trace.c:451:1: 5008 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:2420:1: 4912 drivers/media/dvb-frontends/stv090x.c:4265:1: 4840 net/wireless/nl80211.c:2102:1: 4656 drivers/net/wireless/ralink/rt2x00/rt61pci.c:2631:1: 4416 drivers/net/wireless/realtek/rtl818x/rtl8180/rtl8225se.c:431:1: 4376 drivers/scsi/lpfc/lpfc_attr.c:510:1: 4272 fs/btrfs/relocation.c:1149:1: 4224 drivers/staging/rtlwifi/btcoexist/halbtc8822b2ant.c:3545:1: 4216 drivers/acpi/nfit/core.c:3213:1: 4176 drivers/net/wireless/ralink/rt2x00/rt2400pci.c:909:1: 4128 drivers/net/wireless/ralink/rt2x00/rt61pci.c:1963:1: 4032 drivers/net/wireless/ralink/rt2x00/rt73usb.c:1853:1: 4000 drivers/net/wireless/ralink/rt2x00/rt61pci.c:2497:1: 3968 fs/jbd2/commit.c:1129:1: 3920 drivers/scsi/lpfc/lpfc_debugfs.c:978:1: 3896 fs/ocfs2/super.c:1234:1: 3856 drivers/media/dvb-frontends/cxd2841er.c:3302:1: 3816 drivers/media/dvb-frontends/stv0910.c:1547:1: 3808 drivers/media/dvb-frontends/stv0367.c:2607:1: 3744 net/core/ethtool.c:2872:1: 3584 drivers/net/wireless/ralink/rt2x00/rt73usb.c:1962:1: 3472 kernel/rcu/tree.c:2239:1: 3448 drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:16563:1: 3424 fs/ocfs2/namei.c:1691:1: 3400 drivers/net/wireless/ralink/rt2x00/rt73usb.c:1527:1: 3392 mm/khugepaged.c:1565:1: 3296 drivers/media/dvb-frontends/stv090x.c:1952:1: 3272 drivers/staging/rtl8723bs/hal/HalBtc8723b2Ant.c:2924:1: 3208 kernel/fork.c:2104:1: 3192 drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:16905:1: 3160 drivers/media/dvb-frontends/cxd2841er.c:3501:1: 3152 drivers/scsi/snic/snic_debugfs.c:355:1: 3064 fs/ocfs2/aops.c:1906:1: 3032 drivers/media/dvb-frontends/stv090x.c:1854:1: 3008 drivers/net/wireless/ralink/rt2x00/rt2400pci.c:1171:1: 2992 drivers/net/wireless/ralink/rt2x00/rt2400pci.c:399:1: 2984 drivers/net/wireless/ralink/rt2x00/rt2500pci.c:405:1: 2984 drivers/net/wireless/ralink/rt2x00/rt2500usb.c:1434:1: 2976 arch/x86/kernel/cpu/mshyperv.c:307:1: 2960 net/sched/cls_flower.c:1286:1: 2936 drivers/isdn/hardware/avm/b1.c:637:1: 2936 fs/f2fs/segment.c:4182:1: 2912 drivers/scsi/mpt3sas/mpt3sas_scsih.c:9467:1: 2904 net/rxrpc/call_event.c:459:1: 2896 drivers/rapidio/devices/rio_mport_cdev.c:2129:1: 2872 drivers/net/wireless/ralink/rt2x00/rt61pci.c:1503:1: 2848 fs/nfs/write.c:1357:1: 2840 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:3999:1: 2840 fs/nfs/pnfs.c:2023:1: 2832 net/caif/cfctrl.c:549:1: 2824 drivers/block/drbd/drbd_req.c:1447:1: 2800 mm/gup.c:1757:1: 2792 fs/ocfs2/xattr.c:3686:1: 2784 drivers/media/dvb-frontends/cxd2841er.c:3639:1: 2784 fs/ocfs2/super.c:2355:1: 2768 net/sctp/socket.c:7632:1: 2768 drivers/net/wireless/ralink/rt2x00/rt73usb.c:734:1: 2760 drivers/net/ethernet/rocker/rocker_ofdpa.c:563:1: 2760 net/rxrpc/input.c:1362:1: 2752 drivers/net/wireless/ralink/rt2x00/rt2500pci.c:1323:1: 2752 drivers/media/dvb-frontends/cxd2841er.c:3132:1: 2744 fs/btrfs/disk-io.c:3341:1: 2720 drivers/iio/common/ssp_sensors/ssp_spi.c:449:1: 2720 fs/f2fs/gc.c:825:1: 2704 fs/xfs/libxfs/xfs_rmap.c:1348:1: 2696 fs/ocfs2/dlm/dlmmaster.c:2767:1: 2664 lib/test_overflow.c:588:1: 2648 net/core/rtnetlink.c:1700:1: 2648 drivers/gpu/drm/tinydrm/ili9225.c:260:1: 2616 drivers/net/wireless/ralink/rt2x00/rt2500usb.c:591:1: 2616 fs/f2fs/gc.c:1250:1: 2608 fs/nfsd/export.c:652:1: 2608 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:6805:1: 2608 mm/rmap.c:1627:1: 2600 net/bridge/br_netlink.c:1481:1: 2592 drivers/media/tuners/r820t.c:1327:1: 2576 drivers/media/dvb-frontends/stv090x.c:2069:1: 2544 drivers/scsi/qla2xxx/qla_isr.c:1257:1: 2544 fs/btrfs/tree-log.c:3185:1: 2528 drivers/md/md.c:8645:1: 2528 drivers/media/dvb-frontends/stv090x.c:2508:1: 2528 lib/rbtree.c:481:1: 2520 lib/rbtree.c:463:1: 2520 mm/page_alloc.c:5041:1: 2512 mm/migrate.c:576:1: 2512 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:4834:1: 2512 net/rxrpc/recvmsg.c:608:1: 2488 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:3181:1: 2488 drivers/firmware/efi/test/efi_test.c:688:1: 2480 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:836:1: 2480 drivers/media/pci/solo6x10/solo6x10-disp.c:289:1: 2472 net/mac80211/util.c:2263:1: 2464 drivers/gpu/drm/nouveau/nvkm/subdev/i2c/anx9805.c:189:1: 2464 mm/ksm.c:2180:1: 2456 fs/ocfs2/dlm/dlmrecovery.c:752:1: 2456 drivers/block/drbd/drbd_req.c:1624:1: 2448 drivers/input/mouse/cyapa_gen5.c:2434:1: 2440 drivers/net/macsec.c:2597:1: 2432 drivers/scsi/fnic/fnic_fcs.c:918:1: 2424 net/rxrpc/rxkad.c:399:1: 2400 fs/nilfs2/segment.c:1530:1: 2392 fs/cachefiles/rdwr.c:674:1: 2384 mm/memory-failure.c:1925:1: 2384 fs/userfaultfd.c:1843:1: 2384 net/wireless/nl80211.c:4796:1: 2376 drivers/scsi/lpfc/lpfc_nvmet.c:1462:1: 2376 fs/ocfs2/namei.c:2064:1: 2368 drivers/net/wireless/ralink/rt2x00/rt2500usb.c:1122:1: 2368 drivers/infiniband/hw/hfi1/rc.c:2482:1: 2360 fs/ocfs2/dlm/dlmmaster.c:1013:1: 2352 drivers/media/dvb-frontends/cxd2841er.c:3211:1: 2344 net/wireless/nl80211.c:6130:1: 2328 drivers/input/touchscreen/ads7846.c:1459:1: 2312 mm/swapfile.c:2236:1: 2304 mm/migrate.c:726:1: 2304 fs/ocfs2/dir.c:3107:1: 2304 sound/soc/intel/haswell/sst-haswell-ipc.c:640:1: 2296 net/smc/smc_cdc.c:323:1: 2296 drivers/media/i2c/saa6752hs.c:555:1: 2288 net/wireless/nl80211.c:1586:1: 2272 drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:26034:1: 2272 mm/shmem.c:1937:1: 2256 fs/xfs/xfs_log_recover.c:2008:1: 2256 fs/f2fs/file.c:1189:1: 2248 net/ieee802154/nl802154.c:551:1: 2248 drivers/block/drbd/drbd_state.c:2039:1: 2240 fs/ocfs2/namei.c:503:1: 2232 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:1681:1: 2224 mm/memory-failure.c:1419:1: 2216 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:7378:1: 2216 mm/madvise.c:441:1: 2208 drivers/net/virtio_net.c:984:1: 2200 drivers/media/dvb-frontends/stv0910.c:1613:1: 2192 drivers/media/i2c/cx25840/cx25840-core.c:460:1: 2192 drivers/net/wireless/ath/ath9k/ar9003_eeprom.c:5138:1: 2176 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c:830:1: 2168 drivers/isdn/hardware/eicon/message.c:5985:1: 2160 drivers/media/pci/saa7134/saa7134-cards.c:8068:1: 2160 arch/x86/kvm/emulate.c:2562:1: 2152 drivers/net/wireless/ath/ath9k/ar9003_mac.c:158:1: 2152 fs/xfs/libxfs/xfs_alloc.c:1383:1: 2144 lib/test_overflow.c:606:1: 2128 drivers/i2c/busses/i2c-fsi.c:638:1: 2128 arch/x86/kvm/x86.c:4059:1: 2120 fs/btrfs/scrub.c:3394:1: 2112 block/blk-cgroup.c:1026:1: 2112 drivers/staging/erofs/unzip_vle.c:999:1: 2112 drivers/media/dvb-frontends/stv090x.c:4580:1: 2104 drivers/media/dvb-frontends/stv090x.c:2137:1: 2104 drivers/net/ethernet/intel/igb/igb_ethtool.c:2079:1: 2104 drivers/scsi/qla4xxx/ql4_nx.c:3218:1: 2104 fs/ntfs/mft.c:2764:1: 2096 net/wireless/scan.c:1059:1: 2096 drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c:4349:1: 2096 drivers/net/ethernet/broadcom/cnic.c:2725:1: 2096 net/rxrpc/rxkad.c:499:1: 2088 drivers/net/bonding/bond_netlink.c:677:1: 2088 mm/shmem.c:987:1: 2080 fs/f2fs/debug.c:130:1: 2080 drivers/gpu/drm/nouveau/nvkm/subdev/i2c/anx9805.c:95:1: 2080 drivers/media/dvb-frontends/stv090x.c:1207:1: 2080 drivers/media/dvb-frontends/stv090x.c:1164:1: 2080 drivers/staging/pi433/rf69.c:613:1: 2080 net/mac802154/iface.c:201:1: 2072 fs/ocfs2/dlm/dlmmaster.c:2050:1: 2064 drivers/media/dvb-frontends/stv090x.c:4774:1: 2064 mm/compaction.c:959:1: 2056 after: TOTAL warnings: 43 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:5653:1: 11880 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:4518:1: 7264 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:3882:1: 5840 lib/atomic64_test.c:250:1: 5656 lib/atomic64_test.c:148:1: 5232 drivers/net/wireless/ralink/rt2x00/rt73usb.c:1294:1: 4392 drivers/net/wireless/ralink/rt2x00/rt2500pci.c:1047:1: 3904 drivers/net/wireless/ralink/rt2x00/rt2500usb.c:891:1: 3848 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:2717:1: 3792 drivers/media/dvb-frontends/stv090x.c:3431:1: 3512 mm/vmscan.c:1497:1: 3488 fs/fscache/stats.c:286:1: 3336 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:8659:1: 3288 drivers/media/dvb-frontends/stv090x.c:3090:1: 3032 lib/test_overflow.c:483:1: 2976 drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:16563:1: 2976 drivers/net/wireless/ralink/rt2x00/rt61pci.c:1647:1: 2944 drivers/media/dvb-frontends/stv090x.c:1595:1: 2712 drivers/scsi/fnic/fnic_trace.c:451:1: 2640 drivers/media/dvb-frontends/stv0910.c:1547:1: 2624 drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:16905:1: 2616 net/wireless/nl80211.c:2102:1: 2544 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:2420:1: 2512 net/core/ethtool.c:2872:1: 2496 fs/jbd2/commit.c:1129:1: 2480 drivers/media/dvb-frontends/stv090x.c:4265:1: 2472 net/sched/cls_flower.c:1286:1: 2392 fs/btrfs/relocation.c:1149:1: 2368 drivers/acpi/nfit/core.c:3213:1: 2352 drivers/scsi/lpfc/lpfc_attr.c:510:1: 2320 drivers/net/wireless/ralink/rt2x00/rt61pci.c:2631:1: 2272 drivers/net/wireless/realtek/rtl818x/rtl8180/rtl8225se.c:431:1: 2264 drivers/iio/common/ssp_sensors/ssp_spi.c:449:1: 2176 drivers/staging/rtlwifi/btcoexist/halbtc8822b2ant.c:3545:1: 2168 fs/ocfs2/super.c:1234:1: 2160 fs/ocfs2/namei.c:1691:1: 2152 drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:26034:1: 2144 kernel/rcu/tree.c:2239:1: 2136 drivers/net/wireless/ralink/rt2x00/rt2400pci.c:909:1: 2112 drivers/net/wireless/ralink/rt2x00/rt61pci.c:1963:1: 2080 drivers/net/wireless/ralink/rt2x00/rt61pci.c:2497:1: 2080 drivers/scsi/lpfc/lpfc_debugfs.c:978:1: 2072 drivers/scsi/mpt3sas/mpt3sas_scsih.c:9467:1: 2072 Which is very promising improvement I would say.
(In reply to Martin Liška from comment #29) > I'm got a patch candidate for which I did testing of allmodconfig > configuration. > Sorting all violations against 2KB of stack memory: > > Before: > TOTAL warnings: 185 > > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:5653:1: 23624 > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:4518:1: 14144 > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:3882:1: 11504 > lib/atomic64_test.c:250:1: 11192 > lib/atomic64_test.c:148:1: 10352 This is with -fsanitize-address-use-after-scope, right? > after: > > TOTAL warnings: 43 > > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:5653:1: 11880 > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:4518:1: 7264 > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:3882:1: 5840 > lib/atomic64_test.c:250:1: 5656 > lib/atomic64_test.c:148:1: 5232 > > Which is very promising improvement I would say. Agreed, this looks great. With most of the warnings against the 2048 byte limit gone, we can probably work around the remaining ones by doing local code changes in the kernel. I had patches for some of these in the past, which I can dig up then.
(In reply to Arnd Bergmann from comment #30) > (In reply to Martin Liška from comment #29) > > I'm got a patch candidate for which I did testing of allmodconfig > > configuration. > > Sorting all violations against 2KB of stack memory: > > > > Before: > > TOTAL warnings: 185 > > > > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:5653:1: 23624 > > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:4518:1: 14144 > > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:3882:1: 11504 > > lib/atomic64_test.c:250:1: 11192 > > lib/atomic64_test.c:148:1: 10352 > > This is with -fsanitize-address-use-after-scope, right? Yes. > > > after: > > > > TOTAL warnings: 43 > > > > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:5653:1: 11880 > > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:4518:1: 7264 > > drivers/net/wireless/ralink/rt2x00/rt2800lib.c:3882:1: 5840 > > lib/atomic64_test.c:250:1: 5656 > > lib/atomic64_test.c:148:1: 5232 > > > > Which is very promising improvement I would say. > > Agreed, this looks great. With most of the warnings against the > 2048 byte limit gone, we can probably work around the remaining > ones by doing local code changes in the kernel. I had patches for > some of these in the past, which I can dig up then. Just out of curiosity. Am I right that you're using KASAN build for syzkaller or an other fuzzer? If so, I bet you can't hit most of the stack overflows in drivers as you very probably don't have the real hardware?
(In reply to Martin Liška from comment #31) > (In reply to Arnd Bergmann from comment #30) > > (In reply to Martin Liška from comment #29) > > > Which is very promising improvement I would say. > > > > Agreed, this looks great. With most of the warnings against the > > 2048 byte limit gone, we can probably work around the remaining > > ones by doing local code changes in the kernel. I had patches for > > some of these in the past, which I can dig up then. > > Just out of curiosity. Am I right that you're using KASAN build for > syzkaller or an other fuzzer? If so, I bet you can't hit most of the > stack overflows in drivers as you very probably don't have the > real hardware? No, I don't do any fuzzing myself. The side project that I'm interested in here is to build the kernel in all random configurations without compile-time warnings that may indicate bugs. I tend to build several hundred such kernels per day to catch new bugs in both the (linux-next) kernel and in the toolchain (clang and gcc).
(In reply to Arnd Bergmann from comment #32) > (In reply to Martin Liška from comment #31) > > (In reply to Arnd Bergmann from comment #30) > > > (In reply to Martin Liška from comment #29) > > > > Which is very promising improvement I would say. > > > > > > Agreed, this looks great. With most of the warnings against the > > > 2048 byte limit gone, we can probably work around the remaining > > > ones by doing local code changes in the kernel. I had patches for > > > some of these in the past, which I can dig up then. > > > > Just out of curiosity. Am I right that you're using KASAN build for > > syzkaller or an other fuzzer? If so, I bet you can't hit most of the > > stack overflows in drivers as you very probably don't have the > > real hardware? > > No, I don't do any fuzzing myself. The side project that I'm > interested in here is to build the kernel in all random > configurations without compile-time warnings that may indicate > bugs. I tend to build several hundred such kernels per day to > catch new bugs in both the (linux-next) kernel and in the > toolchain (clang and gcc). Ok, btw. it helped to expose multiple issues in ASAN implementation in GCC. So then it's useful even though you're then not running the instrumented kernels.
For the next version of the patch: https://gcc.gnu.org/ml/gcc-patches/2018-09/msg01529.html I seen even better results: TOTAL warnings: 23 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:5653:1: 11800 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:4518:1: 6352 lib/atomic64_test.c:250:1: 5640 drivers/net/wireless/ralink/rt2x00/rt2800lib.c:3882:1: 4256 drivers/net/wireless/ralink/rt2x00/rt2500pci.c:1047:1: 3888 drivers/net/wireless/ralink/rt2x00/rt73usb.c:1294:1: 3720 mm/vmscan.c:1497:1: 3408 drivers/net/wireless/ralink/rt2x00/rt61pci.c:1647:1: 2928 drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:16563:1: 2816 lib/atomic64_test.c:148:1: 2656 drivers/scsi/fnic/fnic_trace.c:451:1: 2624 drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:16905:1: 2552 net/sched/cls_flower.c:1286:1: 2376 fs/btrfs/relocation.c:1149:1: 2352 drivers/acpi/nfit/core.c:3213:1: 2304 net/core/ethtool.c:2872:1: 2272 fs/jbd2/commit.c:1129:1: 2192 fs/ocfs2/namei.c:1691:1: 2120 drivers/net/wireless/ralink/rt2x00/rt2400pci.c:909:1: 2096 drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_n.c:26034:1: 2080 drivers/iio/common/ssp_sensors/ssp_spi.c:449:1: 2064 drivers/media/dvb-frontends/stv0910.c:1547:1: 2064 drivers/net/wireless/ralink/rt2x00/rt61pci.c:1963:1: 2064
Author: marxin Date: Fri Nov 30 14:25:15 2018 New Revision: 266664 URL: https://gcc.gnu.org/viewcvs?rev=266664&root=gcc&view=rev Log: Make red zone size more flexible for stack variables (PR sanitizer/81715). 2018-11-30 Martin Liska <mliska@suse.cz> PR sanitizer/81715 * asan.c (asan_shadow_cst): Remove, partially transform into flush_redzone_payload. (RZ_BUFFER_SIZE): New. (struct asan_redzone_buffer): New. (asan_redzone_buffer::emit_redzone_byte): Likewise. (asan_redzone_buffer::flush_redzone_payload): Likewise. (asan_redzone_buffer::flush_if_full): Likewise. (asan_emit_stack_protection): Use asan_redzone_buffer class that is responsible for proper aligned stores and flushing of shadow memory payload. * asan.h (ASAN_MIN_RED_ZONE_SIZE): New. (asan_var_and_redzone_size): Likewise. * cfgexpand.c (expand_stack_vars): Use smaller alignment (ASAN_MIN_RED_ZONE_SIZE) in order to make shadow memory for automatic variables more compact. 2018-11-30 Martin Liska <mliska@suse.cz> PR sanitizer/81715 * c-c++-common/asan/asan-stack-small.c: New test. Added: trunk/gcc/testsuite/c-c++-common/asan/asan-stack-small.c Modified: trunk/gcc/ChangeLog trunk/gcc/asan.c trunk/gcc/asan.h trunk/gcc/cfgexpand.c trunk/gcc/testsuite/ChangeLog
Implemented on trunk.