Commit 74cbc3c0 authored by Ido Schimmel's avatar Ido Schimmel Committed by Jakub Kicinski
Browse files

mlxsw: spectrum_acl_tcam: Move devlink param to TCAM code 



Cited commit added 'DEVLINK_CMD_PARAM_DEL' notifications whenever the
network namespace of the devlink instance is changed. Specifically, the
notifications are generated after calling reload_down(), but before
calling reload_up(). At this stage, the data structures accessed while
reading the value of the "acl_region_rehash_interval" devlink parameter
are uninitialized, resulting in a use-after-free [1].

Fix by moving the registration and unregistration of the devlink
parameter to the TCAM code where it is actually used. This means that
the parameter is unregistered during reload_down() and then
re-registered during reload_up(), avoiding the use-after-free between
these two operations.

Reproducer:

 # ip netns add test123
 # devlink dev reload pci/0000:06:00.0 netns test123

[1]
BUG: KASAN: use-after-free in mlxsw_sp_acl_tcam_vregion_rehash_intrvl_get+0xb2/0xd0
Read of size 4 at addr ffff888162fd37d8 by task devlink/1323
[...]
Call Trace:
 <TASK>
 dump_stack_lvl+0x95/0xbd
 print_report+0x181/0x4a1
 kasan_report+0xdb/0x200
 mlxsw_sp_acl_tcam_vregion_rehash_intrvl_get+0xb2/0xd0
 mlxsw_sp_params_acl_region_rehash_intrvl_get+0x32/0x80
 devlink_nl_param_fill.constprop.0+0x29a/0x11e0
 devlink_param_notify.constprop.0+0xb9/0x250
 devlink_notify_unregister+0xbc/0x470
 devlink_reload+0x1aa/0x440
 devlink_nl_cmd_reload+0x559/0x11b0
 genl_family_rcv_msg_doit.isra.0+0x1f8/0x2e0
 genl_rcv_msg+0x558/0x7f0
 netlink_rcv_skb+0x170/0x440
 genl_rcv+0x2d/0x40
 netlink_unicast+0x53f/0x810
 netlink_sendmsg+0x961/0xe80
 __sys_sendto+0x2a4/0x420
 __x64_sys_sendto+0xe5/0x1c0
 do_syscall_64+0x38/0x80
 entry_SYSCALL_64_after_hwframe+0x63/0xcd

Fixes: 7d7e9169 ("devlink: move devlink reload notifications back in between _down() and _up() calls")
Signed-off-by: default avatarIdo Schimmel <idosch@nvidia.com>
Reviewed-by: default avatarJiri Pirko <jiri@nvidia.com>
Signed-off-by: default avatarPetr Machata <petrm@nvidia.com>
Reviewed-by: default avatarJacob Keller <jacob.e.keller@intel.com>
Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 194ab947

drivers/net/ethernet/mellanox/mlxsw/core.c

+1 −18
Original line number Diff line number Diff line
@@ -1750,29 +1750,12 @@ static const struct devlink_ops mlxsw_devlink_ops = { 


static int mlxsw_core_params_register(struct mlxsw_core *mlxsw_core)

{

	int err;



	err = mlxsw_core_fw_params_register(mlxsw_core);

	if (err)

		return err;



	if (mlxsw_core->driver->params_register) {

		err = mlxsw_core->driver->params_register(mlxsw_core);

		if (err)

			goto err_params_register;

	}

	return 0;



err_params_register:

	mlxsw_core_fw_params_unregister(mlxsw_core);

	return err;

	return mlxsw_core_fw_params_register(mlxsw_core);

}



static void mlxsw_core_params_unregister(struct mlxsw_core *mlxsw_core)

{

	mlxsw_core_fw_params_unregister(mlxsw_core);

	if (mlxsw_core->driver->params_register)

		mlxsw_core->driver->params_unregister(mlxsw_core);

}



struct mlxsw_core_health_event {

drivers/net/ethernet/mellanox/mlxsw/core.h

+0 −2
Original line number Diff line number Diff line
@@ -421,8 +421,6 @@ struct mlxsw_driver { 
			     const struct mlxsw_config_profile *profile,

			     u64 *p_single_size, u64 *p_double_size,

			     u64 *p_linear_size);

	int (*params_register)(struct mlxsw_core *mlxsw_core);

	void (*params_unregister)(struct mlxsw_core *mlxsw_core);



	/* Notify a driver that a timestamped packet was transmitted. Driver

	 * is responsible for freeing the passed-in SKB.

drivers/net/ethernet/mellanox/mlxsw/spectrum.c

+0 −52
Original line number Diff line number Diff line
@@ -3861,52 +3861,6 @@ static int mlxsw_sp_kvd_sizes_get(struct mlxsw_core *mlxsw_core, 
	return 0;

}



static int

mlxsw_sp_params_acl_region_rehash_intrvl_get(struct devlink *devlink, u32 id,

					     struct devlink_param_gset_ctx *ctx)

{

	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);

	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);



	ctx->val.vu32 = mlxsw_sp_acl_region_rehash_intrvl_get(mlxsw_sp);

	return 0;

}



static int

mlxsw_sp_params_acl_region_rehash_intrvl_set(struct devlink *devlink, u32 id,

					     struct devlink_param_gset_ctx *ctx)

{

	struct mlxsw_core *mlxsw_core = devlink_priv(devlink);

	struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);



	return mlxsw_sp_acl_region_rehash_intrvl_set(mlxsw_sp, ctx->val.vu32);

}



static const struct devlink_param mlxsw_sp2_devlink_params[] = {

	DEVLINK_PARAM_DRIVER(MLXSW_DEVLINK_PARAM_ID_ACL_REGION_REHASH_INTERVAL,

			     "acl_region_rehash_interval",

			     DEVLINK_PARAM_TYPE_U32,

			     BIT(DEVLINK_PARAM_CMODE_RUNTIME),

			     mlxsw_sp_params_acl_region_rehash_intrvl_get,

			     mlxsw_sp_params_acl_region_rehash_intrvl_set,

			     NULL),

};



static int mlxsw_sp2_params_register(struct mlxsw_core *mlxsw_core)

{

	struct devlink *devlink = priv_to_devlink(mlxsw_core);



	return devl_params_register(devlink, mlxsw_sp2_devlink_params,

				    ARRAY_SIZE(mlxsw_sp2_devlink_params));

}



static void mlxsw_sp2_params_unregister(struct mlxsw_core *mlxsw_core)

{

	devl_params_unregister(priv_to_devlink(mlxsw_core),

			       mlxsw_sp2_devlink_params,

			       ARRAY_SIZE(mlxsw_sp2_devlink_params));

}



static void mlxsw_sp_ptp_transmitted(struct mlxsw_core *mlxsw_core,

				     struct sk_buff *skb, u16 local_port)

{
@@ -3984,8 +3938,6 @@ static struct mlxsw_driver mlxsw_sp2_driver = { 
	.trap_policer_counter_get	= mlxsw_sp_trap_policer_counter_get,

	.txhdr_construct		= mlxsw_sp_txhdr_construct,

	.resources_register		= mlxsw_sp2_resources_register,

	.params_register		= mlxsw_sp2_params_register,

	.params_unregister		= mlxsw_sp2_params_unregister,

	.ptp_transmitted		= mlxsw_sp_ptp_transmitted,

	.txhdr_len			= MLXSW_TXHDR_LEN,

	.profile			= &mlxsw_sp2_config_profile,
@@ -4023,8 +3975,6 @@ static struct mlxsw_driver mlxsw_sp3_driver = { 
	.trap_policer_counter_get	= mlxsw_sp_trap_policer_counter_get,

	.txhdr_construct		= mlxsw_sp_txhdr_construct,

	.resources_register		= mlxsw_sp2_resources_register,

	.params_register		= mlxsw_sp2_params_register,

	.params_unregister		= mlxsw_sp2_params_unregister,

	.ptp_transmitted		= mlxsw_sp_ptp_transmitted,

	.txhdr_len			= MLXSW_TXHDR_LEN,

	.profile			= &mlxsw_sp2_config_profile,
@@ -4060,8 +4010,6 @@ static struct mlxsw_driver mlxsw_sp4_driver = { 
	.trap_policer_counter_get	= mlxsw_sp_trap_policer_counter_get,

	.txhdr_construct		= mlxsw_sp_txhdr_construct,

	.resources_register		= mlxsw_sp2_resources_register,

	.params_register		= mlxsw_sp2_params_register,

	.params_unregister		= mlxsw_sp2_params_unregister,

	.ptp_transmitted		= mlxsw_sp_ptp_transmitted,

	.txhdr_len			= MLXSW_TXHDR_LEN,

	.profile			= &mlxsw_sp4_config_profile,

drivers/net/ethernet/mellanox/mlxsw/spectrum.h

+1 −2
Original line number Diff line number Diff line
@@ -973,6 +973,7 @@ enum mlxsw_sp_acl_profile { 
};



struct mlxsw_afk *mlxsw_sp_acl_afk(struct mlxsw_sp_acl *acl);

struct mlxsw_sp_acl_tcam *mlxsw_sp_acl_to_tcam(struct mlxsw_sp_acl *acl);



int mlxsw_sp_acl_ruleset_bind(struct mlxsw_sp *mlxsw_sp,

			      struct mlxsw_sp_flow_block *block,
@@ -1096,8 +1097,6 @@ mlxsw_sp_acl_act_cookie_lookup(struct mlxsw_sp *mlxsw_sp, u32 cookie_index) 


int mlxsw_sp_acl_init(struct mlxsw_sp *mlxsw_sp);

void mlxsw_sp_acl_fini(struct mlxsw_sp *mlxsw_sp);

u32 mlxsw_sp_acl_region_rehash_intrvl_get(struct mlxsw_sp *mlxsw_sp);

int mlxsw_sp_acl_region_rehash_intrvl_set(struct mlxsw_sp *mlxsw_sp, u32 val);



struct mlxsw_sp_acl_mangle_action;

drivers/net/ethernet/mellanox/mlxsw/spectrum_acl.c

+5 −16
Original line number Diff line number Diff line
@@ -40,6 +40,11 @@ struct mlxsw_afk *mlxsw_sp_acl_afk(struct mlxsw_sp_acl *acl) 
	return acl->afk;

}



struct mlxsw_sp_acl_tcam *mlxsw_sp_acl_to_tcam(struct mlxsw_sp_acl *acl)

{

	return &acl->tcam;

}



struct mlxsw_sp_acl_ruleset_ht_key {

	struct mlxsw_sp_flow_block *block;

	u32 chain_index;
@@ -1099,22 +1104,6 @@ void mlxsw_sp_acl_fini(struct mlxsw_sp *mlxsw_sp) 
	kfree(acl);

}



u32 mlxsw_sp_acl_region_rehash_intrvl_get(struct mlxsw_sp *mlxsw_sp)

{

	struct mlxsw_sp_acl *acl = mlxsw_sp->acl;



	return mlxsw_sp_acl_tcam_vregion_rehash_intrvl_get(mlxsw_sp,

							   &acl->tcam);

}



int mlxsw_sp_acl_region_rehash_intrvl_set(struct mlxsw_sp *mlxsw_sp, u32 val)

{

	struct mlxsw_sp_acl *acl = mlxsw_sp->acl;



	return mlxsw_sp_acl_tcam_vregion_rehash_intrvl_set(mlxsw_sp,

							   &acl->tcam, val);

}



struct mlxsw_sp_acl_rulei_ops mlxsw_sp1_acl_rulei_ops = {

	.act_mangle_field = mlxsw_sp1_acl_rulei_act_mangle_field,

};