linux-kernel - [PATCH v2] RDMA/erdma: Fix exception handling in erdma_accept

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <2e9ae1d6-4bbb-470f-957f-bb6ea2e0829e@web.de>
Date: Thu, 13 Mar 2025 13:10:21 +0100
From: Markus Elfring <Markus.Elfring@....de>
To: linux-rdma@...r.kernel.org, Cheng Xu <chengyou@...ux.alibaba.com>,
 Jason Gunthorpe <jgg@...pe.ca>, Kai Shen <kaishen@...ux.alibaba.com>,
 Leon Romanovsky <leon@...nel.org>, Yang Li <yang.lee@...ux.alibaba.com>
Cc: LKML <linux-kernel@...r.kernel.org>, kernel-janitors@...r.kernel.org
Subject: [PATCH v2] RDMA/erdma: Fix exception handling in
 erdma_accept_newconn()

From: Markus Elfring <elfring@...rs.sourceforge.net>
Date: Thu, 13 Mar 2025 11:44:50 +0100

The label “error” was used to jump to another pointer check despite of
the detail in the implementation of the function “erdma_accept_newconn”
that it was determined already that corresponding variables contained
still null pointers.

1. Thus return directly if
   * the cep state is not the value “ERDMA_EPSTATE_LISTENING”
     or
   * a call of the function “erdma_cep_alloc” failed.

2. Use more appropriate labels instead.

3. Delete two questionable checks.

4. Omit extra initialisations (for the variables “new_cep”, “new_s” and “ret”)
   which became unnecessary with this refactoring.


This issue was detected by using the Coccinelle software.

Fixes: 920d93eac8b9 ("RDMA/erdma: Add connection management (CM) support")
Cc: stable@...r.kernel.org
Signed-off-by: Markus Elfring <elfring@...rs.sourceforge.net>
---

See also:
* https://lore.kernel.org/cocci/167179d0-e1ea-39a8-4143-949ad57294c2@linux.alibaba.com/
* https://lkml.org/lkml/2023/3/19/191


V2:
The change suggestion was rebased on source files of the software “Linux next-20250313”.
A corresponding implementation detail was improved by the commit 83437689249e6a17b25e27712fbee292e42e7855
("RDMA/erdma: Prevent use-after-free in erdma_accept_newconn()") on 2025-03-06.


 drivers/infiniband/hw/erdma/erdma_cm.c | 37 +++++++++++---------------
 1 file changed, 16 insertions(+), 21 deletions(-)

diff --git a/drivers/infiniband/hw/erdma/erdma_cm.c b/drivers/infiniband/hw/erdma/erdma_cm.c
index e0acc185e719..a7a79722e940 100644
--- a/drivers/infiniband/hw/erdma/erdma_cm.c
+++ b/drivers/infiniband/hw/erdma/erdma_cm.c
@@ -642,16 +642,16 @@ static int erdma_proc_mpareply(struct erdma_cep *cep)
 static void erdma_accept_newconn(struct erdma_cep *cep)
 {
 	struct socket *s = cep->sock;
-	struct socket *new_s = NULL;
-	struct erdma_cep *new_cep = NULL;
-	int ret = 0;
+	struct socket *new_s;
+	struct erdma_cep *new_cep;
+	int ret;

 	if (cep->state != ERDMA_EPSTATE_LISTENING)
-		goto error;
+		return;

 	new_cep = erdma_cep_alloc(cep->dev);
 	if (!new_cep)
-		goto error;
+		return;

 	/*
 	 * 4: Allocate a sufficient number of work elements
@@ -659,7 +659,7 @@ static void erdma_accept_newconn(struct erdma_cep *cep)
 	 * events, MPA header processing + MPA timeout.
 	 */
 	if (erdma_cm_alloc_work(new_cep, 4) != 0)
-		goto error;
+		goto put_cep;

 	/*
 	 * Copy saved socket callbacks from listening CEP
@@ -671,7 +671,7 @@ static void erdma_accept_newconn(struct erdma_cep *cep)

 	ret = kernel_accept(s, &new_s, O_NONBLOCK);
 	if (ret != 0)
-		goto error;
+		goto put_cep;

 	new_cep->sock = new_s;
 	erdma_cep_get(new_cep);
@@ -682,7 +682,7 @@ static void erdma_accept_newconn(struct erdma_cep *cep)

 	ret = erdma_cm_queue_work(new_cep, ERDMA_CM_WORK_MPATIMEOUT);
 	if (ret)
-		goto error;
+		goto disassoc_socket;

 	new_cep->listen_cep = cep;
 	erdma_cep_get(cep);
@@ -696,25 +696,20 @@ static void erdma_accept_newconn(struct erdma_cep *cep)
 			new_cep->listen_cep = NULL;
 			if (ret) {
 				erdma_cep_set_free(new_cep);
-				goto error;
+				goto disassoc_socket;
 			}
 		}
 		erdma_cep_set_free(new_cep);
 	}
 	return;

-error:
-	if (new_cep) {
-		new_cep->state = ERDMA_EPSTATE_CLOSED;
-		erdma_cancel_mpatimer(new_cep);
-
-		erdma_cep_put(new_cep);
-	}
-
-	if (new_s) {
-		erdma_socket_disassoc(new_s);
-		sock_release(new_s);
-	}
+disassoc_socket:
+	erdma_socket_disassoc(new_s);
+	sock_release(new_s);
+	new_cep->state = ERDMA_EPSTATE_CLOSED;
+	erdma_cancel_mpatimer(new_cep);
+put_cep:
+	erdma_cep_put(new_cep);
 }

 static int erdma_newconn_connected(struct erdma_cep *cep)
--
2.48.1