diff mbox series

[rdma-next,1/3] RDMA/cma: Use correct address when leaving multicast group

Message ID 913bc6783fd7a95fe71ad9454e01653ee6fb4a9a.1642491047.git.leonro@nvidia.com (mailing list archive)
State Accepted
Delegated to: Jason Gunthorpe
Headers show
Series Various fixes in RDMA/core | expand

Commit Message

Leon Romanovsky Jan. 18, 2022, 7:35 a.m. UTC 
From: Maor Gottlieb <maorg@nvidia.com>

In RoCE we should use cma_iboe_set_mgid and not cma_set_mgid to generate
the mgid, otherwise we will try to remove incorrect address.

Fixes: b5de0c60cc30 ("RDMA/cma: Fix use after free race in roce multicast join")
Signed-off-by: Maor Gottlieb <maorg@nvidia.com>
Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
---
 drivers/infiniband/core/cma.c | 61 +++++++++++++++++------------------
 1 file changed, 30 insertions(+), 31 deletions(-)

Comments

Jason Gunthorpe Jan. 28, 2022, 3:35 p.m. UTC | #1 
On Tue, Jan 18, 2022 at 09:35:00AM +0200, Leon Romanovsky wrote:
> From: Maor Gottlieb <maorg@nvidia.com>
> 
> In RoCE we should use cma_iboe_set_mgid and not cma_set_mgid to generate
> the mgid, otherwise we will try to remove incorrect address.
> 
> Fixes: b5de0c60cc30 ("RDMA/cma: Fix use after free race in roce multicast join")
> Signed-off-by: Maor Gottlieb <maorg@nvidia.com>
> Signed-off-by: Leon Romanovsky <leonro@nvidia.com>
> ---
>  drivers/infiniband/core/cma.c | 61 +++++++++++++++++------------------
>  1 file changed, 30 insertions(+), 31 deletions(-)

I didn't like this one so much and changed it into this:

@@ -67,8 +67,8 @@ static const char * const cma_events[] = {
        [RDMA_CM_EVENT_TIMEWAIT_EXIT]    = "timewait exit",
 };
 
-static void cma_set_mgid(struct rdma_id_private *id_priv, struct sockaddr *addr,
-                        union ib_gid *mgid);
+static void cma_iboe_set_mgid(struct sockaddr *addr, union ib_gid *mgid,
+                             enum ib_gid_type gid_type);
 
 const char *__attribute_const__ rdma_event_msg(enum rdma_cm_event_type event)
 {
@@ -1846,17 +1846,19 @@ static void destroy_mc(struct rdma_id_private *id_priv,
                if (dev_addr->bound_dev_if)
                        ndev = dev_get_by_index(dev_addr->net,
                                                dev_addr->bound_dev_if);
-               if (ndev) {
+               if (ndev && !send_only) {
+                       enum ib_gid_type gid_type;
                        union ib_gid mgid;
 
-                       cma_set_mgid(id_priv, (struct sockaddr *)&mc->addr,
-                                    &mgid);
-
-                       if (!send_only)
-                               cma_igmp_send(ndev, &mgid, false);
-
-                       dev_put(ndev);
+                       gid_type = id_priv->cma_dev->default_gid_type
+                                          [id_priv->id.port_num -
+                                           rdma_start_port(
+                                                   id_priv->cma_dev->device)];
+                       cma_iboe_set_mgid((struct sockaddr *)&mc->addr, &mgid,
+                                         gid_type);
+                       cma_igmp_send(ndev, &mgid, false);
                }
+               dev_put(ndev);
 
                cancel_work_sync(&mc->iboe_join.work);
        }

Thanks,
Jason
diff mbox series

Patch

diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 27a00ce2e101..69c9a12dd14e 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -1830,6 +1830,31 @@  static void cma_release_port(struct rdma_id_private *id_priv)
 	mutex_unlock(&lock);
 }
 
+static void cma_iboe_set_mgid(struct sockaddr *addr, union ib_gid *mgid,
+			      enum ib_gid_type gid_type)
+{
+	struct sockaddr_in *sin = (struct sockaddr_in *)addr;
+	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)addr;
+
+	if (!cma_any_addr(addr) && addr->sa_family == AF_INET6) {
+		memcpy(mgid, &sin6->sin6_addr, sizeof(*mgid));
+		return;
+	}
+
+	memset(mgid, 0, sizeof(*mgid));
+	if (cma_any_addr(addr))
+		return;
+
+	/* AF_INET4 */
+	if (gid_type != IB_GID_TYPE_ROCE_UDP_ENCAP) {
+		mgid->raw[0] = 0xff;
+		mgid->raw[1] = 0x0e;
+	}
+	mgid->raw[10] = 0xff;
+	mgid->raw[11] = 0xff;
+	*(__be32 *)(&mgid->raw[12]) = sin->sin_addr.s_addr;
+}
+
 static void destroy_mc(struct rdma_id_private *id_priv,
 		       struct cma_multicast *mc)
 {
@@ -1847,10 +1872,13 @@  static void destroy_mc(struct rdma_id_private *id_priv,
 			ndev = dev_get_by_index(dev_addr->net,
 						dev_addr->bound_dev_if);
 		if (ndev) {
+			enum ib_gid_type gid_type;
 			union ib_gid mgid;
 
-			cma_set_mgid(id_priv, (struct sockaddr *)&mc->addr,
-				     &mgid);
+			gid_type = cma_get_default_gid_type(
+				id_priv->cma_dev, id_priv->id.port_num);
+			cma_iboe_set_mgid((struct sockaddr *)&mc->addr, &mgid,
+					  gid_type);
 
 			if (!send_only)
 				cma_igmp_send(ndev, &mgid, false);
@@ -4702,35 +4730,6 @@  static int cma_join_ib_multicast(struct rdma_id_private *id_priv,
 	return PTR_ERR_OR_ZERO(mc->sa_mc);
 }
 
-static void cma_iboe_set_mgid(struct sockaddr *addr, union ib_gid *mgid,
-			      enum ib_gid_type gid_type)
-{
-	struct sockaddr_in *sin = (struct sockaddr_in *)addr;
-	struct sockaddr_in6 *sin6 = (struct sockaddr_in6 *)addr;
-
-	if (cma_any_addr(addr)) {
-		memset(mgid, 0, sizeof *mgid);
-	} else if (addr->sa_family == AF_INET6) {
-		memcpy(mgid, &sin6->sin6_addr, sizeof *mgid);
-	} else {
-		mgid->raw[0] =
-			(gid_type == IB_GID_TYPE_ROCE_UDP_ENCAP) ? 0 : 0xff;
-		mgid->raw[1] =
-			(gid_type == IB_GID_TYPE_ROCE_UDP_ENCAP) ? 0 : 0x0e;
-		mgid->raw[2] = 0;
-		mgid->raw[3] = 0;
-		mgid->raw[4] = 0;
-		mgid->raw[5] = 0;
-		mgid->raw[6] = 0;
-		mgid->raw[7] = 0;
-		mgid->raw[8] = 0;
-		mgid->raw[9] = 0;
-		mgid->raw[10] = 0xff;
-		mgid->raw[11] = 0xff;
-		*(__be32 *)(&mgid->raw[12]) = sin->sin_addr.s_addr;
-	}
-}
-
 static int cma_iboe_join_multicast(struct rdma_id_private *id_priv,
 				   struct cma_multicast *mc)
 {