net/mlx5e: Fix nullptr in mlx5e_hairpin_get_mdev()

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>

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
index 629a61e8022f05a79c89d18cac6613a5d88818bb..d273758255c3a5e048e253444f7caf926d0ac69d 100644 (file)
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_tc.c
@@ -452,12 +452,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)
@@ -666,6 +686,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)) {
@@ -804,6 +828,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;