Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 7c1eb45a authored by Haggai Eran's avatar Haggai Eran Committed by Doug Ledford
Browse files

IB/core: lock client data with lists_rwsem 



An ib_client callback that is called with the lists_rwsem locked only for
read is protected from changes to the IB client lists, but not from
ib_unregister_device() freeing its client data. This is because
ib_unregister_device() will remove the device from the device list with
lists_rwsem locked for write, but perform the rest of the cleanup,
including the call to remove() without that lock.

Mark client data that is undergoing de-registration with a new going_down
flag in the client data context. Lock the client data list with lists_rwsem
for write in addition to using the spinlock, so that functions calling the
callback would be able to lock only lists_rwsem for read and let callbacks
sleep.

Since ib_unregister_client() now marks the client data context, no need for
remove() to search the context again, so pass the client data directly to
remove() callbacks.

Reviewed-by: default avatarJason Gunthorpe <jgunthorpe@obsidianresearch.com>
Signed-off-by: default avatarHaggai Eran <haggaie@mellanox.com>
Signed-off-by: default avatarDoug Ledford <dledford@redhat.com>
parent 5aa44bb9

drivers/infiniband/core/cache.c

+1 −1
Original line number Original line Diff line number Diff line
@@ -394,7 +394,7 @@ static void ib_cache_setup_one(struct ib_device *device) 
	kfree(device->cache.lmc_cache);

	kfree(device->cache.lmc_cache);

}

}





static void ib_cache_cleanup_one(struct ib_device *device)

static void ib_cache_cleanup_one(struct ib_device *device, void *client_data)

{

{

	int p;

	int p;

drivers/infiniband/core/cm.c

+3 −4
Original line number Original line Diff line number Diff line
@@ -58,7 +58,7 @@ MODULE_DESCRIPTION("InfiniBand CM"); 
MODULE_LICENSE("Dual BSD/GPL");

MODULE_LICENSE("Dual BSD/GPL");





static void cm_add_one(struct ib_device *device);

static void cm_add_one(struct ib_device *device);

static void cm_remove_one(struct ib_device *device);

static void cm_remove_one(struct ib_device *device, void *client_data);





static struct ib_client cm_client = {

static struct ib_client cm_client = {

	.name   = "cm",

	.name   = "cm",
@@ -3886,9 +3886,9 @@ static void cm_add_one(struct ib_device *ib_device) 
	kfree(cm_dev);

	kfree(cm_dev);

}

}





static void cm_remove_one(struct ib_device *ib_device)

static void cm_remove_one(struct ib_device *ib_device, void *client_data)

{

{

	struct cm_device *cm_dev;

	struct cm_device *cm_dev = client_data;

	struct cm_port *port;

	struct cm_port *port;

	struct ib_port_modify port_modify = {

	struct ib_port_modify port_modify = {

		.clr_port_cap_mask = IB_PORT_CM_SUP

		.clr_port_cap_mask = IB_PORT_CM_SUP
@@ -3896,7 +3896,6 @@ static void cm_remove_one(struct ib_device *ib_device) 
	unsigned long flags;

	unsigned long flags;

	int i;

	int i;





	cm_dev = ib_get_client_data(ib_device, &cm_client);

	if (!cm_dev)

	if (!cm_dev)

		return;

		return;

drivers/infiniband/core/cma.c

+3 −4
Original line number Original line Diff line number Diff line
@@ -94,7 +94,7 @@ const char *rdma_event_msg(enum rdma_cm_event_type event) 
EXPORT_SYMBOL(rdma_event_msg);

EXPORT_SYMBOL(rdma_event_msg);





static void cma_add_one(struct ib_device *device);

static void cma_add_one(struct ib_device *device);

static void cma_remove_one(struct ib_device *device);

static void cma_remove_one(struct ib_device *device, void *client_data);





static struct ib_client cma_client = {

static struct ib_client cma_client = {

	.name   = "cma",

	.name   = "cma",
@@ -3554,11 +3554,10 @@ static void cma_process_remove(struct cma_device *cma_dev) 
	wait_for_completion(&cma_dev->comp);

	wait_for_completion(&cma_dev->comp);

}

}





static void cma_remove_one(struct ib_device *device)

static void cma_remove_one(struct ib_device *device, void *client_data)

{

{

	struct cma_device *cma_dev;

	struct cma_device *cma_dev = client_data;





	cma_dev = ib_get_client_data(device, &cma_client);

	if (!cma_dev)

	if (!cma_dev)

		return;

		return;

drivers/infiniband/core/device.c

+44 −9
Original line number Original line Diff line number Diff line
@@ -50,6 +50,9 @@ struct ib_client_data { 
	struct list_head  list;

	struct list_head  list;

	struct ib_client *client;

	struct ib_client *client;

	void *            data;

	void *            data;

	/* The device or client is going down. Do not call client or device

	 * callbacks other than remove(). */

	bool		  going_down;

};

};





struct workqueue_struct *ib_wq;

struct workqueue_struct *ib_wq;
@@ -69,6 +72,8 @@ static LIST_HEAD(client_list); 
 * to the lists must be done with a write lock. A special case is when the

 * to the lists must be done with a write lock. A special case is when the

 * device_mutex is locked. In this case locking the lists for read access is

 * device_mutex is locked. In this case locking the lists for read access is

 * not necessary as the device_mutex implies it.

 * not necessary as the device_mutex implies it.

 *

 * lists_rwsem also protects access to the client data list.

 */

 */

static DEFINE_MUTEX(device_mutex);

static DEFINE_MUTEX(device_mutex);

static DECLARE_RWSEM(lists_rwsem);

static DECLARE_RWSEM(lists_rwsem);
@@ -210,10 +215,13 @@ static int add_client_context(struct ib_device *device, struct ib_client *client 




	context->client = client;

	context->client = client;

	context->data   = NULL;

	context->data   = NULL;

	context->going_down = false;





	down_write(&lists_rwsem);

	spin_lock_irqsave(&device->client_data_lock, flags);

	spin_lock_irqsave(&device->client_data_lock, flags);

	list_add(&context->list, &device->client_data_list);

	list_add(&context->list, &device->client_data_list);

	spin_unlock_irqrestore(&device->client_data_lock, flags);

	spin_unlock_irqrestore(&device->client_data_lock, flags);

	up_write(&lists_rwsem);





	return 0;

	return 0;

}

}
@@ -339,7 +347,6 @@ EXPORT_SYMBOL(ib_register_device); 
 */

 */

void ib_unregister_device(struct ib_device *device)

void ib_unregister_device(struct ib_device *device)

{

{

	struct ib_client *client;

	struct ib_client_data *context, *tmp;

	struct ib_client_data *context, *tmp;

	unsigned long flags;

	unsigned long flags;
@@ -347,20 +354,29 @@ void ib_unregister_device(struct ib_device *device) 




	down_write(&lists_rwsem);

	down_write(&lists_rwsem);

	list_del(&device->core_list);

	list_del(&device->core_list);

	up_write(&lists_rwsem);

	spin_lock_irqsave(&device->client_data_lock, flags);

	list_for_each_entry_safe(context, tmp, &device->client_data_list, list)

		context->going_down = true;

	spin_unlock_irqrestore(&device->client_data_lock, flags);

	downgrade_write(&lists_rwsem);





	list_for_each_entry_reverse(client, &client_list, list)

	list_for_each_entry_safe(context, tmp, &device->client_data_list,

		if (client->remove)

				 list) {

			client->remove(device);

		if (context->client->remove)

			context->client->remove(device, context->data);

	}

	up_read(&lists_rwsem);





	mutex_unlock(&device_mutex);

	mutex_unlock(&device_mutex);





	ib_device_unregister_sysfs(device);

	ib_device_unregister_sysfs(device);





	down_write(&lists_rwsem);

	spin_lock_irqsave(&device->client_data_lock, flags);

	spin_lock_irqsave(&device->client_data_lock, flags);

	list_for_each_entry_safe(context, tmp, &device->client_data_list, list)

	list_for_each_entry_safe(context, tmp, &device->client_data_list, list)

		kfree(context);

		kfree(context);

	spin_unlock_irqrestore(&device->client_data_lock, flags);

	spin_unlock_irqrestore(&device->client_data_lock, flags);

	up_write(&lists_rwsem);





	device->reg_state = IB_DEV_UNREGISTERED;

	device->reg_state = IB_DEV_UNREGISTERED;

}

}
@@ -420,16 +436,35 @@ void ib_unregister_client(struct ib_client *client) 
	up_write(&lists_rwsem);

	up_write(&lists_rwsem);





	list_for_each_entry(device, &device_list, core_list) {

	list_for_each_entry(device, &device_list, core_list) {

		if (client->remove)

		struct ib_client_data *found_context = NULL;

			client->remove(device);





		down_write(&lists_rwsem);

		spin_lock_irqsave(&device->client_data_lock, flags);

		spin_lock_irqsave(&device->client_data_lock, flags);

		list_for_each_entry_safe(context, tmp, &device->client_data_list, list)

		list_for_each_entry_safe(context, tmp, &device->client_data_list, list)

			if (context->client == client) {

			if (context->client == client) {

				list_del(&context->list);

				context->going_down = true;

				kfree(context);

				found_context = context;

				break;

			}

		spin_unlock_irqrestore(&device->client_data_lock, flags);

		up_write(&lists_rwsem);



		if (client->remove)

			client->remove(device, found_context ?

					       found_context->data : NULL);



		if (!found_context) {

			pr_warn("No client context found for %s/%s\n",

				device->name, client->name);

			continue;

		}

		}



		down_write(&lists_rwsem);

		spin_lock_irqsave(&device->client_data_lock, flags);

		list_del(&found_context->list);

		kfree(found_context);

		spin_unlock_irqrestore(&device->client_data_lock, flags);

		spin_unlock_irqrestore(&device->client_data_lock, flags);

		up_write(&lists_rwsem);

	}

	}





	mutex_unlock(&device_mutex);

	mutex_unlock(&device_mutex);

drivers/infiniband/core/mad.c

+1 −1
Original line number Original line Diff line number Diff line
@@ -3335,7 +3335,7 @@ static void ib_mad_init_device(struct ib_device *device) 
	}

	}

}

}





static void ib_mad_remove_device(struct ib_device *device)

static void ib_mad_remove_device(struct ib_device *device, void *client_data)

{

{

	int i;

	int i;