| Summary: | asan-stack=1 redzone allocation is too inflexible | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Arnd Bergmann <arnd> |
| Component: | sanitizer | Assignee: | Martin Liška <marxin> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | dimhen, dodji, jakub, kcc, pageexec |
| Priority: | P3 | ||
| Version: | 7.1.1 | ||
| Target Milestone: | 9.0 | ||
| Host: | Target: | ||
| Build: | Known to work: | ||
| Known to fail: | Last reconfirmed: | 2017-09-15 00:00:00 | |
| Attachments: |
preprocessed linux/drivers/media/dvb-frontends/stv090x.c, compressed
preprocessed net/wireless/nl80211.c, compressed gcc8-pr81715.patch |
||
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. |
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.