trelay: fix deadlock on remove
Upon writing to "remove" file, debugfs_remove_recursive() blocks while
holding rtnl_lock. This is because debugfs' file_ops callbacks are
executed in debugfs_use_file_*() context which prevents file removal.
Fix this by only flagging the device for removal and then do the cleanup
in file_ops.release callback which is executed out of that context.
Signed-off-by: Ali MJ Al-Nasrawy <alimjalnasrawy@gmail.com>
(cherry picked from commit c2635b871d)
			
			
This commit is contained in:
		 Ali MJ Al-Nasrawy
					Ali MJ Al-Nasrawy
				
			
				
					committed by
					
						 Hauke Mehrtens
						Hauke Mehrtens
					
				
			
			
				
	
			
			
			 Hauke Mehrtens
						Hauke Mehrtens
					
				
			
						parent
						
							9784a470bb
						
					
				
				
					commit
					b17c95bbdc
				
			| @@ -27,6 +27,7 @@ struct trelay { | |||||||
| 	struct list_head list; | 	struct list_head list; | ||||||
| 	struct net_device *dev1, *dev2; | 	struct net_device *dev1, *dev2; | ||||||
| 	struct dentry *debugfs; | 	struct dentry *debugfs; | ||||||
|  | 	int to_remove; | ||||||
| 	char name[]; | 	char name[]; | ||||||
| }; | }; | ||||||
|  |  | ||||||
| @@ -60,13 +61,16 @@ static int trelay_do_remove(struct trelay *tr) | |||||||
| { | { | ||||||
| 	list_del(&tr->list); | 	list_del(&tr->list); | ||||||
|  |  | ||||||
|  | 	/* First and before all, ensure that the debugfs file is removed | ||||||
|  | 	 * to prevent dangling pointer in file->private_data */ | ||||||
|  | 	debugfs_remove_recursive(tr->debugfs); | ||||||
|  |  | ||||||
| 	dev_put(tr->dev1); | 	dev_put(tr->dev1); | ||||||
| 	dev_put(tr->dev2); | 	dev_put(tr->dev2); | ||||||
|  |  | ||||||
| 	netdev_rx_handler_unregister(tr->dev1); | 	netdev_rx_handler_unregister(tr->dev1); | ||||||
| 	netdev_rx_handler_unregister(tr->dev2); | 	netdev_rx_handler_unregister(tr->dev2); | ||||||
|  |  | ||||||
| 	debugfs_remove_recursive(tr->debugfs); |  | ||||||
| 	kfree(tr); | 	kfree(tr); | ||||||
|  |  | ||||||
| 	return 0; | 	return 0; | ||||||
| @@ -106,23 +110,33 @@ static ssize_t trelay_remove_write(struct file *file, const char __user *ubuf, | |||||||
| 				   size_t count, loff_t *ppos) | 				   size_t count, loff_t *ppos) | ||||||
| { | { | ||||||
| 	struct trelay *tr = file->private_data; | 	struct trelay *tr = file->private_data; | ||||||
| 	int ret; | 	tr->to_remove = 1; | ||||||
|  |  | ||||||
| 	rtnl_lock(); |  | ||||||
| 	ret = trelay_do_remove(tr); |  | ||||||
| 	rtnl_unlock(); |  | ||||||
|  |  | ||||||
| 	if (ret < 0) |  | ||||||
| 		 return ret; |  | ||||||
|  |  | ||||||
| 	return count; | 	return count; | ||||||
| } | } | ||||||
|  |  | ||||||
|  | static int trelay_remove_release(struct inode *inode, struct file *file) | ||||||
|  | { | ||||||
|  | 	struct trelay *tr, *tmp; | ||||||
|  |  | ||||||
|  | 	/* This is the only file op that is called outside debugfs_use_file_*() | ||||||
|  | 	 * context which means that: (1) this file can be removed and | ||||||
|  | 	 * (2) file->private_data may no longer be valid */ | ||||||
|  | 	rtnl_lock(); | ||||||
|  | 	list_for_each_entry_safe(tr, tmp, &trelay_devs, list) | ||||||
|  | 		if (tr->to_remove) | ||||||
|  | 			trelay_do_remove(tr); | ||||||
|  | 	rtnl_unlock(); | ||||||
|  |  | ||||||
|  | 	return 0; | ||||||
|  | } | ||||||
|  |  | ||||||
| static const struct file_operations fops_remove = { | static const struct file_operations fops_remove = { | ||||||
| 	.owner = THIS_MODULE, | 	.owner = THIS_MODULE, | ||||||
| 	.open = trelay_open, | 	.open = trelay_open, | ||||||
| 	.write = trelay_remove_write, | 	.write = trelay_remove_write, | ||||||
| 	.llseek = default_llseek, | 	.llseek = default_llseek, | ||||||
|  | 	.release = trelay_remove_release, | ||||||
| }; | }; | ||||||
|  |  | ||||||
|  |  | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user