Skip to contentSkip to conversation

IB/mlx4: Fix memory leak in add_gid error flow

PATCH Applied Leon Romanovsky 2 participants 2 messages Jan 15, 2020 – Jan 16, 2020

Subsystems (2)
InfiniBand Mellanox MLX4 IB driver
lore
Share & tools
lore

Leon Romanovsky

author
#0 lore raw

PATCH 1/1 IB/mlx4: Fix memory leak in add_gid error flow

From: Jack Morgenstein <[email protected]>

In procedure mlx4_ib_add_gid(), if the driver is unable to update the FW gid table, there is a memory leak in the driver's copy of the gid table: the gid entry's context buffer is not freed.

If such an error occurs, free the entry's context buffer, and mark the entry as available (by setting its context pointer to NULL).

Fixes e26be1bfef81 ("IB/mlx4: Implement ib_device callbacks")
Signed-off-by Jack Morgenstein <[email protected]>
Reviewed-by Parav Pandit <[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 drivers/infiniband/hw/mlx4/main.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)
drivers/infiniband/hw/mlx4/main.c +16 -4
index 34055cbab38c..2f5d9b181848 100644
@@ -246,6 +246,13 @@ static int mlx4_ib_update_gids(struct gid_entry *gids,
 	return mlx4_ib_update_gids_v1(gids, ibdev, port_num);
 }
 
+static void free_gid_entry(struct gid_entry *entry)
+{
+	memset(&entry->gid, 0, sizeof(entry->gid));
+	kfree(entry->ctx);
+	entry->ctx = NULL;
+}
+
 static int mlx4_ib_add_gid(const struct ib_gid_attr *attr, void **context)
 {
 	struct mlx4_ib_dev *ibdev = to_mdev(attr->device);

@@ -313,6 +320,8 @@ static int mlx4_ib_add_gid(const struct ib_gid_attr *attr, void **context)
 				     GFP_ATOMIC);
 		if (!gids) {
 			ret = -ENOMEM;
+			*context = NULL;
+			free_gid_entry(&port_gid_table->gids[free]);
 		} else {
 			for (i = 0; i < MLX4_MAX_PORT_GIDS; i++) {
 				memcpy(&gids[i].gid, &port_gid_table->gids[i].gid, sizeof(union ib_gid));

@@ -324,6 +333,12 @@ static int mlx4_ib_add_gid(const struct ib_gid_attr *attr, void **context)
 
 	if (!ret && hw_update) {
 		ret = mlx4_ib_update_gids(gids, ibdev, attr->port_num);
+		if (ret) {
+			spin_lock_bh(&iboe->lock);
+			*context = NULL;
+			free_gid_entry(&port_gid_table->gids[free]);
+			spin_unlock_bh(&iboe->lock);
+		}
 		kfree(gids);
 	}
 

@@ -353,10 +368,7 @@ static int mlx4_ib_del_gid(const struct ib_gid_attr *attr, void **context)
 		if (!ctx->refcount) {
 			unsigned int real_index = ctx->real_index;
 
-			memset(&port_gid_table->gids[real_index].gid, 0,
-			       sizeof(port_gid_table->gids[real_index].gid));
-			kfree(port_gid_table->gids[real_index].ctx);
-			port_gid_table->gids[real_index].ctx = NULL;
+			free_gid_entry(&port_gid_table->gids[real_index]);
 			hw_update = 1;
 		}
 	}
Signature · 1 line
2.20.1
Applied
#1 lore raw
Quoted 9 lines · Leon Romanovsky

From: Jack Morgenstein <[email protected]>

In procedure mlx4_ib_add_gid(), if the driver is unable to update the FW gid table, there is a memory leak in the driver's copy of the gid table: the gid entry's context buffer is not freed.

If such an error occurs, free the entry's context buffer, and mark the entry as available (by setting its context pointer to NULL).

Quoted 7 lines
Fixes e26be1bfef81 ("IB/mlx4: Implement ib_device callbacks")
Signed-off-by Jack Morgenstein <[email protected]>
Reviewed-by Parav Pandit <[email protected]>
Signed-off-by Leon Romanovsky <[email protected]>
---
 drivers/infiniband/hw/mlx4/main.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

Applied to for-next, thanks

Jason