linux-kernel - Re: [PATCH 2/3] media: vicodec: avoic clang frame size warning

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 19 Feb 2019 20:14:08 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Nick Desaulniers <ndesaulniers@...gle.com>
Cc:     Hans Verkuil <hans.verkuil@...co.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Mark Brown <broonie@...nel.org>,
        Nathan Chancellor <natechancellor@...il.com>,
        Dafna Hirschfeld <dafna3@...il.com>,
        Tom aan de Wiel <tom.aandewiel@...il.com>,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 2/3] media: vicodec: avoic clang frame size warning

On Tue, Feb 19, 2019 at 8:02 PM Nick Desaulniers
<ndesaulniers@...gle.com> wrote:
> On Tue, Feb 19, 2019 at 9:02 AM Arnd Bergmann <arnd@...db.de> wrote:
> >
> > Clang-9 makes some different inlining decisions compared to gcc, which
> > leads to a warning about a possible stack overflow problem when building
> > with CONFIG_KASAN, including when setting asan-stack=0, which avoids
> > most other frame overflow warnings:
> >
> > drivers/media/platform/vicodec/codec-fwht.c:673:12: error: stack frame size of 2224 bytes in function 'encode_plane'
> >
> > Manually adding noinline_for_stack annotations in those functions
>
> Thanks for the fix! In general, for -Wstack-frame-larger-than=
> warnings, is it possible that these sets of stack frames are already
> too large if entered?  Sure, inlining was a little aggressive, causing
> more stack space use than maybe otherwise necessary at runtime, but
> isn't it also possible that "no inlining" a stack frame can still be a
> problem should the stack frame be entered?  Doesn't the kernel have a
> way of estimating the stack depth for any given frame?  I guess I was
> always curious if the best fix for these kind of warnings was to
> non-stack allocate (kmalloc) certain locally allocated structs, or
> no-inline the function.  Surely there's cases where no-inlining is
> safe, but I was curious if it's still maybe dangerous to enter the
> problematic child most stack frame?

What I think is happening here is that llvm fails to combine the
stack allocations for the inlined functions in certain conditions,
while gcc can reuse it here. We had similar issues in gcc
a few years ago, and they got fixed there, but I have not looked
at this one in more detail. My guess is that it's related to
the bug I mentioned in patch 3.

      Arnd