| From 941b1a55997b0264195f4be6389934fe3bce10e0 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Mon, 26 Apr 2021 15:16:26 +0300 |
| Subject: net/mlx5e: Fix nullptr in mlx5e_hairpin_get_mdev() |
| |
| From: Dima Chumak <dchumak@nvidia.com> |
| |
| [ Upstream commit b1c2f6312c5005c928a72e668bf305a589d828d4 ] |
| |
| The result of __dev_get_by_index() is not checked for NULL and then gets |
| dereferenced immediately. |
| |
| Also, __dev_get_by_index() must be called while holding either RTNL lock |
| or @dev_base_lock, which isn't satisfied by mlx5e_hairpin_get_mdev() or |
| its callers. This makes the underlying hlist_for_each_entry() loop not |
| safe, and can have adverse effects in itself. |
| |
| Fix by using dev_get_by_index() and handling nullptr return value when |
| ifindex device is not found. Update mlx5e_hairpin_get_mdev() callers to |
| check for possible PTR_ERR() result. |
| |
| Fixes: 77ab67b7f0f9 ("net/mlx5e: Basic setup of hairpin object") |
| Addresses-Coverity: ("Dereference null return value") |
| Signed-off-by: Dima Chumak <dchumak@nvidia.com> |
| Reviewed-by: Vlad Buslov <vladbu@nvidia.com> |
| Reviewed-by: Roi Dayan <roid@nvidia.com> |
| Signed-off-by: Saeed Mahameed <saeedm@nvidia.com> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| .../net/ethernet/mellanox/mlx5/core/en_tc.c | 33 +++++++++++++++++-- |
| 1 file changed, 31 insertions(+), 2 deletions(-) |
| |
| diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c |
| index 59837af959d0..1ad1692a5b2d 100644 |
| --- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c |
| +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c |
| @@ -481,12 +481,32 @@ static void mlx5e_detach_mod_hdr(struct mlx5e_priv *priv, |
| static |
| struct mlx5_core_dev *mlx5e_hairpin_get_mdev(struct net *net, int ifindex) |
| { |
| + struct mlx5_core_dev *mdev; |
| struct net_device *netdev; |
| struct mlx5e_priv *priv; |
| |
| - netdev = __dev_get_by_index(net, ifindex); |
| + netdev = dev_get_by_index(net, ifindex); |
| + if (!netdev) |
| + return ERR_PTR(-ENODEV); |
| + |
| priv = netdev_priv(netdev); |
| - return priv->mdev; |
| + mdev = priv->mdev; |
| + dev_put(netdev); |
| + |
| + /* Mirred tc action holds a refcount on the ifindex net_device (see |
| + * net/sched/act_mirred.c:tcf_mirred_get_dev). So, it's okay to continue using mdev |
| + * after dev_put(netdev), while we're in the context of adding a tc flow. |
| + * |
| + * The mdev pointer corresponds to the peer/out net_device of a hairpin. It is then |
| + * stored in a hairpin object, which exists until all flows, that refer to it, get |
| + * removed. |
| + * |
| + * On the other hand, after a hairpin object has been created, the peer net_device may |
| + * be removed/unbound while there are still some hairpin flows that are using it. This |
| + * case is handled by mlx5e_tc_hairpin_update_dead_peer, which is hooked to |
| + * NETDEV_UNREGISTER event of the peer net_device. |
| + */ |
| + return mdev; |
| } |
| |
| static int mlx5e_hairpin_create_transport(struct mlx5e_hairpin *hp) |
| @@ -685,6 +705,10 @@ mlx5e_hairpin_create(struct mlx5e_priv *priv, struct mlx5_hairpin_params *params |
| |
| func_mdev = priv->mdev; |
| peer_mdev = mlx5e_hairpin_get_mdev(dev_net(priv->netdev), peer_ifindex); |
| + if (IS_ERR(peer_mdev)) { |
| + err = PTR_ERR(peer_mdev); |
| + goto create_pair_err; |
| + } |
| |
| pair = mlx5_core_hairpin_create(func_mdev, peer_mdev, params); |
| if (IS_ERR(pair)) { |
| @@ -823,6 +847,11 @@ static int mlx5e_hairpin_flow_add(struct mlx5e_priv *priv, |
| int err; |
| |
| peer_mdev = mlx5e_hairpin_get_mdev(dev_net(priv->netdev), peer_ifindex); |
| + if (IS_ERR(peer_mdev)) { |
| + NL_SET_ERR_MSG_MOD(extack, "invalid ifindex of mirred device"); |
| + return PTR_ERR(peer_mdev); |
| + } |
| + |
| if (!MLX5_CAP_GEN(priv->mdev, hairpin) || !MLX5_CAP_GEN(peer_mdev, hairpin)) { |
| NL_SET_ERR_MSG_MOD(extack, "hairpin is not supported"); |
| return -EOPNOTSUPP; |
| -- |
| 2.30.2 |
| |