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-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <AM0PR05MB48664AE114EE357CC33EA945D1DD0@AM0PR05MB4866.eurprd05.prod.outlook.com>
Date:   Mon, 29 Jul 2019 05:25:13 +0000
From:   Parav Pandit <parav@...lanox.com>
To:     Jia-Ju Bai <baijiaju1990@...il.com>,
        "leon@...nel.org" <leon@...nel.org>,
        "dledford@...hat.com" <dledford@...hat.com>,
        "jgg@...pe.ca" <jgg@...pe.ca>
CC:     "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [BUG] infiniband: mlx5: a possible null-pointer dereference in
 set_roce_addr()

Hi Jia,

> -----Original Message-----
> From: linux-rdma-owner@...r.kernel.org <linux-rdma-
> owner@...r.kernel.org> On Behalf Of Jia-Ju Bai
> Sent: Monday, July 29, 2019 7:47 AM
> To: leon@...nel.org; dledford@...hat.com; jgg@...pe.ca
> Cc: linux-rdma@...r.kernel.org; linux-kernel@...r.kernel.org
> Subject: [BUG] infiniband: mlx5: a possible null-pointer dereference in
> set_roce_addr()
> 
> In set_roce_addr(), there is an if statement on line 589 to check whether gid is
> NULL:
>      if (gid)
> 
> When gid is NULL, it is used on line 613:
>      return mlx5_core_roce_gid_set(..., gid->raw, ...);
> 
> Thus, a possible null-pointer dereference may occur.
> 
> This bug is found by a static analysis tool STCheck written by us.
> 
While static checker is right, it is not a real bug, because gid->raw pointer points to GID entry itself so when GID is NULL, gid->raw is NULL too.

One way to suppress the static checker warning/error is below patch.
Will let Leon review it.

> I do not know how to correctly fix this bug, so I only report it.
> 
> 
> Best wishes,
> Jia-Ju Bai

From 30e055dba77e595bf88aebd3a9c75ed76bc9c65a Mon Sep 17 00:00:00 2001
From: Parav Pandit <parav@...lanox.com>
Date: Mon, 29 Jul 2019 00:13:21 -0500
Subject: [PATCH] IB/mlx5: Avoid static checker warning for NULL access

union ib_gid *gid and gid->raw pointers refers to the same address.
However some static checker reports this as possible NULL access
warning in call to mlx5_core_roce_gid_set().

To suppress such warning, instead of working on raw GID element,
expose API using union ib_gid*.

Reported-by: Jia-Ju Bai <baijiaju1990@...il.com>
Signed-off-by: Parav Pandit <parav@...lanox.com>
---
 drivers/infiniband/hw/mlx5/main.c                   |  2 +-
 drivers/net/ethernet/mellanox/mlx5/core/fpga/conn.c | 12 +++++++-----
 drivers/net/ethernet/mellanox/mlx5/core/lib/gid.c   |  5 +++--
 drivers/net/ethernet/mellanox/mlx5/core/rdma.c      |  2 +-
 include/linux/mlx5/driver.h                         |  4 +++-
 5 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/infiniband/hw/mlx5/main.c b/drivers/infiniband/hw/mlx5/main.c
index c2a5780cb394..e60785bad7ef 100644
--- a/drivers/infiniband/hw/mlx5/main.c
+++ b/drivers/infiniband/hw/mlx5/main.c
@@ -610,7 +610,7 @@ static int set_roce_addr(struct mlx5_ib_dev *dev, u8 port_num,
 	}
 
 	return mlx5_core_roce_gid_set(dev->mdev, index, roce_version,
-				      roce_l3_type, gid->raw, mac,
+				      roce_l3_type, &gid, mac,
 				      vlan_id < VLAN_CFI_MASK, vlan_id,
 				      port_num);
 }
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fpga/conn.c b/drivers/net/ethernet/mellanox/mlx5/core/fpga/conn.c
index 4c50efe4e7f1..76b8236af9c7 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fpga/conn.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fpga/conn.c
@@ -850,6 +850,7 @@ struct mlx5_fpga_conn *mlx5_fpga_conn_create(struct mlx5_fpga_device *fdev,
 					     enum mlx5_ifc_fpga_qp_type qp_type)
 {
 	struct mlx5_fpga_conn *ret, *conn;
+	struct ib_gid remote_gid = {};
 	u8 *remote_mac, *remote_ip;
 	int err;
 
@@ -876,11 +877,12 @@ struct mlx5_fpga_conn *mlx5_fpga_conn_create(struct mlx5_fpga_device *fdev,
 		goto err;
 	}
 
-	/* Build Modified EUI-64 IPv6 address from the MAC address */
 	remote_ip = MLX5_ADDR_OF(fpga_qpc, conn->fpga_qpc, remote_ip);
-	remote_ip[0] = 0xfe;
-	remote_ip[1] = 0x80;
-	addrconf_addr_eui48(&remote_ip[8], remote_mac);
+	memcpy(remote_gid.raw[0], remote_ip, sizeof(remote_gid.raw));
+	/* Build Modified EUI-64 IPv6 address from the MAC address */
+	remte_gid.raw[0] = 0xfe;
+	remte_gid.raw[1] = 0x80;
+	addrconf_addr_eui48(&remote_gid.raw[8], remote_mac);
 
 	err = mlx5_core_reserved_gid_alloc(fdev->mdev, &conn->qp.sgid_index);
 	if (err) {
@@ -892,7 +894,7 @@ struct mlx5_fpga_conn *mlx5_fpga_conn_create(struct mlx5_fpga_device *fdev,
 	err = mlx5_core_roce_gid_set(fdev->mdev, conn->qp.sgid_index,
 				     MLX5_ROCE_VERSION_2,
 				     MLX5_ROCE_L3_TYPE_IPV6,
-				     remote_ip, remote_mac, true, 0,
+				     &remote_gid, remote_mac, true, 0,
 				     MLX5_FPGA_PORT_NUM);
 	if (err) {
 		mlx5_fpga_err(fdev, "Failed to set SGID: %d\n", err);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/lib/gid.c b/drivers/net/ethernet/mellanox/mlx5/core/lib/gid.c
index 7722a3f9bb68..9b8563a2bd50 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/lib/gid.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/lib/gid.c
@@ -120,7 +120,8 @@ unsigned int mlx5_core_reserved_gids_count(struct mlx5_core_dev *dev)
 EXPORT_SYMBOL_GPL(mlx5_core_reserved_gids_count);
 
 int mlx5_core_roce_gid_set(struct mlx5_core_dev *dev, unsigned int index,
-			   u8 roce_version, u8 roce_l3_type, const u8 *gid,
+			   u8 roce_version, u8 roce_l3_type,
+			   const union ib_gid *gid,
 			   const u8 *mac, bool vlan, u16 vlan_id, u8 port_num)
 {
 #define MLX5_SET_RA(p, f, v) MLX5_SET(roce_addr_layout, p, f, v)
@@ -145,7 +146,7 @@ int mlx5_core_roce_gid_set(struct mlx5_core_dev *dev, unsigned int index,
 		ether_addr_copy(addr_mac, mac);
 		MLX5_SET_RA(in_addr, roce_version, roce_version);
 		MLX5_SET_RA(in_addr, roce_l3_type, roce_l3_type);
-		memcpy(addr_l3_addr, gid, gidsz);
+		memcpy(addr_l3_addr, &gid->raw[0], gidsz);
 	}
 
 	if (MLX5_CAP_GEN(dev, num_vhca_ports) > 0)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/rdma.c b/drivers/net/ethernet/mellanox/mlx5/core/rdma.c
index 17ce9dd56b13..2a4467346231 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/rdma.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/rdma.c
@@ -139,7 +139,7 @@ static int mlx5_rdma_add_roce_addr(struct mlx5_core_dev *dev)
 	mlx5_rdma_make_default_gid(dev, &gid);
 	return mlx5_core_roce_gid_set(dev, 0,
 				      MLX5_ROCE_VERSION_1,
-				      0, gid.raw, mac,
+				      0, &gid, mac,
 				      false, 0, 1);
 }
 
diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
index 1e42c13819ae..691864e853be 100644
--- a/include/linux/mlx5/driver.h
+++ b/include/linux/mlx5/driver.h
@@ -47,6 +47,7 @@
 #include <linux/interrupt.h>
 #include <linux/idr.h>
 #include <linux/notifier.h>
+#include <rdma/ib_verbs.h>
 
 #include <linux/mlx5/device.h>
 #include <linux/mlx5/doorbell.h>
@@ -1002,7 +1003,8 @@ struct cpumask *
 mlx5_comp_irq_get_affinity_mask(struct mlx5_core_dev *dev, int vector);
 unsigned int mlx5_core_reserved_gids_count(struct mlx5_core_dev *dev);
 int mlx5_core_roce_gid_set(struct mlx5_core_dev *dev, unsigned int index,
-			   u8 roce_version, u8 roce_l3_type, const u8 *gid,
+			   u8 roce_version, u8 roce_l3_type,
+			   const union ib_gid *gid,
 			   const u8 *mac, bool vlan, u16 vlan_id, u8 port_num);
 
 static inline int fw_initializing(struct mlx5_core_dev *dev)
-- 
2.19.2

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ