linux-kernel - Re: drivers/net/ethernet/chelsio/cxgb4/sge.c:2396:13: warning: stack frame size of 1168 bytes in function 'ethofld

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] [day] [month] [year] [list]

Date:   Tue, 23 Mar 2021 10:24:22 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     kernel test robot <lkp@...el.com>
Cc:     kbuild-all@...ts.01.org,
        clang-built-linux <clang-built-linux@...glegroups.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Nathan Chancellor <nathan@...nel.org>,
        Kees Cook <keescook@...omium.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linux Memory Management List <linux-mm@...ck.org>
Subject: Re: drivers/net/ethernet/chelsio/cxgb4/sge.c:2396:13: warning: stack
 frame size of 1168 bytes in function 'ethofld_xmit'

On Tue, Mar 23, 2021 at 7:37 AM kernel test robot <lkp@...el.com> wrote:
>
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   84196390620ac0e5070ae36af84c137c6216a7dc
> commit: 97e4910232fa1f81e806aa60c25a0450276d99a2 linux/compiler-clang.h: define HAVE_BUILTIN_BSWAP*
> date:   9 days ago
> config: mips-randconfig-r023-20210322 (attached as .config)
> compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 78a65cd945d006ff02f9d24d9cc20a302ed93b08)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # install mips cross compiling tool for clang build
>         # apt-get install binutils-mips-linux-gnu
>         # https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=97e4910232fa1f81e806aa60c25a0450276d99a2
>         git remote add linus https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
>         git fetch --no-tags linus master
>         git checkout 97e4910232fa1f81e806aa60c25a0450276d99a2
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=mips
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@...el.com>
>
> All warnings (new ones prefixed by >>):
>
>    drivers/net/ethernet/chelsio/cxgb4/sge.c:814:28: warning: unused function 'calc_tx_descs' [-Wunused-function]
>    static inline unsigned int calc_tx_descs(const struct sk_buff *skb,
>                               ^
> >> drivers/net/ethernet/chelsio/cxgb4/sge.c:2396:13: warning: stack frame size of 1168 bytes in function 'ethofld_xmit' [-Wframe-larger-than=]
>    static void ethofld_xmit(struct net_device *dev, struct sge_eosw_txq *eosw_txq)
>                ^
>    2 warnings generated.

This looks related to a warning we saw on powerpc. I've tried digging
into it a little
bit more, but all I found is that the use of __builtin_bswap32() changes the
inlining decisions but doesn't actively cause worse code.

In fact, if I force the inlining like this:

--- a/drivers/net/ethernet/chelsio/cxgb4/sge.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/sge.c
@@ -2257,7 +2257,7 @@ static void *write_eo_wr(struct adapter *adap,
struct sge_eosw_txq *eosw_txq,
        return cpl;
 }

-static int ethofld_hard_xmit(struct net_device *dev,
+static __attribute__((flatten)) __always_inline int
ethofld_hard_xmit(struct net_device *dev,
                             struct sge_eosw_txq *eosw_txq)
 {
        struct port_info *pi = netdev2pinfo(dev);
@@ -2393,7 +2393,7 @@ static int ethofld_hard_xmit(struct net_device *dev,
        return ret;
 }

-static void ethofld_xmit(struct net_device *dev, struct sge_eosw_txq *eosw_txq)
+static noinline void ethofld_xmit(struct net_device *dev, struct
sge_eosw_txq *eosw_txq)
 {
        struct sk_buff *skb;
        int pktcount, ret;

I see a different effect: the function's frame grows to 2232 bytes with the
open-coded bswap32 slightly less at 2200 bytes with the builtin bswap32,
all because of too many variables getting spilled.

On the other hand, marking ethofld_hard_xmit as flatten+noinline, I don't
get these spills with either version of bswap32, and the stack usage of
ethofld_hard_xmit()/ethofld_xmit() goes down to 472+112 bytes.

If I remove -fsanitize=alignment, the total stack size for these functions is
no more than 368 bytes regardless of the inlining or the bswap32()
implementation.

I would conclude that there is something wrong in clang that leads to badly
optimized code in this file, but that my __builtin_bswap32() change is only
what triggers the right conditions here, not the root cause.

        Arnd