From 554fe75c1b3f679b1eebf193a4e56492837d3f5a Mon Sep 17 00:00:00 2001 From: Dmytro Linkin Date: Thu, 31 Oct 2019 18:15:51 +0200 Subject: [PATCH] net/mlx5e: Avoid duplicating rule destinations Following scenario easily break driver logic and crash the kernel: 1. Add rule with mirred actions to same device. 2. Delete this rule. In described scenario rule is not added to database and on deletion driver access invalid entry. Example: $ tc filter add dev ens1f0_0 ingress protocol ip prio 1 \ flower skip_sw \ action mirred egress mirror dev ens1f0_1 pipe \ action mirred egress redirect dev ens1f0_1 $ tc filter del dev ens1f0_0 ingress protocol ip prio 1 Dmesg output: [ 376.634396] mlx5_core 0000:82:00.0: mlx5_cmd_check:756:(pid 3439): DESTROY_FLOW_GROUP(0x934) op_mod(0x0) failed, status bad resource state(0x9), syndrome (0x563e2f) [ 376.654983] mlx5_core 0000:82:00.0: del_hw_flow_group:567:(pid 3439): flow steering can't destroy fg 89 of ft 3145728 [ 376.673433] kasan: CONFIG_KASAN_INLINE enabled [ 376.683769] kasan: GPF could be caused by NULL-ptr deref or user memory access [ 376.695229] general protection fault: 0000 [#1] PREEMPT SMP KASAN PTI [ 376.705069] CPU: 7 PID: 3439 Comm: tc Not tainted 5.4.0-rc5+ #76 [ 376.714959] Hardware name: Supermicro SYS-2028TP-DECTR/X10DRT-PT, BIOS 2.0a 08/12/2016 [ 376.726371] RIP: 0010:mlx5_del_flow_rules+0x105/0x960 [mlx5_core] [ 376.735817] Code: 01 00 00 00 48 83 eb 08 e8 28 d9 ff ff 4c 39 e3 75 d8 4c 8d bd c0 02 00 00 48 b8 00 00 00 00 00 fc ff df 4c 89 fa 48 c1 ea 03 <0f> b6 04 02 84 c0 74 08 3c 03 0f 8e 84 04 00 00 48 8d 7d 28 8b 9 d [ 376.761261] RSP: 0018:ffff888847c56db8 EFLAGS: 00010202 [ 376.770054] RAX: dffffc0000000000 RBX: ffff8888582a6da0 RCX: ffff888847c56d60 [ 376.780743] RDX: 0000000000000058 RSI: 0000000000000008 RDI: 0000000000000282 [ 376.791328] RBP: 0000000000000000 R08: fffffbfff0c60ea6 R09: fffffbfff0c60ea6 [ 376.802050] R10: fffffbfff0c60ea5 R11: ffffffff8630752f R12: ffff8888582a6da0 [ 376.812798] R13: dffffc0000000000 R14: ffff8888582a6da0 R15: 00000000000002c0 [ 376.823445] FS: 00007f675f9a8840(0000) GS:ffff88886d200000(0000) knlGS:0000000000000000 [ 376.834971] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 376.844179] CR2: 00000000007d9640 CR3: 00000007d3f26003 CR4: 00000000001606e0 [ 376.854843] Call Trace: [ 376.868542] __mlx5_eswitch_del_rule+0x49/0x300 [mlx5_core] [ 376.877735] mlx5e_tc_del_fdb_flow+0x6ec/0x9e0 [mlx5_core] [ 376.921549] mlx5e_flow_put+0x2b/0x50 [mlx5_core] [ 376.929813] mlx5e_delete_flower+0x5b6/0xbd0 [mlx5_core] [ 376.973030] tc_setup_cb_reoffload+0x29/0xc0 [ 376.980619] fl_reoffload+0x50a/0x770 [cls_flower] [ 377.015087] tcf_block_playback_offloads+0xbd/0x250 [ 377.033400] tcf_block_setup+0x1b2/0xc60 [ 377.057247] tcf_block_offload_cmd+0x195/0x240 [ 377.098826] tcf_block_offload_unbind+0xe7/0x180 [ 377.107056] __tcf_block_put+0xe5/0x400 [ 377.114528] ingress_destroy+0x3d/0x60 [sch_ingress] [ 377.122894] qdisc_destroy+0xf1/0x5a0 [ 377.129993] qdisc_graft+0xa3d/0xe50 [ 377.151227] tc_get_qdisc+0x48e/0xa20 [ 377.165167] rtnetlink_rcv_msg+0x35d/0x8d0 [ 377.199528] netlink_rcv_skb+0x11e/0x340 [ 377.219638] netlink_unicast+0x408/0x5b0 [ 377.239913] netlink_sendmsg+0x71b/0xb30 [ 377.267505] sock_sendmsg+0xb1/0xf0 [ 377.273801] ___sys_sendmsg+0x635/0x900 [ 377.312784] __sys_sendmsg+0xd3/0x170 [ 377.338693] do_syscall_64+0x95/0x460 [ 377.344833] entry_SYSCALL_64_after_hwframe+0x49/0xbe [ 377.352321] RIP: 0033:0x7f675e58e090 To avoid this, for every mirred action check if output device was already processed. If so - drop rule with EOPNOTSUPP error. Signed-off-by: Dmytro Linkin Reviewed-by: Roi Dayan Reviewed-by: Vlad Buslov Signed-off-by: Saeed Mahameed --- drivers/net/ethernet/mellanox/mlx5/core/en_tc.c | 58 ++++++++++++++++++++++++- 1 file changed, 57 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c index 9b32a9c0f497..fe83886f5435 100644 --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c @@ -2999,6 +2999,25 @@ static struct ip_tunnel_info *dup_tun_info(const struct ip_tunnel_info *tun_info return kmemdup(tun_info, tun_size, GFP_KERNEL); } +static bool is_duplicated_encap_entry(struct mlx5e_priv *priv, + struct mlx5e_tc_flow *flow, + int out_index, + struct mlx5e_encap_entry *e, + struct netlink_ext_ack *extack) +{ + int i; + + for (i = 0; i < out_index; i++) { + if (flow->encaps[i].e != e) + continue; + NL_SET_ERR_MSG_MOD(extack, "can't duplicate encap action"); + netdev_err(priv->netdev, "can't duplicate encap action\n"); + return true; + } + + return false; +} + static int mlx5e_attach_encap(struct mlx5e_priv *priv, struct mlx5e_tc_flow *flow, struct net_device *mirred_dev, @@ -3034,6 +3053,12 @@ static int mlx5e_attach_encap(struct mlx5e_priv *priv, /* must verify if encap is valid or not */ if (e) { + /* Check that entry was not already attached to this flow */ + if (is_duplicated_encap_entry(priv, flow, out_index, e, extack)) { + err = -EOPNOTSUPP; + goto out_err; + } + mutex_unlock(&esw->offloads.encap_tbl_lock); wait_for_completion(&e->res_ready); @@ -3220,6 +3245,26 @@ bool mlx5e_is_valid_eswitch_fwd_dev(struct mlx5e_priv *priv, same_hw_devs(priv, netdev_priv(out_dev)); } +static bool is_duplicated_output_device(struct net_device *dev, + struct net_device *out_dev, + int *ifindexes, int if_count, + struct netlink_ext_ack *extack) +{ + int i; + + for (i = 0; i < if_count; i++) { + if (ifindexes[i] == out_dev->ifindex) { + NL_SET_ERR_MSG_MOD(extack, + "can't duplicate output to same device"); + netdev_err(dev, "can't duplicate output to same device: %s\n", + out_dev->name); + return true; + } + } + + return false; +} + static int parse_tc_fdb_actions(struct mlx5e_priv *priv, struct flow_action *flow_action, struct mlx5e_tc_flow *flow, @@ -3231,11 +3276,12 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv, struct mlx5e_tc_flow_parse_attr *parse_attr = attr->parse_attr; struct mlx5e_rep_priv *rpriv = priv->ppriv; const struct ip_tunnel_info *info = NULL; + int ifindexes[MLX5_MAX_FLOW_FWD_VPORTS]; bool ft_flow = mlx5e_is_ft_flow(flow); const struct flow_action_entry *act; + int err, i, if_count = 0; bool encap = false; u32 action = 0; - int err, i; if (!flow_action_has_entries(flow_action)) return -EINVAL; @@ -3312,6 +3358,16 @@ static int parse_tc_fdb_actions(struct mlx5e_priv *priv, struct net_device *uplink_dev = mlx5_eswitch_uplink_get_proto_dev(esw, REP_ETH); struct net_device *uplink_upper; + if (is_duplicated_output_device(priv->netdev, + out_dev, + ifindexes, + if_count, + extack)) + return -EOPNOTSUPP; + + ifindexes[if_count] = out_dev->ifindex; + if_count++; + rcu_read_lock(); uplink_upper = netdev_master_upper_dev_get_rcu(uplink_dev); -- 2.11.0