[OpenWrt-Devel,2/3] kernel: trelay: fix deadlock on remove
diff mbox series

Message ID 20190925144713.10632-2-alimjalnasrawy@gmail.com
State Accepted
Delegated to: Hauke Mehrtens
Headers show
Series
  • [OpenWrt-Devel,1/3] kernel: trelay: handle netdevice events correctly
Related show

Commit Message

Ali MJ Al-Nasrawy Sept. 25, 2019, 2:47 p.m. UTC
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>
---
 package/kernel/trelay/src/trelay.c | 32 +++++++++++++++++++++---------
 1 file changed, 23 insertions(+), 9 deletions(-)

Patch
diff mbox series

diff --git a/package/kernel/trelay/src/trelay.c b/package/kernel/trelay/src/trelay.c
index 6d9d9cc14b..0e3d85bfef 100644
--- a/package/kernel/trelay/src/trelay.c
+++ b/package/kernel/trelay/src/trelay.c
@@ -27,6 +27,7 @@  struct trelay {
 	struct list_head list;
 	struct net_device *dev1, *dev2;
 	struct dentry *debugfs;
+	int to_remove;
 	char name[];
 };
 
@@ -60,13 +61,16 @@  static int trelay_do_remove(struct trelay *tr)
 {
 	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->dev2);
 
 	netdev_rx_handler_unregister(tr->dev1);
 	netdev_rx_handler_unregister(tr->dev2);
 
-	debugfs_remove_recursive(tr->debugfs);
 	kfree(tr);
 
 	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)
 {
 	struct trelay *tr = file->private_data;
-	int ret;
-
-	rtnl_lock();
-	ret = trelay_do_remove(tr);
-	rtnl_unlock();
-
-	if (ret < 0)
-		 return ret;
+	tr->to_remove = 1;
 
 	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 = {
 	.owner = THIS_MODULE,
 	.open = trelay_open,
 	.write = trelay_remove_write,
 	.llseek = default_llseek,
+	.release = trelay_remove_release,
 };