* [PATCH] mlxsw: core_env: Fix stack info leak in mlxsw_env_linecard_modules_power_mode_apply
@ 2025-08-30 8:11 Miaoqian Lin
2025-08-31 10:53 ` Ido Schimmel
0 siblings, 1 reply; 4+ messages in thread
From: Miaoqian Lin @ 2025-08-30 8:11 UTC (permalink / raw)
To: Ido Schimmel, Petr Machata, Andrew Lunn, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni, Vadim Pasternak,
netdev, linux-kernel
Cc: linmq006
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)
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] mlxsw: core_env: Fix stack info leak in mlxsw_env_linecard_modules_power_mode_apply 2025-08-30 8:11 [PATCH] mlxsw: core_env: Fix stack info leak in mlxsw_env_linecard_modules_power_mode_apply Miaoqian Lin @ 2025-08-31 10:53 ` Ido Schimmel 2025-09-01 9:10 ` Petr Machata 0 siblings, 1 reply; 4+ messages in thread From: Ido Schimmel @ 2025-08-31 10:53 UTC (permalink / raw) To: Miaoqian Lin Cc: Petr Machata, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Vadim Pasternak, netdev, linux-kernel 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) > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mlxsw: core_env: Fix stack info leak in mlxsw_env_linecard_modules_power_mode_apply 2025-08-31 10:53 ` Ido Schimmel @ 2025-09-01 9:10 ` Petr Machata 2025-09-01 10:21 ` Ido Schimmel 0 siblings, 1 reply; 4+ messages in thread From: Petr Machata @ 2025-09-01 9:10 UTC (permalink / raw) To: Ido Schimmel Cc: Miaoqian Lin, Petr Machata, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Vadim Pasternak, netdev, linux-kernel 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) >> ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] mlxsw: core_env: Fix stack info leak in mlxsw_env_linecard_modules_power_mode_apply 2025-09-01 9:10 ` Petr Machata @ 2025-09-01 10:21 ` Ido Schimmel 0 siblings, 0 replies; 4+ messages in thread From: Ido Schimmel @ 2025-09-01 10:21 UTC (permalink / raw) To: Petr Machata Cc: Miaoqian Lin, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni, Vadim Pasternak, netdev, linux-kernel 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); } } ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-09-01 10:22 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-08-30 8:11 [PATCH] mlxsw: core_env: Fix stack info leak in mlxsw_env_linecard_modules_power_mode_apply Miaoqian Lin 2025-08-31 10:53 ` Ido Schimmel 2025-09-01 9:10 ` Petr Machata 2025-09-01 10:21 ` Ido Schimmel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).