-Wframe-larger-than= if KASAN_STACK is enabled

[nickdesaulniers]

Probably from CONFIG_KASAN=y but at least two instances:

  CC      fs/ext4/inode.o
fs/ext4/inode.c:53:14: warning: stack frame size of 2688 bytes in function 'ext4_inode_csum' [-Wframe-larger-than=]
static __u32 ext4_inode_csum(struct inode *inode, struct ext4_inode *raw,
             ^
  CC      fs/f2fs/inode.o
fs/f2fs/inode.c:137:14: warning: stack frame size of 2304 bytes in function 'f2fs_inode_chksum' [-Wframe-larger-than=]
static __u32 f2fs_inode_chksum(struct f2fs_sb_info *sbi, struct page *page)
             ^

[nickdesaulniers]

  CC      fs/jbd2/commit.o
fs/jbd2/commit.c:344:6: warning: stack frame size of 2304 bytes in function
      'jbd2_journal_commit_transaction' [-Wframe-larger-than=]
void jbd2_journal_commit_transaction(journal_t *journal)
     ^
1 warning generated.
  CC      fs/jbd2/recovery.o
fs/jbd2/recovery.c:416:12: warning: stack frame size of 2688 bytes in function
      'do_one_pass' [-Wframe-larger-than=]
static int do_one_pass(journal_t *journal,
           ^

[nickdesaulniers]

  CC      fs/ocfs2/cluster/heartbeat.o
fs/ocfs2/cluster/heartbeat.c:1212:12: warning: stack frame size of 2144 bytes in
      function 'o2hb_thread' [-Wframe-larger-than=]
static int o2hb_thread(void *data)
           ^
  CC      fs/ocfs2/dlm/dlmdomain.o
fs/ocfs2/dlm/dlmdomain.c:2125:19: warning: stack frame size of 3104 bytes in function
      'dlm_register_domain' [-Wframe-larger-than=]
struct dlm_ctxt * dlm_register_domain(const char *domain,
                  ^
  CC      fs/ocfs2/dlm/dlmrecovery.o
fs/ocfs2/dlm/dlmrecovery.c:301:12: warning: stack frame size of 3872 bytes in function
      'dlm_recovery_thread' [-Wframe-larger-than=]
static int dlm_recovery_thread(void *data)
           ^
fs/ocfs2/dlm/dlmmaster.c:718:28: warning: stack frame size of 2400 bytes in function
      'dlm_get_lock_resource' [-Wframe-larger-than=]
struct dlm_lock_resource * dlm_get_lock_resource(struct dlm_ctxt *dlm,
                           ^
  CC      fs/ocfs2/aops.o
fs/ocfs2/aops.c:1672:5: warning: stack frame size of 2336 bytes in function
      'ocfs2_write_begin_nolock' [-Wframe-larger-than=]
int ocfs2_write_begin_nolock(struct address_space *mapping,
    ^
  CC      fs/ocfs2/dir.o
fs/ocfs2/dir.c:4263:5: warning: stack frame size of 2400 bytes in function
      'ocfs2_prepare_dir_for_insert' [-Wframe-larger-than=]
int ocfs2_prepare_dir_for_insert(struct ocfs2_super *osb,
    ^
fs/ocfs2/dir.c:3175:12: warning: stack frame size of 2080 bytes in function
      'ocfs2_extend_dir' [-Wframe-larger-than=]
static int ocfs2_extend_dir(struct ocfs2_super *osb,
           ^
  CC      fs/ocfs2/refcounttree.o
fs/ocfs2/refcounttree.c:4420:5: warning: stack frame size of 2208 bytes in function
      'ocfs2_reflink_ioctl' [-Wframe-larger-than=]
int ocfs2_reflink_ioctl(struct inode *inode,
    ^
  CC      fs/ocfs2/move_extents.o
fs/ocfs2/move_extents.c:971:5: warning: stack frame size of 2144 bytes in function
      'ocfs2_ioctl_move_extents' [-Wframe-larger-than=]
int ocfs2_ioctl_move_extents(struct file *filp, void __user *argp)
    ^
  CC      fs/ocfs2/super.o
fs/ocfs2/super.c:1002:12: warning: stack frame size of 3712 bytes in function
      'ocfs2_fill_super' [-Wframe-larger-than=]
static int ocfs2_fill_super(struct super_block *sb, void *data, int silent)
           ^
  CC      fs/quota/quota.o
fs/quota/quota.c:836:5: warning: stack frame size of 3456 bytes in function
      'kernel_quotactl' [-Wframe-larger-than=]
int kernel_quotactl(unsigned int cmd, const char __user *special,
    ^
  CC      drivers/block/DAC960.o
drivers/block/DAC960.c:6511:16: warning: stack frame size of 2624 bytes in function
      'dac960_user_command_proc_write' [-Wframe-larger-than=]
static ssize_t dac960_user_command_proc_write(struct file *file,
               ^
  CC      drivers/crypto/ccp/ccp-ops.o
drivers/crypto/ccp/ccp-ops.c:2434:5: warning: stack frame size of 8928 bytes in
      function 'ccp_run_cmd' [-Wframe-larger-than=]
  CC      drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.o
drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.c:2401:6: warning: stack frame
      size of 2272 bytes in function 'core_link_enable_stream' [-Wframe-larger-than=]
void core_link_enable_stream(
     ^
  CC      drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.o
drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgt215.c:940:1: warning: stack frame size of
      2848 bytes in function 'gt215_ram_new' [-Wframe-larger-than=]
gt215_ram_new(struct nvkm_fb *fb, struct nvkm_ram **pram)
^
  CC      drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgf100.o
drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgf100.c:567:1: warning: stack frame size of
      3744 bytes in function 'gf100_ram_new_' [-Wframe-larger-than=]
gf100_ram_new_(const struct nvkm_ram_func *func,
^
  CC      drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.o
drivers/gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c:1521:1: warning: stack frame size of
      5824 bytes in function 'gk104_ram_new_' [-Wframe-larger-than=]
gk104_ram_new_(const struct nvkm_ram_func *func, struct nvkm_fb *fb,
^

[tpimh]

I guess this can be related (but I feel like submitting a separate issue for this):

arch/x86/kvm/x86.c:7369:12: warning: stack frame size of 2200 bytes in function 'vcpu_enter_guest' [-Wframe-larger-than=]
static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
           ^

arch/x86/kvm/emulate.c:3341:5: warning: stack frame size of 2200 bytes in function 'emulator_task_switch' [-Wframe-larger-than=]
int emulator_task_switch(struct x86_emulate_ctxt *ctxt,
    ^

[tpimh]

One more:

fs/f2fs/gc.c:1131:5: warning: stack frame size of 2168 bytes in function 'f2fs_gc' [-Wframe-larger-than=]
int f2fs_gc(struct f2fs_sb_info *sbi, bool sync,
    ^

[nickdesaulniers]

Useful comment in #96 (comment)

[nathanchance]

Jason Donenfeld happened to submit a patch independent of this issue to raise the stack frame limit to 3072 for CONFIG_KASAN, not just CONFIG_KASAN_EXTRA

https://lore.kernel.org/lkml/20180921001513.12504-1-Jason@zx2c4.com/

That should take care of most of the warnings reported here. If/when applied, we should see if any of the still present warnings should be addressed. If not, we could raise (or eliminate) the limit for Clang like so:

default x if KASAN && CC_IS_CLANG

[nickdesaulniers]

grep 'stack frame' stack.log.txt | cut -d ' ' -f 7 > stack.txt

import matplotlib.pyplot as plt
import numpy as np

with open('stack.txt', 'r') as f:
  stack_sizes = np.array(f.readlines(), dtype=np.int)
  default = 3072
  above = np.count_nonzero(stack_sizes > default)
  total = len(stack_sizes)
  below = total - above
  plt.hist(stack_sizes, total)
  plt.axvline(x=default, color='r')
  plt.text(default / 2, 20, str(below))
  plt.text(default + 1000, 20, str(above))
  plt.text(default + 2000, 20, "total: " + str(total))
  plt.show()

[nathanchance]

LLVM bug: https://llvm.org/pr38809

[nathanchance]

More discussion upstream: https://lore.kernel.org/lkml/20190219214940.391081-1-arnd@arndb.de/

[melver]

And just to summarize:

• KASAN stack instrumentation's purpose is to detect use-after-return bugs.
• Disabling it for purely cosmetic reasons (vs. real bug at runtime) would negatively affect our ability to find bugs.
• If KASAN's increased stack utilization would cause bugs at runtime, testing would have already shown this -- the major architectures can build KASAN with VMAP_STACK, which would detect stack overflows.
• KASAN builds already at least double stack size. If that's not enough, increasing that again for KASAN builds would be appropriate given the already significantly increased memory usage of KASAN_GENERIC (due to shadow memory).
• Build testers should always select COMPILE_TEST, given it already disables KASAN stack instrumentation (already the case for allyesconfig and allmodconfig).
• KASAN_SW_TAGS does not suffer from this problem (requires arm64 TBI, or in future, Intel LAM).
• Increasing the warning threshold or turning Wframe-larger-than into a non-fatal error for KASAN+STACK builds would be preferred.
• Use dynamic alloca approach, but that's not going to change stack usage. More complex, and effect is the same as making the warning non-fatal or increasing the threshold.
• It would be good to understand what GCC does differently, and if there's a difference between stack-use-after-return coverage.

[nickdesaulniers]

Also, whether VMAP_STACK can be required for KASAN_STACK. Currently VMAP_STACK depends on !KASAN || ...

[arndb]

On Wed, Sep 29, 2021 at 6:27 PM Marco Elver ***@***.***> wrote: And just to summarize: KASAN stack instrumentation's purpose is to detect use-after-return bugs. Disabling it for purely cosmetic reasons (vs. real bug at runtime) would negatively affect our ability to find bugs. If KASAN's increased stack utilization would cause bugs at runtime, testing would have already shown this -- the major architectures can build KASAN with VMAP_STACK, which would detect stack overflows. KASAN builds already at least double stack size. If that's not enough, increasing that again for KASAN builds would be appropriate given the already significantly memory usage of KASAN_GENERIC (due to shadow memory). Build testers should always select COMPILE_TEST, given it already disables KASAN stack instrumentation (already the case for allyesconfig and allmodconfig). KASAN_SW_TAGS does not suffer from this problem (requires arm64 TBI, or in future, Intel LAM). Increasing the warning threshold or turning Wframe-larger-than into a non-fatal error for KASAN+STACK builds would be preferred.

I think we can change the warning limit for normal KASAN builds to something around 1400 bytes, that covers everything that I see with both gcc-11 and clang-14, except for the amdgpu driver. I really don't want to make any special case in the kernel for KASAN_STACK with clang. No matter how useful you may personally find it, it's been broken for many years, and I don't think it will ever get fixed if we pretend it's not broken now. I reported the bug[1] over three years ago for llvm, and it has been mostly ignored so far. There seems to be some movement now, and I'd like to keep it that way. gcc-8 had similar problems, which I reported as well, and those were fixed in gcc-9 and gcc-10. While we are waiting for llvm to address this, here are some things you can tell your users: - use gcc-10 or higher if possible - turn off any drivers that show those errors when they don't require the drivers for their machines. - if it happens in a driver they do care about, ask the users to report the specific errors with a .config file in [1], so those can be prioritized in fixing. - raise the warning limit in the local .config that turns on KASAN_STACK - if nothing helps, turn off CONFIG_WERROR in those configs

Use dynamic alloca approach, but that's not going to change stack usage. More complex, and effect is the same as making the warning non-fatal or increasing the threshold. It would be good to understand what GCC does differently, and if there's a difference between stack-use-after-return coverage

I spent months coming up with detailed analysis back in 2018 on both compilers. The main issues are: - the amount of padding between variables is different between compilers. gcc used to have excessive amounts of padding here, and I suggested they do whatever llvm has, but they came up with a better algorithm. It would make sense to revisit the llvm algorithm for determining that passing, to make it work well both for large variables and for lots of small ones. - inlining is fundamentally different between gcc and llvm, and it changed drastically between gcc versions as well. This is important here because with KASAN_STACK, local variables of multiple called functions can not share stack slots. The kernel code has some __always_inline and __noinline_for_stack annotations on certain functions to work around cases where this goes wrong, but additional annotations may be needed for llvm. - some optimizations that are used to create better code are disabled when using KASAN (regardless of the stack instrumentation), which makes analysis easier, but also causes larger stack usage when combined with the other effects. It might help to turn certain optimization passes back on when using the stack instrumentation. - As a special case of this, some of the missed optimizations cause the register allocator to fail spectacularly, leading to a simple function that should use only a few bytes using many kilobytes for spilling registers, sometimes 100KB or more. Arnd [1] https://llvm.org/pr38809

[nickdesaulniers]

I wonder if -mllvm -max-inline-stacksize=<N> might be useful here.
https://reviews.llvm.org/rGc50e6f590cd4

[nathanchance]

I wonder if -mllvm -max-inline-stacksize=<N> might be useful here. https://reviews.llvm.org/rGc50e6f590cd4

Even better, there is now a clang option for it: llvm/llvm-project@8564e2f

[nickdesaulniers]

One thought on -finline-max-stacksize=; it only looks at the callee size, so llvm can still create large callers since the caller may have multiple callees.

void foo (void) {
  bar();
  baz();
}

individually, bar and baz may be below the threshold of -finline-max-stacksize= but when inlined together may still exceed -Wframe-larger-than=. Still probably better than nothing though!

[nickdesaulniers]

commit ebb6d35 ("kasan: remove clang version check for KASAN_STACK")

is the kernel workaround to avoid a majority of these warnings for now.

I need to manually:

1. disable TINY_SLUB
2. disable COMPILE_TEST
3. enable KASAN_STACK

to start reproducing these warnings again. (The use of ASan.CompileKernel in FunctionStackPoisoner::processStaticAllocas in LLVM is potentially interesting).

[kees]

We are seeing similar issues under UBSAN (triggered by bounds checking being expanded to more structures):

https://lore.kernel.org/linux-mm/202306100035.VTusNhm4-lkp@intel.com/
https://lore.kernel.org/linux-mm/202306210637.TpfYq9gM-lkp@intel.com

[kees]

And likely some more inlining-causes-too-many-spills cases:

https://lore.kernel.org/lkml/202306171406.rQWGJPpr-lkp@intel.com
https://lore.kernel.org/linux-mm/202306210722.cJWYeZkQ-lkp@intel.com

[nickdesaulniers]

clang-18 should have some improved stack usage when passing structs by value: https://reviews.llvm.org/rGe698695fbbf62e6676f8907665187f2d2c4d814b

[nickdesaulniers]

the fix was reverted: llvm/llvm-project#43598 (comment) due to an issue with C++ references and ASAN. Will work on relanding.

[nickdesaulniers]

llvm/llvm-project#68746
llvm/llvm-project#68747