linux-kernel - Re: [PATCH] mlxsw: core_env: Fix stack info leak in mlxsw_env_linecard_modules_power_mode

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]

Message-ID: <87plcav86o.fsf@nvidia.com>
Date: Mon, 1 Sep 2025 11:10:19 +0200
From: Petr Machata <petrm@...dia.com>
To: Ido Schimmel <idosch@...dia.com>
CC: Miaoqian Lin <linmq006@...il.com>, Petr Machata <petrm@...dia.com>,
	"Andrew Lunn" <andrew+netdev@...n.ch>, "David S. Miller"
	<davem@...emloft.net>, "Eric Dumazet" <edumazet@...gle.com>, Jakub Kicinski
	<kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Vadim Pasternak
	<vadimp@...dia.com>, <netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] mlxsw: core_env: Fix stack info leak in
 mlxsw_env_linecard_modules_power_mode_apply


Ido Schimmel <idosch@...dia.com> writes:

> On Sat, Aug 30, 2025 at 04:11:22PM +0800, Miaoqian Lin wrote:
>> The extack was declared on the stack without initialization.
>> If mlxsw_env_set_module_power_mode_apply() fails to set extack,
>> accessing extack._msg could leak information.
>
> Unless I'm missing something, I don't see a case where
> mlxsw_env_set_module_power_mode_apply() returns an error without setting
> extack. IOW, I don't see how this info leak can happen with existing
> code.

Yeah, I agree it all looks initialized.

The patch still makes sense to me, it will make the code less prone to
footguns in the future. The expectation with extack is that it's
optional, though functions that take the argument typically also take
care to set it (or propagate further). But here it is mandatory to
initialize it, or else things break. With the patch we'd get a
"(null)\n" instead of garbage. Not great, but better.

>
>> 
>> Fixes: 06a0fc43bb10 ("mlxsw: core_env: Add interfaces for line card initialization and de-initialization")

Yeah, it doesn't really fix anything, it's a cleanup if anything, so
this tag should go away.

>> Signed-off-by: Miaoqian Lin <linmq006@...il.com>
>> ---
>>  drivers/net/ethernet/mellanox/mlxsw/core_env.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_env.c b/drivers/net/ethernet/mellanox/mlxsw/core_env.c
>> index 294e758f1067..38941c1c35d3 100644
>> --- a/drivers/net/ethernet/mellanox/mlxsw/core_env.c
>> +++ b/drivers/net/ethernet/mellanox/mlxsw/core_env.c
>> @@ -1332,7 +1332,7 @@ mlxsw_env_linecard_modules_power_mode_apply(struct mlxsw_core *mlxsw_core,
>>  	for (i = 0; i < env->line_cards[slot_index]->module_count; i++) {
>>  		enum ethtool_module_power_mode_policy policy;
>>  		struct mlxsw_env_module_info *module_info;
>> -		struct netlink_ext_ack extack;
>> +		struct netlink_ext_ack extack = {};
>>  		int err;
>>  
>>  		module_info = &env->line_cards[slot_index]->module_info[i];
>> -- 
>> 2.39.5 (Apple Git-154)
>>