trelay: fix deadlock on remove
authorAli MJ Al-Nasrawy <alimjalnasrawy@gmail.com>
Wed, 25 Sep 2019 14:47:12 +0000 (17:47 +0300)
committerHauke Mehrtens <hauke@hauke-m.de>
Sat, 12 Oct 2019 21:51:29 +0000 (23:51 +0200)
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

index d09dc072464ca2c350a35f5a115fb90c9f85ed9f..3871ace070e7e83778f04c5395e5697a972b110c 100644 (file)
@@ -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,16 +110,25 @@ 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;
+       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();
-       ret = trelay_do_remove(tr);
+       list_for_each_entry_safe(tr, tmp, &trelay_devs, list)
+               if (tr->to_remove)
+                       trelay_do_remove(tr);
        rtnl_unlock();
 
-       if (ret < 0)
-                return ret;
-
-       return count;
+       return 0;
 }
 
 static const struct file_operations fops_remove = {
@@ -123,6 +136,7 @@ static const struct file_operations fops_remove = {
        .open = trelay_open,
        .write = trelay_remove_write,
        .llseek = default_llseek,
+       .release = trelay_remove_release,
 };