From patchwork Mon Oct 11 22:08:14 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Thadeu Lima de Souza Cascardo X-Patchwork-Id: 1539521 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=canonical.com header.i=@canonical.com header.a=rsa-sha256 header.s=20210705 header.b=bH14ZBuy; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4HStH450DQz9sP7 for ; Tue, 12 Oct 2021 09:08:48 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1ma3TM-0003td-4F; Mon, 11 Oct 2021 22:08:40 +0000 Received: from smtp-relay-canonical-1.internal ([10.131.114.174] helo=smtp-relay-canonical-1.canonical.com) by huckleberry.canonical.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1ma3TJ-0003ru-5j for kernel-team@lists.ubuntu.com; Mon, 11 Oct 2021 22:08:37 +0000 Received: from mussarela.. (unknown [177.9.89.30]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-canonical-1.canonical.com (Postfix) with ESMTPSA id 6ECF43F0B8; Mon, 11 Oct 2021 22:08:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1633990116; bh=EDtUUDMwKLcAabzTZH3yl7OfGQ6yH0bEoT7BcKt7eow=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=bH14ZBuygNTFUHT1Wka2/Pnm3u4dkdRFqhXt+pSNs/pRsYt+GOJZnNPoc7LVvsFgs 8iIEgCUVEYcwtCZCdI7gkCsUCIP7qjD+al7W6Obdx8y06X8bpLY9Uu1W/YLIQK7GFb oMb+qH+igszLP+yczTtiJ+6Jf8P5ztGAIcqBGc2gGN7R7kW6EF0YxNfwBO1i4yQA6Y kc1lutN1w1CS3BmJkrnIU+4zXqKSQC66fZqwj793YOPSP3NmnReKgqxMfcZHMSUlgJ N7mzzd/OC6pCxHE0gY2CM1A3JsQJvniEy9seP1s8042WYEz0XGG2eXErB5bByZPZzo ShvCxNzdZ/17A== From: Thadeu Lima de Souza Cascardo To: kernel-team@lists.ubuntu.com Subject: [SRU Focal 1/3] RDMA/cma: Add missing locking to rdma_accept() Date: Mon, 11 Oct 2021 19:08:14 -0300 Message-Id: <20211011220819.582548-5-cascardo@canonical.com> X-Mailer: git-send-email 2.30.2 In-Reply-To: <20211011220819.582548-1-cascardo@canonical.com> References: <20211011220819.582548-1-cascardo@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Leon Romanovsky , Jason Gunthorpe , Thadeu Lima de Souza Cascardo Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Jason Gunthorpe 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 Signed-off-by: Jason Gunthorpe (backported from commit d114c6feedfe0600c19b9f9479a4026354d1f7fd) [cascardo: context adjustments because of missing commit 0cb15372a615a9835893f43e86ae45399eb63996] CVE-2020-36385 Signed-off-by: Thadeu Lima de Souza Cascardo --- 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 ec9e9598894f..0c2b4a9d7ee4 100644 --- a/drivers/infiniband/core/cma.c +++ b/drivers/infiniband/core/cma.c @@ -4051,14 +4051,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, const char *caller) { - 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); rdma_restrack_set_task(&id_priv->res, caller); - if (!cma_comp(id_priv, RDMA_CM_CONNECT)) + if (READ_ONCE(id_priv->state) != RDMA_CM_CONNECT) return -EINVAL; if (!id->qp && conn_param) { @@ -4098,6 +4099,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 ef4be14af3bb..7ecb094dc7ad 100644 --- a/drivers/infiniband/core/ucma.c +++ b/drivers/infiniband/core/ucma.c @@ -1133,16 +1133,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, NULL); - 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, 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 71f48cfdc24c..19c5fa24607f 100644 --- a/include/rdma/rdma_cm.h +++ b/include/rdma/rdma_cm.h @@ -276,6 +276,9 @@ int rdma_listen(struct rdma_cm_id *id, int backlog); int __rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param, const char *caller); +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. @@ -290,6 +293,9 @@ int __rdma_accept(struct rdma_cm_id *id, struct rdma_conn_param *conn_param, * 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. */ #define rdma_accept(id, conn_param) \ __rdma_accept((id), (conn_param), KBUILD_MODNAME)