diff mbox series

[bionic:linux-azure-4.15,1/3] RDMA/rxe: Close a race after ib_register_device

Message ID 20200825181228.3274075-2-marcelo.cerri@canonical.com
State New
Headers show
Series LP:#1874503 - Mellanox Check For Write Combining Support | expand

Commit Message

Marcelo Henrique Cerri Aug. 25, 2020, 6:12 p.m. UTC 
From: Jason Gunthorpe <jgg@mellanox.com>

BugLink: https://bugs.launchpad.net/bugs/1874503

Since rxe allows unregistration from other threads the rxe pointer can
become invalid any moment after ib_register_driver returns. This could
cause a user triggered use after free.

Add another driver callback to be called right after the device becomes
registered to complete any device setup required post-registration.  This
callback has enough core locking to prevent the device from becoming
unregistered.

Signed-off-by: Jason Gunthorpe <jgg@mellanox.com>
(backported from commit ca22354b140853b8155692d5b2bc0110aa54e937)
Signed-off-by: Sultan Alsawaf <sultan.alsawaf@canonical.com>
Signed-off-by: Marcelo Henrique Cerri <marcelo.cerri@canonical.com>
---
 drivers/infiniband/core/device.c      |  8 ++++++++
 drivers/infiniband/sw/rxe/rxe_net.c   |  8 ++++----
 drivers/infiniband/sw/rxe/rxe_net.h   |  2 +-
 drivers/infiniband/sw/rxe/rxe_sysfs.c |  8 ++------
 drivers/infiniband/sw/rxe/rxe_verbs.c | 14 ++++++++++++++
 include/rdma/ib_verbs.h               |  5 +++++
 6 files changed, 34 insertions(+), 11 deletions(-)
diff mbox series

Patch

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 6b0d1d8609ca..664e497d2daf 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -547,6 +547,12 @@  int ib_register_device(struct ib_device *device,
 
 	device->reg_state = IB_DEV_REGISTERED;
 
+	if (device->enable_driver) {
+		ret = device->enable_driver(device);
+		if (ret)
+			goto sysfs_cleanup;
+	}
+
 	list_for_each_entry(client, &client_list, list)
 		if (!add_client_context(device, client) && client->add)
 			client->add(device);
@@ -558,6 +564,8 @@  int ib_register_device(struct ib_device *device,
 	mutex_unlock(&device_mutex);
 	return 0;
 
+sysfs_cleanup:
+	ib_device_unregister_sysfs(device);
 cg_cleanup:
 	ib_device_unregister_rdmacg(device);
 cache_cleanup:
diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index 24a68a9da8be..d568dc12a00b 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -551,27 +551,27 @@  enum rdma_link_layer rxe_link_layer(struct rxe_dev *rxe, unsigned int port_num)
 	return IB_LINK_LAYER_ETHERNET;
 }
 
-struct rxe_dev *rxe_net_add(struct net_device *ndev)
+int rxe_net_add(struct net_device *ndev)
 {
 	int err;
 	struct rxe_dev *rxe = NULL;
 
 	rxe = (struct rxe_dev *)ib_alloc_device(sizeof(*rxe));
 	if (!rxe)
-		return NULL;
+		return -ENOMEM;
 
 	rxe->ndev = ndev;
 
 	err = rxe_add(rxe, ndev->mtu);
 	if (err) {
 		ib_dealloc_device(&rxe->ib_dev);
-		return NULL;
+		return err;
 	}
 
 	spin_lock_bh(&dev_list_lock);
 	list_add_tail(&rxe->list, &rxe_dev_list);
 	spin_unlock_bh(&dev_list_lock);
-	return rxe;
+	return 0;
 }
 
 void rxe_remove_all(void)
diff --git a/drivers/infiniband/sw/rxe/rxe_net.h b/drivers/infiniband/sw/rxe/rxe_net.h
index 1c06b3bfe1b6..0a889f21f64d 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.h
+++ b/drivers/infiniband/sw/rxe/rxe_net.h
@@ -47,7 +47,7 @@  extern struct rxe_recv_sockets recv_sockets;
 extern struct notifier_block rxe_net_notifier;
 void rxe_release_udp_tunnel(struct socket *sk);
 
-struct rxe_dev *rxe_net_add(struct net_device *ndev);
+int rxe_net_add(struct net_device *ndev);
 
 int rxe_net_init(void);
 void rxe_net_exit(void);
diff --git a/drivers/infiniband/sw/rxe/rxe_sysfs.c b/drivers/infiniband/sw/rxe/rxe_sysfs.c
index d5ed7571128f..08c9e9539977 100644
--- a/drivers/infiniband/sw/rxe/rxe_sysfs.c
+++ b/drivers/infiniband/sw/rxe/rxe_sysfs.c
@@ -75,7 +75,6 @@  static int rxe_param_set_add(const char *val, const struct kernel_param *kp)
 	int err = 0;
 	char intf[32];
 	struct net_device *ndev = NULL;
-	struct rxe_dev *rxe;
 
 	len = sanitize_arg(val, intf, sizeof(intf));
 	if (!len) {
@@ -97,15 +96,12 @@  static int rxe_param_set_add(const char *val, const struct kernel_param *kp)
 		goto err;
 	}
 
-	rxe = rxe_net_add(ndev);
-	if (!rxe) {
+	err = rxe_net_add(ndev);
+	if (err) {
 		pr_err("failed to add %s\n", intf);
-		err = -EINVAL;
 		goto err;
 	}
 
-	rxe_set_port_state(ndev);
-	pr_info("added %s to %s\n", rxe->ib_dev.name, intf);
 err:
 	if (ndev)
 		dev_put(ndev);
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
index f9cec72273cd..72f65a5a4ee2 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
@@ -1188,6 +1188,15 @@  static struct device_attribute *rxe_dev_attributes[] = {
 	&dev_attr_parent,
 };
 
+static int rxe_enable_driver(struct ib_device *ib_dev)
+{
+	struct rxe_dev *rxe = container_of(ib_dev, struct rxe_dev, ib_dev);
+
+	rxe_set_port_state(rxe);
+	dev_info(&rxe->ib_dev.dev, "added %s\n", netdev_name(rxe->ndev));
+	return 0;
+}
+
 int rxe_register_device(struct rxe_dev *rxe)
 {
 	int err;
@@ -1290,6 +1299,7 @@  int rxe_register_device(struct rxe_dev *rxe)
 	dev->detach_mcast = rxe_detach_mcast;
 	dev->get_hw_stats = rxe_ib_get_hw_stats;
 	dev->alloc_hw_stats = rxe_ib_alloc_hw_stats;
+	dev->enable_driver = rxe_enable_driver;
 
 	tfm = crypto_alloc_shash("crc32", 0, 0);
 	if (IS_ERR(tfm)) {
@@ -1321,6 +1331,10 @@  int rxe_register_device(struct rxe_dev *rxe)
 err1:
 	crypto_free_shash(rxe->tfm);
 
+	/*
+	 * Note that rxe may be invalid at this point if another thread
+	 * unregistered it.
+	 */
 	return err;
 }
 
diff --git a/include/rdma/ib_verbs.h b/include/rdma/ib_verbs.h
index ad7547c7ffc2..a9a4e15fd0ad 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2317,6 +2317,11 @@  struct ib_device {
 							   struct ib_rwq_ind_table_init_attr *init_attr,
 							   struct ib_udata *udata);
 	int                        (*destroy_rwq_ind_table)(struct ib_rwq_ind_table *wq_ind_table);
+	/*
+	 * Called after the device becomes registered, before clients are
+	 * attached
+	 */
+	int                        (*enable_driver)(struct ib_device *device);
 	/**
 	 * rdma netdev operation
 	 *