RDMA/rxe: Close a race after ib_register_device

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>

diff --git a/drivers/infiniband/core/device.c b/drivers/infiniband/core/device.c
index 2a7d54794ee3ebadda1671191ec49ba278f95e63..bf2a215d94ddfc1bc72e955be0bd902f5d6a3d85 100644 (file)
--- a/drivers/infiniband/core/device.c
+++ b/drivers/infiniband/core/device.c
@@ -803,6 +803,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)
+                       goto out;
+       }
+
        down_read(&clients_rwsem);
        xa_for_each_marked (&clients, index, client, CLIENT_REGISTERED) {
                ret = add_client_context(device, client);
@@ -810,6 +816,8 @@ static int enable_device_and_get(struct ib_device *device)
                        break;
        }
        up_read(&clients_rwsem);
+
+out:
        up_read(&devices_rwsem);
        return ret;
 }
@@ -1775,6 +1783,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 d6dfbcf6a47e7b4f19e9ffbfbd2c64fec0a09ff8..fb792f5bc0b7d49f8b69e60728b15a7f3f20bd6f 100644 (file)
--- 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 106c586dbb268375adc693752aef416f3e588ad8..ad79514191bb9daf5fefda041dd9bea8da68aaae 100644 (file)
--- 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 d51b55b0a311c577e19d52e6e324253f6a74c00f..46587eb0da0e633517813f3301e9257890a2308a 100644 (file)
--- 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 76da8a142bf70ceed29cfb56073f3cfdcc75ec96..79ad93b4140cea3dccc1d3000a52f5fefd6a67b8 100644 (file)
--- a/drivers/infiniband/sw/rxe/rxe_verbs.c
+++ b/drivers/infiniband/sw/rxe/rxe_verbs.c
@@ -1125,6 +1125,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,
@@ -1144,6 +1153,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,
@@ -1245,5 +1255,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 ad83f8c38dc8f9b013d58940c0c3817ea914a439..640263289ab9029fe5f577bb8c8743d7313f891e 100644 (file)
--- 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().
         */