Commit abc4ea7f authored by Jason Gunthorpe's avatar Jason Gunthorpe Committed by Greg Kroah-Hartman

RDMA/ucma: Put a lock around every call to the rdma_cm layer

commit 7c119107 upstream.

The rdma_cm must be used single threaded.

This appears to be a bug in the design, as it does have lots of locking
that seems like it should allow concurrency. However, when it is all said
and done every single place that uses the cma_exch() scheme is broken, and
all the unlocked reads from the ucma of the cm_id data are wrong too.

syzkaller has been finding endless bugs related to this.

Fixing this in any elegant way is some enormous amount of work. Take a
very big hammer and put a mutex around everything to do with the
ucma_context at the top of every syscall.

Fixes: 75216638 ("RDMA/cma: Export rdma cm interface to userspace")
Link: https://lore.kernel.org/r/20200218210432.GA31966@ziepe.ca
Reported-by: syzbot+adb15cf8c2798e4e0db4@syzkaller.appspotmail.com
Reported-by: syzbot+e5579222b6a3edd96522@syzkaller.appspotmail.com
Reported-by: syzbot+4b628fcc748474003457@syzkaller.appspotmail.com
Reported-by: syzbot+29ee8f76017ce6cf03da@syzkaller.appspotmail.com
Reported-by: syzbot+6956235342b7317ec564@syzkaller.appspotmail.com
Reported-by: syzbot+b358909d8d01556b790b@syzkaller.appspotmail.com
Reported-by: syzbot+6b46b135602a3f3ac99e@syzkaller.appspotmail.com
Reported-by: syzbot+8458d13b13562abf6b77@syzkaller.appspotmail.com
Reported-by: syzbot+bd034f3fdc0402e942ed@syzkaller.appspotmail.com
Reported-by: syzbot+c92378b32760a4eef756@syzkaller.appspotmail.com
Reported-by: syzbot+68b44a1597636e0b342c@syzkaller.appspotmail.com
Signed-off-by: default avatarJason Gunthorpe <jgg@mellanox.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 4eeddc62
...@@ -89,6 +89,7 @@ struct ucma_context { ...@@ -89,6 +89,7 @@ struct ucma_context {
struct ucma_file *file; struct ucma_file *file;
struct rdma_cm_id *cm_id; struct rdma_cm_id *cm_id;
struct mutex mutex;
u64 uid; u64 uid;
struct list_head list; struct list_head list;
...@@ -215,6 +216,7 @@ static struct ucma_context *ucma_alloc_ctx(struct ucma_file *file) ...@@ -215,6 +216,7 @@ static struct ucma_context *ucma_alloc_ctx(struct ucma_file *file)
init_completion(&ctx->comp); init_completion(&ctx->comp);
INIT_LIST_HEAD(&ctx->mc_list); INIT_LIST_HEAD(&ctx->mc_list);
ctx->file = file; ctx->file = file;
mutex_init(&ctx->mutex);
mutex_lock(&mut); mutex_lock(&mut);
ctx->id = idr_alloc(&ctx_idr, ctx, 0, 0, GFP_KERNEL); ctx->id = idr_alloc(&ctx_idr, ctx, 0, 0, GFP_KERNEL);
...@@ -596,6 +598,7 @@ static int ucma_free_ctx(struct ucma_context *ctx) ...@@ -596,6 +598,7 @@ static int ucma_free_ctx(struct ucma_context *ctx)
} }
events_reported = ctx->events_reported; events_reported = ctx->events_reported;
mutex_destroy(&ctx->mutex);
kfree(ctx); kfree(ctx);
return events_reported; return events_reported;
} }
...@@ -665,7 +668,10 @@ static ssize_t ucma_bind_ip(struct ucma_file *file, const char __user *inbuf, ...@@ -665,7 +668,10 @@ static ssize_t ucma_bind_ip(struct ucma_file *file, const char __user *inbuf,
if (IS_ERR(ctx)) if (IS_ERR(ctx))
return PTR_ERR(ctx); return PTR_ERR(ctx);
mutex_lock(&ctx->mutex);
ret = rdma_bind_addr(ctx->cm_id, (struct sockaddr *) &cmd.addr); ret = rdma_bind_addr(ctx->cm_id, (struct sockaddr *) &cmd.addr);
mutex_unlock(&ctx->mutex);
ucma_put_ctx(ctx); ucma_put_ctx(ctx);
return ret; return ret;
} }
...@@ -688,7 +694,9 @@ static ssize_t ucma_bind(struct ucma_file *file, const char __user *inbuf, ...@@ -688,7 +694,9 @@ static ssize_t ucma_bind(struct ucma_file *file, const char __user *inbuf,
if (IS_ERR(ctx)) if (IS_ERR(ctx))
return PTR_ERR(ctx); return PTR_ERR(ctx);
mutex_lock(&ctx->mutex);
ret = rdma_bind_addr(ctx->cm_id, (struct sockaddr *) &cmd.addr); ret = rdma_bind_addr(ctx->cm_id, (struct sockaddr *) &cmd.addr);
mutex_unlock(&ctx->mutex);
ucma_put_ctx(ctx); ucma_put_ctx(ctx);
return ret; return ret;
} }
...@@ -712,8 +720,10 @@ static ssize_t ucma_resolve_ip(struct ucma_file *file, ...@@ -712,8 +720,10 @@ static ssize_t ucma_resolve_ip(struct ucma_file *file,
if (IS_ERR(ctx)) if (IS_ERR(ctx))
return PTR_ERR(ctx); return PTR_ERR(ctx);
mutex_lock(&ctx->mutex);
ret = rdma_resolve_addr(ctx->cm_id, (struct sockaddr *) &cmd.src_addr, ret = rdma_resolve_addr(ctx->cm_id, (struct sockaddr *) &cmd.src_addr,
(struct sockaddr *) &cmd.dst_addr, cmd.timeout_ms); (struct sockaddr *) &cmd.dst_addr, cmd.timeout_ms);
mutex_unlock(&ctx->mutex);
ucma_put_ctx(ctx); ucma_put_ctx(ctx);
return ret; return ret;
} }
...@@ -738,8 +748,10 @@ static ssize_t ucma_resolve_addr(struct ucma_file *file, ...@@ -738,8 +748,10 @@ static ssize_t ucma_resolve_addr(struct ucma_file *file,
if (IS_ERR(ctx)) if (IS_ERR(ctx))
return PTR_ERR(ctx); return PTR_ERR(ctx);
mutex_lock(&ctx->mutex);
ret = rdma_resolve_addr(ctx->cm_id, (struct sockaddr *) &cmd.src_addr, ret = rdma_resolve_addr(ctx->cm_id, (struct sockaddr *) &cmd.src_addr,
(struct sockaddr *) &cmd.dst_addr, cmd.timeout_ms); (struct sockaddr *) &cmd.dst_addr, cmd.timeout_ms);
mutex_unlock(&ctx->mutex);
ucma_put_ctx(ctx); ucma_put_ctx(ctx);
return ret; return ret;
} }
...@@ -759,7 +771,9 @@ static ssize_t ucma_resolve_route(struct ucma_file *file, ...@@ -759,7 +771,9 @@ static ssize_t ucma_resolve_route(struct ucma_file *file,
if (IS_ERR(ctx)) if (IS_ERR(ctx))
return PTR_ERR(ctx); return PTR_ERR(ctx);
mutex_lock(&ctx->mutex);
ret = rdma_resolve_route(ctx->cm_id, cmd.timeout_ms); ret = rdma_resolve_route(ctx->cm_id, cmd.timeout_ms);
mutex_unlock(&ctx->mutex);
ucma_put_ctx(ctx); ucma_put_ctx(ctx);
return ret; return ret;
} }
...@@ -848,6 +862,7 @@ static ssize_t ucma_query_route(struct ucma_file *file, ...@@ -848,6 +862,7 @@ static ssize_t ucma_query_route(struct ucma_file *file,
if (IS_ERR(ctx)) if (IS_ERR(ctx))
return PTR_ERR(ctx); return PTR_ERR(ctx);
mutex_lock(&ctx->mutex);
memset(&resp, 0, sizeof resp); memset(&resp, 0, sizeof resp);
addr = (struct sockaddr *) &ctx->cm_id->route.addr.src_addr; addr = (struct sockaddr *) &ctx->cm_id->route.addr.src_addr;
memcpy(&resp.src_addr, addr, addr->sa_family == AF_INET ? memcpy(&resp.src_addr, addr, addr->sa_family == AF_INET ?
...@@ -871,6 +886,7 @@ static ssize_t ucma_query_route(struct ucma_file *file, ...@@ -871,6 +886,7 @@ static ssize_t ucma_query_route(struct ucma_file *file,
ucma_copy_iw_route(&resp, &ctx->cm_id->route); ucma_copy_iw_route(&resp, &ctx->cm_id->route);
out: out:
mutex_unlock(&ctx->mutex);
if (copy_to_user(u64_to_user_ptr(cmd.response), if (copy_to_user(u64_to_user_ptr(cmd.response),
&resp, sizeof(resp))) &resp, sizeof(resp)))
ret = -EFAULT; ret = -EFAULT;
...@@ -1022,6 +1038,7 @@ static ssize_t ucma_query(struct ucma_file *file, ...@@ -1022,6 +1038,7 @@ static ssize_t ucma_query(struct ucma_file *file,
if (IS_ERR(ctx)) if (IS_ERR(ctx))
return PTR_ERR(ctx); return PTR_ERR(ctx);
mutex_lock(&ctx->mutex);
switch (cmd.option) { switch (cmd.option) {
case RDMA_USER_CM_QUERY_ADDR: case RDMA_USER_CM_QUERY_ADDR:
ret = ucma_query_addr(ctx, response, out_len); ret = ucma_query_addr(ctx, response, out_len);
...@@ -1036,6 +1053,7 @@ static ssize_t ucma_query(struct ucma_file *file, ...@@ -1036,6 +1053,7 @@ static ssize_t ucma_query(struct ucma_file *file,
ret = -ENOSYS; ret = -ENOSYS;
break; break;
} }
mutex_unlock(&ctx->mutex);
ucma_put_ctx(ctx); ucma_put_ctx(ctx);
return ret; return ret;
...@@ -1076,7 +1094,9 @@ static ssize_t ucma_connect(struct ucma_file *file, const char __user *inbuf, ...@@ -1076,7 +1094,9 @@ static ssize_t ucma_connect(struct ucma_file *file, const char __user *inbuf,
return PTR_ERR(ctx); return PTR_ERR(ctx);
ucma_copy_conn_param(ctx->cm_id, &conn_param, &cmd.conn_param); ucma_copy_conn_param(ctx->cm_id, &conn_param, &cmd.conn_param);
mutex_lock(&ctx->mutex);
ret = rdma_connect(ctx->cm_id, &conn_param); ret = rdma_connect(ctx->cm_id, &conn_param);
mutex_unlock(&ctx->mutex);
ucma_put_ctx(ctx); ucma_put_ctx(ctx);
return ret; return ret;
} }
...@@ -1097,7 +1117,9 @@ static ssize_t ucma_listen(struct ucma_file *file, const char __user *inbuf, ...@@ -1097,7 +1117,9 @@ static ssize_t ucma_listen(struct ucma_file *file, const char __user *inbuf,
ctx->backlog = cmd.backlog > 0 && cmd.backlog < max_backlog ? ctx->backlog = cmd.backlog > 0 && cmd.backlog < max_backlog ?
cmd.backlog : max_backlog; cmd.backlog : max_backlog;
mutex_lock(&ctx->mutex);
ret = rdma_listen(ctx->cm_id, ctx->backlog); ret = rdma_listen(ctx->cm_id, ctx->backlog);
mutex_unlock(&ctx->mutex);
ucma_put_ctx(ctx); ucma_put_ctx(ctx);
return ret; return ret;
} }
...@@ -1120,13 +1142,17 @@ static ssize_t ucma_accept(struct ucma_file *file, const char __user *inbuf, ...@@ -1120,13 +1142,17 @@ static ssize_t ucma_accept(struct ucma_file *file, const char __user *inbuf,
if (cmd.conn_param.valid) { if (cmd.conn_param.valid) {
ucma_copy_conn_param(ctx->cm_id, &conn_param, &cmd.conn_param); ucma_copy_conn_param(ctx->cm_id, &conn_param, &cmd.conn_param);
mutex_lock(&file->mut); mutex_lock(&file->mut);
mutex_lock(&ctx->mutex);
ret = __rdma_accept(ctx->cm_id, &conn_param, NULL); ret = __rdma_accept(ctx->cm_id, &conn_param, NULL);
mutex_unlock(&ctx->mutex);
if (!ret) if (!ret)
ctx->uid = cmd.uid; ctx->uid = cmd.uid;
mutex_unlock(&file->mut); mutex_unlock(&file->mut);
} else } else {
mutex_lock(&ctx->mutex);
ret = __rdma_accept(ctx->cm_id, NULL, NULL); ret = __rdma_accept(ctx->cm_id, NULL, NULL);
mutex_unlock(&ctx->mutex);
}
ucma_put_ctx(ctx); ucma_put_ctx(ctx);
return ret; return ret;
} }
...@@ -1145,7 +1171,9 @@ static ssize_t ucma_reject(struct ucma_file *file, const char __user *inbuf, ...@@ -1145,7 +1171,9 @@ static ssize_t ucma_reject(struct ucma_file *file, const char __user *inbuf,
if (IS_ERR(ctx)) if (IS_ERR(ctx))
return PTR_ERR(ctx); return PTR_ERR(ctx);
mutex_lock(&ctx->mutex);
ret = rdma_reject(ctx->cm_id, cmd.private_data, cmd.private_data_len); ret = rdma_reject(ctx->cm_id, cmd.private_data, cmd.private_data_len);
mutex_unlock(&ctx->mutex);
ucma_put_ctx(ctx); ucma_put_ctx(ctx);
return ret; return ret;
} }
...@@ -1164,7 +1192,9 @@ static ssize_t ucma_disconnect(struct ucma_file *file, const char __user *inbuf, ...@@ -1164,7 +1192,9 @@ static ssize_t ucma_disconnect(struct ucma_file *file, const char __user *inbuf,
if (IS_ERR(ctx)) if (IS_ERR(ctx))
return PTR_ERR(ctx); return PTR_ERR(ctx);
mutex_lock(&ctx->mutex);
ret = rdma_disconnect(ctx->cm_id); ret = rdma_disconnect(ctx->cm_id);
mutex_unlock(&ctx->mutex);
ucma_put_ctx(ctx); ucma_put_ctx(ctx);
return ret; return ret;
} }
...@@ -1195,7 +1225,9 @@ static ssize_t ucma_init_qp_attr(struct ucma_file *file, ...@@ -1195,7 +1225,9 @@ static ssize_t ucma_init_qp_attr(struct ucma_file *file,
resp.qp_attr_mask = 0; resp.qp_attr_mask = 0;
memset(&qp_attr, 0, sizeof qp_attr); memset(&qp_attr, 0, sizeof qp_attr);
qp_attr.qp_state = cmd.qp_state; qp_attr.qp_state = cmd.qp_state;
mutex_lock(&ctx->mutex);
ret = rdma_init_qp_attr(ctx->cm_id, &qp_attr, &resp.qp_attr_mask); ret = rdma_init_qp_attr(ctx->cm_id, &qp_attr, &resp.qp_attr_mask);
mutex_unlock(&ctx->mutex);
if (ret) if (ret)
goto out; goto out;
...@@ -1274,9 +1306,13 @@ static int ucma_set_ib_path(struct ucma_context *ctx, ...@@ -1274,9 +1306,13 @@ static int ucma_set_ib_path(struct ucma_context *ctx,
struct sa_path_rec opa; struct sa_path_rec opa;
sa_convert_path_ib_to_opa(&opa, &sa_path); sa_convert_path_ib_to_opa(&opa, &sa_path);
mutex_lock(&ctx->mutex);
ret = rdma_set_ib_path(ctx->cm_id, &opa); ret = rdma_set_ib_path(ctx->cm_id, &opa);
mutex_unlock(&ctx->mutex);
} else { } else {
mutex_lock(&ctx->mutex);
ret = rdma_set_ib_path(ctx->cm_id, &sa_path); ret = rdma_set_ib_path(ctx->cm_id, &sa_path);
mutex_unlock(&ctx->mutex);
} }
if (ret) if (ret)
return ret; return ret;
...@@ -1309,7 +1345,9 @@ static int ucma_set_option_level(struct ucma_context *ctx, int level, ...@@ -1309,7 +1345,9 @@ static int ucma_set_option_level(struct ucma_context *ctx, int level,
switch (level) { switch (level) {
case RDMA_OPTION_ID: case RDMA_OPTION_ID:
mutex_lock(&ctx->mutex);
ret = ucma_set_option_id(ctx, optname, optval, optlen); ret = ucma_set_option_id(ctx, optname, optval, optlen);
mutex_unlock(&ctx->mutex);
break; break;
case RDMA_OPTION_IB: case RDMA_OPTION_IB:
ret = ucma_set_option_ib(ctx, optname, optval, optlen); ret = ucma_set_option_ib(ctx, optname, optval, optlen);
...@@ -1369,8 +1407,10 @@ static ssize_t ucma_notify(struct ucma_file *file, const char __user *inbuf, ...@@ -1369,8 +1407,10 @@ static ssize_t ucma_notify(struct ucma_file *file, const char __user *inbuf,
if (IS_ERR(ctx)) if (IS_ERR(ctx))
return PTR_ERR(ctx); return PTR_ERR(ctx);
mutex_lock(&ctx->mutex);
if (ctx->cm_id->device) if (ctx->cm_id->device)
ret = rdma_notify(ctx->cm_id, (enum ib_event_type)cmd.event); ret = rdma_notify(ctx->cm_id, (enum ib_event_type)cmd.event);
mutex_unlock(&ctx->mutex);
ucma_put_ctx(ctx); ucma_put_ctx(ctx);
return ret; return ret;
...@@ -1413,8 +1453,10 @@ static ssize_t ucma_process_join(struct ucma_file *file, ...@@ -1413,8 +1453,10 @@ static ssize_t ucma_process_join(struct ucma_file *file,
mc->join_state = join_state; mc->join_state = join_state;
mc->uid = cmd->uid; mc->uid = cmd->uid;
memcpy(&mc->addr, addr, cmd->addr_size); memcpy(&mc->addr, addr, cmd->addr_size);
mutex_lock(&ctx->mutex);
ret = rdma_join_multicast(ctx->cm_id, (struct sockaddr *)&mc->addr, ret = rdma_join_multicast(ctx->cm_id, (struct sockaddr *)&mc->addr,
join_state, mc); join_state, mc);
mutex_unlock(&ctx->mutex);
if (ret) if (ret)
goto err2; goto err2;
...@@ -1518,7 +1560,10 @@ static ssize_t ucma_leave_multicast(struct ucma_file *file, ...@@ -1518,7 +1560,10 @@ static ssize_t ucma_leave_multicast(struct ucma_file *file,
goto out; goto out;
} }
mutex_lock(&mc->ctx->mutex);
rdma_leave_multicast(mc->ctx->cm_id, (struct sockaddr *) &mc->addr); rdma_leave_multicast(mc->ctx->cm_id, (struct sockaddr *) &mc->addr);
mutex_unlock(&mc->ctx->mutex);
mutex_lock(&mc->ctx->file->mut); mutex_lock(&mc->ctx->file->mut);
ucma_cleanup_mc_events(mc); ucma_cleanup_mc_events(mc);
list_del(&mc->list); list_del(&mc->list);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment