Skip to content

Commit 9b82054

Browse files
jgunthorpegregkh
authored andcommitted
IB/cm: Fix av cm device leak on an error path in cm_init_av_by_path()
[ Upstream commit 9b2207b ] Codex pointed out that cm_init_av_by_path() can call cm_set_av_port() which takes a reference on the cm device, but then can immediately return error if ib_init_ah_attr_from_path() fails. Since callers like ib_send_cm_req() put the av on the stack this leaks that cm device reference. Re-order cm_init_av_by_path() so it doesn't touch the av until it has done all its failable work, and then update the av in one shot so it is either left alone or fully init'd. Sashiko also pointed out that the cm_destroy_av() prior to cm_init_av_by_path() is harmful as it leaves the AV broken in the error case and thus the REJ won't send. Since cm_init_av_by_path() is now atomic it is safe to delete the cm_destroy_av(). On succees the av from cm_init_av_for_response() is cleaned up by cm_init_av_by_path(), on failure the 'goto rejected' guarentees the av is destroyed during ib_destroy_cm_id(). Fixes: 76039ac ("IB/cm: Protect cm_dev, cm_ports and mad_agent with kref and lock") Link: https://patch.msgid.link/r/0-v1-38292501f539+14f-ib_cm_av_leak_jgg@nvidia.com Signed-off-by: Jason Gunthorpe <jgg@nvidia.com> Signed-off-by: Sasha Levin <sashal@kernel.org>
1 parent c121473 commit 9b82054

1 file changed

Lines changed: 8 additions & 5 deletions

File tree

  • drivers/infiniband/core

drivers/infiniband/core/cm.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -514,6 +514,7 @@ static int cm_init_av_by_path(struct sa_path_rec *path,
514514
struct rdma_ah_attr new_ah_attr;
515515
struct cm_device *cm_dev;
516516
struct cm_port *port;
517+
u16 pkey_index;
517518
int ret;
518519

519520
port = get_cm_port_from_path(path, sgid_attr);
@@ -522,12 +523,10 @@ static int cm_init_av_by_path(struct sa_path_rec *path,
522523
cm_dev = port->cm_dev;
523524

524525
ret = ib_find_cached_pkey(cm_dev->ib_device, port->port_num,
525-
be16_to_cpu(path->pkey), &av->pkey_index);
526+
be16_to_cpu(path->pkey), &pkey_index);
526527
if (ret)
527528
return ret;
528529

529-
cm_set_av_port(av, port);
530-
531530
/*
532531
* av->ah_attr might be initialized based on wc or during
533532
* request processing time which might have reference to sgid_attr.
@@ -542,6 +541,8 @@ static int cm_init_av_by_path(struct sa_path_rec *path,
542541
if (ret)
543542
return ret;
544543

544+
av->pkey_index = pkey_index;
545+
cm_set_av_port(av, port);
545546
av->timeout = path->packet_life_time + 1;
546547
rdma_move_ah_attr(&av->ah_attr, &new_ah_attr);
547548
return 0;
@@ -2169,8 +2170,10 @@ static int cm_req_handler(struct cm_work *work)
21692170
cm_id_priv->av.ah_attr.roce.dmac);
21702171
work->path[0].hop_limit = grh->hop_limit;
21712172

2172-
/* This destroy call is needed to pair with cm_init_av_for_response */
2173-
cm_destroy_av(&cm_id_priv->av);
2173+
/*
2174+
* cm_init_av_by_path() will internally pair with the above
2175+
* cm_init_av_for_response() if it succeeds.
2176+
*/
21742177
ret = cm_init_av_by_path(&work->path[0], gid_attr, &cm_id_priv->av);
21752178
if (ret) {
21762179
int err;

0 commit comments

Comments
 (0)