[bug report] net/mlx5e: Extend encap entry with reference counter

[Dan Carpenter <dan.carpenter@xxxxxxxxxx>]

[ I already wrote this email, but it looks like I deleted it instead of
  sending it.  So weird.  I hopefully don't send it twice! ]

Hi Vlad,

I noticed a possible refcounting bug in commit 948993f2beeb ("net/mlx5e:
Extend encap entry with reference counter") from Jun 3, 2018.

	drivers/net/ethernet/mellanox/mlx5/core/en_tc.c:1435 mlx5e_tc_update_neigh_used_value()
	error: dereferencing freed memory 'e'

drivers/net/ethernet/mellanox/mlx5/core/en_tc.c

  1415  void mlx5e_tc_update_neigh_used_value(struct mlx5e_neigh_hash_entry *nhe)
  1416  {
  1417          struct mlx5e_neigh *m_neigh = &nhe->m_neigh;
  1418          struct mlx5e_tc_flow *flow;
  1419          struct mlx5e_encap_entry *e;
  1420          struct mlx5_fc *counter;
  1421          struct neigh_table *tbl;
  1422          bool neigh_used = false;
  1423          struct neighbour *n;
  1424          u64 lastuse;
  1425  
  1426          if (m_neigh->family == AF_INET)
  1427                  tbl = &arp_tbl;
  1428  #if IS_ENABLED(CONFIG_IPV6)
  1429          else if (m_neigh->family == AF_INET6)
  1430                  tbl = &nd_tbl;
  1431  #endif
  1432          else
  1433                  return;
  1434  
  1435          list_for_each_entry_safe(e, tmp, &nhe->encap_list, encap_list) {
  1436                  struct encap_flow_item *efi, *tmp;
  1437  
  1438                  if (!(e->flags & MLX5_ENCAP_ENTRY_VALID) ||
  1439                      !mlx5e_encap_take(e))
                            ^^^^^^^^^^^^^^^^^^^
We take a reference here.

  1440                          continue;
  1441  
  1442                  list_for_each_entry_safe(efi, tmp, &e->flows, list) {
  1443                          flow = container_of(efi, struct mlx5e_tc_flow,
  1444                                              encaps[efi->index]);
  1445                          if (IS_ERR(mlx5e_flow_get(flow)))
  1446                                  continue;
  1447  
  1448                          if (mlx5e_is_offloaded_flow(flow)) {
  1449                                  counter = mlx5e_tc_get_counter(flow);
  1450                                  lastuse = mlx5_fc_query_lastuse(counter);
  1451                                  if (time_after((unsigned long)lastuse, nhe->reported_lastuse)) {
  1452                                          mlx5e_flow_put(netdev_priv(e->out_dev), flow);
  1453                                          neigh_used = true;
  1454                                          break;

I think we need to call mlx5e_encap_put(netdev_priv(e->out_dev), e);
before this break;

  1455                                  }
  1456                          }
  1457  
  1458                          mlx5e_flow_put(netdev_priv(e->out_dev), flow);
  1459                  }
  1460  
  1461                  mlx5e_encap_put(netdev_priv(e->out_dev), e);
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  1462                  if (neigh_used)
  1463                          break;
  1464          }
  1465  

regards,
dan carpenter