[SRU Bionic 1/3] RDMA/cma: Add missing locking to rdma_accept()
Thadeu Lima de Souza Cascardo
cascardo at canonical.com
Mon Oct 11 22:08:11 UTC 2021
From: Jason Gunthorpe <jgg at nvidia.com>
In almost all cases rdma_accept() is called under the handler_mutex by
ULPs from their handler callbacks. The one exception was ucma which did
not get the handler_mutex.
To improve the understand-ability of the locking scheme obtain the mutex
for ucma as well.
This improves how ucma works by allowing it to directly use handler_mutex
for some of its internal locking against the handler callbacks intead of
the global file->mut lock.
There does not seem to be a serious bug here, other than a DISCONNECT event
can be delivered concurrently with accept succeeding.
Link: https://lore.kernel.org/r/20200818120526.702120-7-leon@kernel.org
Signed-off-by: Leon Romanovsky <leonro at mellanox.com>
Signed-off-by: Jason Gunthorpe <jgg at nvidia.com>
(backported from commit d114c6feedfe0600c19b9f9479a4026354d1f7fd)
[cascardo: context adjustments because of missing commits
0cb15372a615a9835893f43e86ae45399eb63996 and
00313983cda6f37f747058e58c1cb8fba02bc134]
CVE-2020-36385
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>
---
drivers/infiniband/core/cma.c | 25 ++++++++++++++++++++++---
drivers/infiniband/core/ucma.c | 12 ++++++++----
include/rdma/rdma_cm.h | 6 ++++++
3 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 4f0eaac2eb6f..e9a3ad39c73a 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -3800,14 +3800,15 @@ static int cma_send_sidr_rep(struct rdma_id_private *id_priv,
int rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param)
{
- struct rdma_id_private *id_priv;
+ struct rdma_id_private *id_priv =
+ container_of(id, struct rdma_id_private, id);
int ret;
- id_priv = container_of(id, struct rdma_id_private, id);
+ lockdep_assert_held(&id_priv->handler_mutex);
id_priv->owner = task_pid_nr(current);
- if (!cma_comp(id_priv, RDMA_CM_CONNECT))
+ if (READ_ONCE(id_priv->state) != RDMA_CM_CONNECT)
return -EINVAL;
if (!id->qp && conn_param) {
@@ -3847,6 +3848,24 @@ int rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param)
}
EXPORT_SYMBOL(rdma_accept);
+void rdma_lock_handler(struct rdma_cm_id *id)
+{
+ struct rdma_id_private *id_priv =
+ container_of(id, struct rdma_id_private, id);
+
+ mutex_lock(&id_priv->handler_mutex);
+}
+EXPORT_SYMBOL(rdma_lock_handler);
+
+void rdma_unlock_handler(struct rdma_cm_id *id)
+{
+ struct rdma_id_private *id_priv =
+ container_of(id, struct rdma_id_private, id);
+
+ mutex_unlock(&id_priv->handler_mutex);
+}
+EXPORT_SYMBOL(rdma_unlock_handler);
+
int rdma_notify(struct rdma_cm_id *id, enum ib_event_type event)
{
struct rdma_id_private *id_priv;
diff --git a/drivers/infiniband/core/ucma.c b/drivers/infiniband/core/ucma.c
index 27d30662a363..42bf8d2ac215 100644
--- a/drivers/infiniband/core/ucma.c
+++ b/drivers/infiniband/core/ucma.c
@@ -1119,16 +1119,20 @@ static ssize_t ucma_accept(struct ucma_file *file, const char __user *inbuf,
if (cmd.conn_param.valid) {
ucma_copy_conn_param(ctx->cm_id, &conn_param, &cmd.conn_param);
- mutex_lock(&file->mut);
mutex_lock(&ctx->mutex);
+ rdma_lock_handler(ctx->cm_id);
ret = rdma_accept(ctx->cm_id, &conn_param);
- mutex_unlock(&ctx->mutex);
- if (!ret)
+ if (!ret) {
+ /* The uid must be set atomically with the handler */
ctx->uid = cmd.uid;
- mutex_unlock(&file->mut);
+ }
+ rdma_unlock_handler(ctx->cm_id);
+ mutex_unlock(&ctx->mutex);
} else {
mutex_lock(&ctx->mutex);
+ rdma_lock_handler(ctx->cm_id);
ret = rdma_accept(ctx->cm_id, NULL);
+ rdma_unlock_handler(ctx->cm_id);
mutex_unlock(&ctx->mutex);
}
ucma_put_ctx(ctx);
diff --git a/include/rdma/rdma_cm.h b/include/rdma/rdma_cm.h
index 3d2eed3c4e75..ace66d7254b2 100644
--- a/include/rdma/rdma_cm.h
+++ b/include/rdma/rdma_cm.h
@@ -284,6 +284,9 @@ int rdma_connect(struct rdma_cm_id *id, struct rdma_conn_param *conn_param);
*/
int rdma_listen(struct rdma_cm_id *id, int backlog);
+void rdma_lock_handler(struct rdma_cm_id *id);
+void rdma_unlock_handler(struct rdma_cm_id *id);
+
/**
* rdma_accept - Called to accept a connection request or response.
* @id: Connection identifier associated with the request.
@@ -298,6 +301,9 @@ int rdma_listen(struct rdma_cm_id *id, int backlog);
* In the case of error, a reject message is sent to the remote side and the
* state of the qp associated with the id is modified to error, such that any
* previously posted receive buffers would be flushed.
+ *
+ * This function is for use by kernel ULPs and must be called from under the
+ * handler callback.
*/
int rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param);
--
2.30.2
More information about the kernel-team
mailing list