[PATCH] mlxsw: core_env: Fix stack info leak in mlxsw_env_linecard_modules_power_mode_apply

[PATCH] mlxsw: core_env: Fix stack info leak in mlxsw_env_linecard_modules_power_mode_apply

[Miaoqian Lin]

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.

Fixes: 06a0fc43bb10 ("mlxsw: core_env: Add interfaces for line card initialization and de-initialization")
Signed-off-by: Miaoqian Lin <linmq006@gmail.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)

Re: [PATCH] mlxsw: core_env: Fix stack info leak in mlxsw_env_linecard_modules_power_mode_apply

[Ido Schimmel]

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.

> 
> Fixes: 06a0fc43bb10 ("mlxsw: core_env: Add interfaces for line card initialization and de-initialization")
> Signed-off-by: Miaoqian Lin <linmq006@gmail.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)
> 

Re: [PATCH] mlxsw: core_env: Fix stack info leak in mlxsw_env_linecard_modules_power_mode_apply

[Petr Machata]

Ido Schimmel <idosch@nvidia.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@gmail.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)
>> 

Re: [PATCH] mlxsw: core_env: Fix stack info leak in mlxsw_env_linecard_modules_power_mode_apply

[Ido Schimmel]

On Mon, Sep 01, 2025 at 11:10:19AM +0200, Petr Machata wrote:
> 
> Ido Schimmel <idosch@nvidia.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.

Can be solved with this diff:

diff --git a/drivers/net/ethernet/mellanox/mlxsw/core_env.c b/drivers/net/ethernet/mellanox/mlxsw/core_env.c
index 294e758f1067..8908520f39d9 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];
@@ -1340,7 +1340,7 @@ mlxsw_env_linecard_modules_power_mode_apply(struct mlxsw_core *mlxsw_core,
 		err = mlxsw_env_set_module_power_mode_apply(mlxsw_core,
 							    slot_index, i,
 							    policy, &extack);
-		if (err)
+		if (err && extack._msg)
 			dev_err(env->bus_info->dev, "%s\n", extack._msg);
 	}
 }