From: Jason Gunthorpe <jgg@xxxxxxxxxxxx>
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@xxxxxxxxxxxx>
---
drivers/infiniband/core/device.c | 7 +++++++
drivers/infiniband/sw/rxe/rxe_net.c | 8 ++++----
drivers/infiniband/sw/rxe/rxe_net.h | 2 +-
drivers/infiniband/sw/rxe/rxe_sysfs.c | 9 ++-------
drivers/infiniband/sw/rxe/rxe_verbs.c | 14 ++++++++++++++
include/rdma/ib_verbs.h | 5 +++++
6 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index c8cacc19625708..e3f41557188b0f 100644
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -796,6 +796,12 @@ static int enable_device_and_get(struct ib_device *device)
*/
downgrade_write(&devices_rwsem);
+ if (device->ops.enable_driver) {
+ ret = device->ops.enable_driver(device);
+ if (ret)
+ return ret;
+ }
+
down_read(&clients_rwsem);
xa_for_each_marked (&clients, index, client, CLIENT_REGISTERED) {
ret = add_client_context(device, client);
@@ -1761,6 +1767,7 @@ void ib_set_device_ops(struct ib_device *dev, const struct ib_device_ops *ops)
SET_DEVICE_OP(dev_ops, disassociate_ucontext);
SET_DEVICE_OP(dev_ops, drain_rq);
SET_DEVICE_OP(dev_ops, drain_sq);
+ SET_DEVICE_OP(dev_ops, enable_driver);
SET_DEVICE_OP(dev_ops, fill_res_entry);
SET_DEVICE_OP(dev_ops, get_dev_fw_str);
SET_DEVICE_OP(dev_ops, get_dma_mr);
diff --git a/drivers/infiniband/sw/rxe/rxe_net.c b/drivers/infiniband/sw/rxe/rxe_net.c
index d6dfbcf6a47e7b..fb792f5bc0b7d4 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.c
+++ b/drivers/infiniband/sw/rxe/rxe_net.c
@@ -517,24 +517,24 @@ 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 = ib_alloc_device(rxe_dev, ib_dev);
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;
}
- return rxe;
+ return 0;
}
static void rxe_port_event(struct rxe_dev *rxe,
diff --git a/drivers/infiniband/sw/rxe/rxe_net.h b/drivers/infiniband/sw/rxe/rxe_net.h
index 106c586dbb2683..ad79514191bb9d 100644
--- a/drivers/infiniband/sw/rxe/rxe_net.h
+++ b/drivers/infiniband/sw/rxe/rxe_net.h
@@ -43,7 +43,7 @@ struct rxe_recv_sockets {
struct socket *sk6;
};
-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 d51b55b0a311c5..46587eb0da0e63 100644
--- a/drivers/infiniband/sw/rxe/rxe_sysfs.c
+++ b/drivers/infiniband/sw/rxe/rxe_sysfs.c
@@ -60,7 +60,6 @@ static int rxe_param_set_add(const char *val, const struct kernel_param *kp)
char intf[32];
struct net_device *ndev;
struct rxe_dev *exists;
- struct rxe_dev *rxe;
len = sanitize_arg(val, intf, sizeof(intf));
if (!len) {
@@ -82,16 +81,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(rxe);
- dev_info(&rxe->ib_dev.dev, "added %s\n", intf);
-
err:
dev_put(ndev);
return err;
diff --git a/drivers/infiniband/sw/rxe/rxe_verbs.c b/drivers/infiniband/sw/rxe/rxe_verbs.c
index c7b7a59c0b2048..2834e16e704747 100644
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
@@ -1123,6 +1123,15 @@ static const struct attribute_group rxe_attr_group = {
.attrs = rxe_dev_attributes,
};
+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;
+}
+
static const struct ib_device_ops rxe_dev_ops = {
.alloc_hw_stats = rxe_ib_alloc_hw_stats,
.alloc_mr = rxe_alloc_mr,
@@ -1142,6 +1151,7 @@ static const struct ib_device_ops rxe_dev_ops = {
.destroy_qp = rxe_destroy_qp,
.destroy_srq = rxe_destroy_srq,
.detach_mcast = rxe_detach_mcast,
+ .enable_driver = rxe_enable_driver,
.get_dma_mr = rxe_get_dma_mr,
.get_hw_stats = rxe_ib_get_hw_stats,
.get_link_layer = rxe_get_link_layer,
@@ -1243,5 +1253,9 @@ int rxe_register_device(struct rxe_dev *rxe)
if (err)
pr_warn("%s failed with error %d\n", __func__, err);
+ /*
+ * 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 282d2903e6d3cf..cde26abd16bb35 100644
--- a/include/rdma/ib_verbs.h
+++ b/include/rdma/ib_verbs.h
@@ -2539,6 +2539,11 @@ struct ib_device_ops {
struct rdma_restrack_entry *entry);
/* Device lifecycle callbacks */
+ /*
+ * Called after the device becomes registered, before clients are
+ * attached
+ */
+ int (*enable_driver)(struct ib_device *dev);
/*
* This is called as part of ib_dealloc_device().
*/
--
2.20.1
