netdev - Re: [PATCH] mlx5: reduce stack usage in mlx5_setup

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, 17 Jan 2023 21:01:03 +0100
From:   "Arnd Bergmann" <arnd@...db.de>
To:     "Tariq Toukan" <ttoukan.linux@...il.com>,
        "Arnd Bergmann" <arnd@...nel.org>,
        "Saeed Mahameed" <saeedm@...dia.com>,
        "Leon Romanovsky" <leon@...nel.org>
Cc:     "David S . Miller" <davem@...emloft.net>,
        "Eric Dumazet" <edumazet@...gle.com>,
        "Jakub Kicinski" <kuba@...nel.org>,
        "Paolo Abeni" <pabeni@...hat.com>,
        "Nathan Chancellor" <nathan@...nel.org>,
        "Nick Desaulniers" <ndesaulniers@...gle.com>,
        "Tom Rix" <trix@...hat.com>, "Tariq Toukan" <tariqt@...dia.com>,
        "Maxim Mikityanskiy" <maximmi@...dia.com>,
        "Gal Pressman" <gal@...dia.com>, "Lama Kayal" <lkayal@...dia.com>,
        "Moshe Tal" <moshet@...dia.com>, Netdev <netdev@...r.kernel.org>,
        linux-rdma@...r.kernel.org, linux-kernel@...r.kernel.org,
        llvm@...ts.linux.dev
Subject: Re: [PATCH] mlx5: reduce stack usage in mlx5_setup_tc

On Tue, Jan 17, 2023, at 18:46, Tariq Toukan wrote:
> On 17/01/2023 19:28, Arnd Bergmann wrote:
>> From: Arnd Bergmann <arnd@...db.de>
>> 
>> Clang warns about excessive stack usage on 32-bit targets:
>> 
>> drivers/net/ethernet/mellanox/mlx5/core/en_main.c:3597:12: error: stack frame size (1184) exceeds limit (1024) in 'mlx5e_setup_tc' [-Werror,-Wframe-larger-than]
>> static int mlx5e_setup_tc(struct net_device *dev, enum tc_setup_type type,
>> 
>> It turns out that both the mlx5e_setup_tc_mqprio_dcb() function and
>> the mlx5e_safe_switch_params() function it calls have a copy of
>> 'struct mlx5e_params' on the stack, and this structure is fairly
>> large.
>> 
>> Use dynamic allocation for both.
>> 

>>   
>> -	err = mlx5e_safe_switch_params(priv, &new_params,
>> +	err = mlx5e_safe_switch_params(priv, new_params,
>>   				       mlx5e_num_channels_changed_ctx, NULL, true);
>>   
>
> Is this change really required, even after new_chs are dynamically 
> allocated?
> As this code pattern of static local new_params repeats in all callers 
> of mlx5e_safe_switch_params, let's not change this one alone if not 
> necessary.

I'm not sure any more now, I actually did the patch a few weeks ago
and only now came across it while going through my backlog.

Generally speaking, the 'new_params' structure on the stack is
too large, but I no longer see warnings after my patch.

> Same for the noinline_for_stack. Are they really needed even after using 
> dynamic allocation for new_chs?

I've reverted both of those hunks now, let me try reproducing the
original randconfig reports and see what still happens.

   Arnd