Commit 01b67117 authored by Parav Pandit's avatar Parav Pandit Committed by Jason Gunthorpe
Browse files

RDMA/core: Sync unregistration with netlink commands 



When the rdma device is getting removed, get resource info can race with
device removal, as below:

      CPU-0                                  CPU-1
    --------                               --------
    rdma_nl_rcv_msg()
       nldev_res_get_cq_dumpit()
          mutex_lock(device_lock);
          get device reference
          mutex_unlock(device_lock);        [..]
                                            ib_unregister_device()
                                            /* Valid reference to
                                             * device->dev exists.
                                             */
                                             ib_dealloc_device()

          [..]
          provider->fill_res_entry();

Even though device object is not freed, fill_res_entry() can get called on
device which doesn't have a driver anymore. Kernel core device reference
count is not sufficient, as this only keeps the structure valid, and
doesn't guarantee the driver is still loaded.

Similar race can occur with device renaming and device removal, where
device_rename() tries to rename a unregistered device. While this is fine
for devices of a class which are not net namespace aware, but it is
incorrect for net namespace aware class coming in subsequent series.  If a
class is net namespace aware, then the below [1] call trace is observed in
above situation.

Therefore, to avoid the race, keep a reference count and let device
unregistration wait until all netlink users drop the reference.

[1] Call trace:
kernfs: ns required in 'infiniband' for 'mlx5_0'
WARNING: CPU: 18 PID: 44270 at fs/kernfs/dir.c:842 kernfs_find_ns+0x104/0x120
libahci i2c_core mlxfw libata dca [last unloaded: devlink]
RIP: 0010:kernfs_find_ns+0x104/0x120
Call Trace:
kernfs_find_and_get_ns+0x2e/0x50
sysfs_rename_link_ns+0x40/0xb0
device_rename+0xb2/0xf0
ib_device_rename+0xb3/0x100 [ib_core]
nldev_set_doit+0x165/0x190 [ib_core]
rdma_nl_rcv_msg+0x249/0x250 [ib_core]
? netlink_deliver_tap+0x8f/0x3e0
rdma_nl_rcv+0xd6/0x120 [ib_core]
netlink_unicast+0x17c/0x230
netlink_sendmsg+0x2f0/0x3e0
sock_sendmsg+0x30/0x40
__sys_sendto+0xdc/0x160

Fixes: da5c8507 ("RDMA/nldev: add driver-specific resource tracking")
Signed-off-by: default avatarParav Pandit <parav@mellanox.com>
Signed-off-by: default avatarLeon Romanovsky <leonro@mellanox.com>
Signed-off-by: default avatarJason Gunthorpe <jgg@mellanox.com>
parent eeb8df87

drivers/infiniband/core/core_priv.h

+1 −0
Original line number Diff line number Diff line
@@ -267,6 +267,7 @@ static inline int ib_mad_enforce_security(struct ib_mad_agent_private *map, 
#endif



struct ib_device *ib_device_get_by_index(u32 ifindex);

void ib_device_put(struct ib_device *device);

/* RDMA device netlink */

void nldev_init(void);

void nldev_exit(void);

drivers/infiniband/core/device.c

+22 −4
Original line number Diff line number Diff line
@@ -145,7 +145,8 @@ static struct ib_device *__ib_device_get_by_index(u32 index) 
}



/*

 * Caller is responsible to return refrerence count by calling put_device()

 * Caller must perform ib_device_put() to return the device reference count

 * when ib_device_get_by_index() returns valid device pointer.

 */

struct ib_device *ib_device_get_by_index(u32 index)

{
@@ -153,13 +154,21 @@ struct ib_device *ib_device_get_by_index(u32 index) 


	down_read(&lists_rwsem);

	device = __ib_device_get_by_index(index);

	if (device)

		get_device(&device->dev);



	if (device) {

		/* Do not return a device if unregistration has started. */

		if (!refcount_inc_not_zero(&device->refcount))

			device = NULL;

	}

	up_read(&lists_rwsem);

	return device;

}



void ib_device_put(struct ib_device *device)

{

	if (refcount_dec_and_test(&device->refcount))

		complete(&device->unreg_completion);

}



static struct ib_device *__ib_device_get_by_name(const char *name)

{

	struct ib_device *device;
@@ -293,6 +302,8 @@ struct ib_device *ib_alloc_device(size_t size) 
	rwlock_init(&device->client_data_lock);

	INIT_LIST_HEAD(&device->client_data_list);

	INIT_LIST_HEAD(&device->port_list);

	refcount_set(&device->refcount, 1);

	init_completion(&device->unreg_completion);



	return device;

}
@@ -641,6 +652,13 @@ void ib_unregister_device(struct ib_device *device) 
	struct ib_client_data *context, *tmp;

	unsigned long flags;



	/*

	 * Wait for all netlink command callers to finish working on the

	 * device.

	 */

	ib_device_put(device);

	wait_for_completion(&device->unreg_completion);



	mutex_lock(&device_mutex);



	down_write(&lists_rwsem);

drivers/infiniband/core/nldev.c

+10 −10
Original line number Diff line number Diff line
@@ -636,13 +636,13 @@ static int nldev_get_doit(struct sk_buff *skb, struct nlmsghdr *nlh, 


	nlmsg_end(msg, nlh);



	put_device(&device->dev);

	ib_device_put(device);

	return rdma_nl_unicast(msg, NETLINK_CB(skb).portid);



err_free:

	nlmsg_free(msg);

err:

	put_device(&device->dev);

	ib_device_put(device);

	return err;

}
@@ -672,7 +672,7 @@ static int nldev_set_doit(struct sk_buff *skb, struct nlmsghdr *nlh, 
		err = ib_device_rename(device, name);

	}



	put_device(&device->dev);

	ib_device_put(device);

	return err;

}
@@ -756,14 +756,14 @@ static int nldev_port_get_doit(struct sk_buff *skb, struct nlmsghdr *nlh, 
		goto err_free;



	nlmsg_end(msg, nlh);

	put_device(&device->dev);

	ib_device_put(device);



	return rdma_nl_unicast(msg, NETLINK_CB(skb).portid);



err_free:

	nlmsg_free(msg);

err:

	put_device(&device->dev);

	ib_device_put(device);

	return err;

}
@@ -820,7 +820,7 @@ static int nldev_port_get_dumpit(struct sk_buff *skb, 
	}



out:

	put_device(&device->dev);

	ib_device_put(device);

	cb->args[0] = idx;

	return skb->len;

}
@@ -859,13 +859,13 @@ static int nldev_res_get_doit(struct sk_buff *skb, struct nlmsghdr *nlh, 
		goto err_free;



	nlmsg_end(msg, nlh);

	put_device(&device->dev);

	ib_device_put(device);

	return rdma_nl_unicast(msg, NETLINK_CB(skb).portid);



err_free:

	nlmsg_free(msg);

err:

	put_device(&device->dev);

	ib_device_put(device);

	return ret;

}
@@ -1058,7 +1058,7 @@ next: idx++; 
	if (!filled)

		goto err;



	put_device(&device->dev);

	ib_device_put(device);

	return skb->len;



res_err:
@@ -1069,7 +1069,7 @@ err: 
	nlmsg_cancel(skb, nlh);



err_index:

	put_device(&device->dev);

	ib_device_put(device);

	return ret;

}

include/rdma/ib_verbs.h

+7 −1
Original line number Diff line number Diff line
@@ -53,7 +53,7 @@ 
#include <linux/string.h>

#include <linux/slab.h>

#include <linux/netdevice.h>



#include <linux/refcount.h>

#include <linux/if_link.h>

#include <linux/atomic.h>

#include <linux/mmu_notifier.h>
@@ -2602,6 +2602,12 @@ struct ib_device { 


	const struct uapi_definition   *driver_def;

	enum rdma_driver_id		driver_id;

	/*

	 * Provides synchronization between device unregistration and netlink

	 * commands on a device. To be used only by core.

	 */

	refcount_t refcount;

	struct completion unreg_completion;

};



struct ib_client {

CRA Git | Maintained and supported by SUSTech CRA and CCSE