Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ubsan, mips: excessive stack usage with -fsanitize=alignment #1379

Open
arndb opened this issue May 19, 2021 · 2 comments
Open

ubsan, mips: excessive stack usage with -fsanitize=alignment #1379

arndb opened this issue May 19, 2021 · 2 comments
Labels
[ARCH] mips This bug impacts ARCH=mips

Comments

@arndb
Copy link

arndb commented May 19, 2021

Vincent got a complaint from a bot about a change he made in the scheduler:

https://groups.google.com/d/msgid/clang-built-linux/CAKfTPtB9B9jv_uSk0fS7uOTtH5FCwLcAOrPjtXzRuURLUMFnoA%40mail.gmail.com

I had a look at this, and it seems that clang ends up excessively spilling local variables when building with -fsanitize=alignment on mips32, causing hundreds of warnings like:

../fs/select.c:621:5: warning: stack frame size of 1048 bytes in function 'core_sys_select' [-Wframe-larger-than=]
int core_sys_select(int n, fd_set __user *inp, fd_set __user *outp,
    ^
1 warning generated.
../drivers/bus/mhi/core/pm.c:740:6: warning: stack frame size of 1400 bytes in function 'mhi_pm_st_worker' [-Wframe-larger-than=]
void mhi_pm_st_worker(struct work_struct *work)
     ^
1 warning generated.
../kernel/sched/fair.c:8990:23: warning: unused variable 'child' [-Wunused-variable]
        struct sched_domain *child = env->sd->child;
                             ^
../kernel/sched/fair.c:8527:22: warning: unused function 'update_sd_pick_busiest' [-Wunused-function]
static noinline bool update_sd_pick_busiest(struct lb_env *env,
                     ^
2 warnings generated.
../security/apparmor/policy_unpack.c:1163:5: warning: stack frame size of 1152 bytes in function 'aa_unpack' [-Wframe-larger-than=]
int aa_unpack(struct aa_loaddata *udata, struct list_head *lh,
    ^
1 warning generated.
../kernel/acct.c:426:32: warning: implicit conversion from 'int' to 'char' changes value from 131 to -125 [-Wconstant-conversion]
        ac->ac_version = ACCT_VERSION | ACCT_BYTEORDER;
                       ~ ~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
1 warning generated.
../drivers/dma/dmatest.c:568:12: warning: stack frame size of 1464 bytes in function 'dmatest_func' [-Wframe-larger-than=]
static int dmatest_func(void *data)
           ^

From the assembler code I can see that it checks every pointer for correct alignment before derefencing it, and that apparently lets the register allocator run out of registers.

I tried the same thing on armv5, which also doesn't have native alignment support, but this did not show the same problem, so I guess it's something about the mips backend.

@arndb
Copy link
Author

arndb commented May 19, 2021

And now I found that there is already a workaround in the kernel for it in Kconfig:

config UBSAN_ALIGNMENT
        bool "Perform checking for misaligned pointer usage"
        default !HAVE_EFFICIENT_UNALIGNED_ACCESS
        depends on !UBSAN_TRAP && !COMPILE_TEST
        depends on $(cc-option,-fsanitize=alignment)

The build bot that reported it did not set COMPILE_TEST, otherwise it should not have been able to turn this option on in the absence of CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS for this target.

@nickdesaulniers nickdesaulniers added the [ARCH] mips This bug impacts ARCH=mips label Jun 2, 2021
@nathanchance
Copy link
Member

nathanchance commented Jun 16, 2021

Seems like this might be a problem for 32-bit PowerPC too? https://lore.kernel.org/r/202106100743.JgPkQBD1-lkp@intel.com/

# Enable CONFIG_UBSAN and CONFIG_UBSAN_ALIGNMENT in menuconfig
$ make -skj"$(nproc)" ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- LLVM=1 distclean mpc83xx_defconfig menuconfig drivers/crypto/talitos.o
...
drivers/crypto/talitos.c:3328:12: warning: stack frame size of 1040 bytes in function 'talitos_probe' [-Wframe-larger-than=]
static int talitos_probe(struct platform_device *ofdev)
           ^
...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[ARCH] mips This bug impacts ARCH=mips
Projects
None yet
Development

No branches or pull requests

3 participants