Re: [PATCH for-rc] IB/mlx4: Fix leak in id_map_find_del

[Leon Romanovsky <leon@kernel.org>]

On Fri, Jan 17, 2020 at 02:56:22PM +0100, Håkon Bugge wrote:
> Using CX-3 virtual functions, either from a bare-metal machine or
> pass-through from a VM, MAD packets are proxied through the PF driver.
>
> Since the VF drivers have separate name spaces for MAD Transaction Ids
> (TIDs), the PF driver has to re-map the TIDs and keep the book keeping
> in a cache.
>
> Following the RDMA Connection Manager (CM) protocol, it is clear when
> an entry has to evicted from the cache. When a DREP is sent from
> mlx4_ib_multiplex_cm_handler(), id_map_find_del() is called. Similar
> when a REJ is received by the mlx4_ib_demux_cm_handler(),
> id_map_find_del() is called.
>
> This function wipes out the TID in use from the IDR or XArray and
> removes the id_map_entry from the table.
>
> In short, it does everything except the topping of the cake, which is
> to remove the entry from the list and free it. In other words, for the
> DREP and REJ cases enumerated above, both will leak one id_map_entry.
>
> Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
> ---
>  drivers/infiniband/hw/mlx4/cm.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/infiniband/hw/mlx4/cm.c b/drivers/infiniband/hw/mlx4/cm.c
> index ecd6cadd529a..1df6d3ccfc62 100644
> --- a/drivers/infiniband/hw/mlx4/cm.c
> +++ b/drivers/infiniband/hw/mlx4/cm.c
> @@ -197,8 +197,13 @@ static void id_map_find_del(struct ib_device *ibdev, int pv_cm_id)
>  	if (!ent)
>  		goto out;
>  	found_ent = id_map_find_by_sl_id(ibdev, ent->slave_id, ent->sl_cm_id);
> -	if (found_ent && found_ent == ent)
> +	if (found_ent && found_ent == ent) {
>  		rb_erase(&found_ent->node, sl_id_map);
> +		if (!ent->scheduled_delete) {

Why do we need to check scheduled_delete?

Isn't this to mark call to timeout cleanup (id_map_ent_timeout), which
can't race with id_map_find_del()? They both hold the same spinlock.

Thanks

> +			list_del(&ent->list);
> +			kfree(ent);
> +		}
> +	}
>  out:
>  	spin_unlock(&sriov->id_map_lock);
>  }
> --
> 2.20.1
>