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?
3326 }
3327
3328 spin_lock_irq(&cm_id_priv->lock);
3329 cm_init_av_for_lap(work->port, work->mad_recv_wc->wc,
3330 &ah_attr, &cm_id_priv->av);
3331 cm_move_av_from_path(&cm_id_priv->alt_av, &alt_av);
3332
3333 if (cm_id_priv->id.state != IB_CM_ESTABLISHED)
3334 goto unlock;
3335
3336 switch (cm_id_priv->id.lap_state) {
3337 case IB_CM_LAP_UNINIT:
3338 case IB_CM_LAP_IDLE:
3339 break;
3340 case IB_CM_MRA_LAP_SENT:
3341 atomic_long_inc(&work->port->counters[CM_RECV_DUPLICATES]
3342 [CM_LAP_COUNTER]);
3343 msg = cm_alloc_response_msg_no_ah(work->port, work->mad_recv_wc);
3344 if (IS_ERR(msg))
3345 goto unlock;
3346
3347 cm_format_mra((struct cm_mra_msg *) msg->mad, cm_id_priv,
3348 CM_MSG_RESPONSE_OTHER,
3349 cm_id_priv->service_timeout,
3350 cm_id_priv->private_data,
3351 cm_id_priv->private_data_len);
3352 spin_unlock_irq(&cm_id_priv->lock);
3353
3354 if (cm_create_response_msg_ah(work->port, work->mad_recv_wc, msg) ||
3355 ib_post_send_mad(msg, NULL))
3356 cm_free_response_msg(msg);
3357 goto deref;
3358 case IB_CM_LAP_RCVD:
3359 atomic_long_inc(&work->port->counters[CM_RECV_DUPLICATES]
3360 [CM_LAP_COUNTER]);
3361 goto unlock;
3362 default:
3363 goto unlock;
3364 }
3365
3366 cm_id_priv->id.lap_state = IB_CM_LAP_RCVD;
3367 cm_id_priv->tid = lap_msg->hdr.tid;
3368 cm_queue_work_unlock(cm_id_priv, work);
3369 return 0;
3370
3371 unlock: spin_unlock_irq(&cm_id_priv->lock);
3372 deref: cm_deref_id(cm_id_priv);
--> 3373 return -EINVAL;
3374 }
regards,
dan carpenter
