Re: [bug report] IB/cm: Improve the calling of cm_init_av_for_lap and cm_init_av_by_path

[Leon Romanovsky <leon@kernel.org>]

On Tue, Jan 18, 2022 at 12:16:43PM +0300, Dan Carpenter wrote:
> Hello Mark Zhang,
> 
> The patch 7345201c3963: "IB/cm: Improve the calling of
> cm_init_av_for_lap and cm_init_av_by_path" from Jun 2, 2021, leads to
> the following Smatch static checker warning:
> 
> drivers/infiniband/core/cm.c:3373 cm_lap_handler() warn: inconsistent refcounting 'cm_id_priv->refcount.refs.counter':
>   inc on: 3325
>   dec on: 3373
> 
> drivers/infiniband/core/cm.c
>     3278 static int cm_lap_handler(struct cm_work *work)
>     3279 {
>     3280         struct cm_id_private *cm_id_priv;
>     3281         struct cm_lap_msg *lap_msg;
>     3282         struct ib_cm_lap_event_param *param;
>     3283         struct ib_mad_send_buf *msg = NULL;
>     3284         struct rdma_ah_attr ah_attr;
>     3285         struct cm_av alt_av = {};
>     3286         int ret;
>     3287 
>     3288         /* Currently Alternate path messages are not supported for
>     3289          * RoCE link layer.
>     3290          */
>     3291         if (rdma_protocol_roce(work->port->cm_dev->ib_device,
>     3292                                work->port->port_num))
>     3293                 return -EINVAL;
>     3294 
>     3295         /* todo: verify LAP request and send reject APR if invalid. */
>     3296         lap_msg = (struct cm_lap_msg *)work->mad_recv_wc->recv_buf.mad;
>     3297         cm_id_priv = cm_acquire_id(
>     3298                 cpu_to_be32(IBA_GET(CM_LAP_REMOTE_COMM_ID, lap_msg)),
>     3299                 cpu_to_be32(IBA_GET(CM_LAP_LOCAL_COMM_ID, lap_msg)));
> 
> cm_acquire_id() bumps the refcount.
> 
>     3300         if (!cm_id_priv)
>     3301                 return -EINVAL;
>     3302 
>     3303         param = &work->cm_event.param.lap_rcvd;
>     3304         memset(&work->path[0], 0, sizeof(work->path[1]));
>     3305         cm_path_set_rec_type(work->port->cm_dev->ib_device,
>     3306                              work->port->port_num, &work->path[0],
>     3307                              IBA_GET_MEM_PTR(CM_LAP_ALTERNATE_LOCAL_PORT_GID,
>     3308                                              lap_msg));
>     3309         param->alternate_path = &work->path[0];
>     3310         cm_format_path_from_lap(cm_id_priv, param->alternate_path, lap_msg);
>     3311         work->cm_event.private_data =
>     3312                 IBA_GET_MEM_PTR(CM_LAP_PRIVATE_DATA, lap_msg);
>     3313 
>     3314         ret = ib_init_ah_attr_from_wc(work->port->cm_dev->ib_device,
>     3315                                       work->port->port_num,
>     3316                                       work->mad_recv_wc->wc,
>     3317                                       work->mad_recv_wc->recv_buf.grh,
>     3318                                       &ah_attr);
>     3319         if (ret)
>     3320                 goto deref;
>                          ^^^^^^^^^^^
> 
>     3321 
>     3322         ret = cm_init_av_by_path(param->alternate_path, NULL, &alt_av);
>     3323         if (ret) {
>     3324                 rdma_destroy_ah_attr(&ah_attr);
>     3325                 return -EINVAL;
> 
> Should this be goto deref as well?

Yes, it should.

Thanks