deal with races between remove_proc_entry() and proc_reg_release()
authorAl Viro <viro@zeniv.linux.org.uk>
Wed, 3 Apr 2013 23:57:00 +0000 (19:57 -0400)
committerAl Viro <viro@zeniv.linux.org.uk>
Tue, 9 Apr 2013 19:16:51 +0000 (15:16 -0400)
* serialize the call of ->release() on per-pdeo mutex
* don't remove pdeo from per-pde list until we are through with it

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
fs/proc/inode.c
fs/proc/internal.h

index 0cd9d80f28e8aaa1a5278914c762392cd487edc4..b5b204d6b07f1c9bc85ea2b071759f063ba58e08 100644 (file)
@@ -156,6 +156,29 @@ static void unuse_pde(struct proc_dir_entry *pde)
        spin_unlock(&pde->pde_unload_lock);
 }
 
+/* pde is locked */
+static void close_pdeo(struct proc_dir_entry *pde, struct pde_opener *pdeo)
+{
+       pdeo->count++;
+       if (!mutex_trylock(&pdeo->mutex)) {
+               /* somebody else is doing that, just wait */
+               spin_unlock(&pde->pde_unload_lock);
+               mutex_lock(&pdeo->mutex);
+               spin_lock(&pde->pde_unload_lock);
+               WARN_ON(!list_empty(&pdeo->lh));
+       } else {
+               struct file *file;
+               spin_unlock(&pde->pde_unload_lock);
+               file = pdeo->file;
+               pde->proc_fops->release(file_inode(file), file);
+               spin_lock(&pde->pde_unload_lock);
+               list_del_init(&pdeo->lh);
+       }
+       mutex_unlock(&pdeo->mutex);
+       if (!--pdeo->count)
+               kfree(pdeo);
+}
+
 void proc_entry_rundown(struct proc_dir_entry *de)
 {
        spin_lock(&de->pde_unload_lock);
@@ -173,15 +196,8 @@ void proc_entry_rundown(struct proc_dir_entry *de)
 
        while (!list_empty(&de->pde_openers)) {
                struct pde_opener *pdeo;
-               struct file *file;
-
                pdeo = list_first_entry(&de->pde_openers, struct pde_opener, lh);
-               list_del(&pdeo->lh);
-               spin_unlock(&de->pde_unload_lock);
-               file = pdeo->file;
-               de->proc_fops->release(file_inode(file), file);
-               kfree(pdeo);
-               spin_lock(&de->pde_unload_lock);
+               close_pdeo(de, pdeo);
        }
        spin_unlock(&de->pde_unload_lock);
 }
@@ -357,6 +373,8 @@ static int proc_reg_open(struct inode *inode, struct file *file)
        spin_lock(&pde->pde_unload_lock);
        if (rv == 0 && release) {
                /* To know what to release. */
+               mutex_init(&pdeo->mutex);
+               pdeo->count = 0;
                pdeo->file = file;
                /* Strictly for "too late" ->release in proc_reg_release(). */
                list_add(&pdeo->lh, &pde->pde_openers);
@@ -367,58 +385,19 @@ static int proc_reg_open(struct inode *inode, struct file *file)
        return rv;
 }
 
-static struct pde_opener *find_pde_opener(struct proc_dir_entry *pde,
-                                       struct file *file)
-{
-       struct pde_opener *pdeo;
-
-       list_for_each_entry(pdeo, &pde->pde_openers, lh) {
-               if (pdeo->file == file)
-                       return pdeo;
-       }
-       return NULL;
-}
-
 static int proc_reg_release(struct inode *inode, struct file *file)
 {
        struct proc_dir_entry *pde = PDE(inode);
-       int rv = 0;
-       int (*release)(struct inode *, struct file *);
        struct pde_opener *pdeo;
-
        spin_lock(&pde->pde_unload_lock);
-       pdeo = find_pde_opener(pde, file);
-       if (pde->pde_users < 0) {
-               /*
-                * Can't simply exit, __fput() will think that everything is OK,
-                * and move on to freeing struct file. remove_proc_entry() will
-                * find slacker in opener's list and will try to do non-trivial
-                * things with struct file. Therefore, remove opener from list.
-                *
-                * But if opener is removed from list, who will ->release it?
-                */
-               if (pdeo) {
-                       list_del(&pdeo->lh);
-                       spin_unlock(&pde->pde_unload_lock);
-                       rv = pde->proc_fops->release(inode, file);
-                       kfree(pdeo);
-               } else
-                       spin_unlock(&pde->pde_unload_lock);
-               return rv;
-       }
-       pde->pde_users++;
-       release = pde->proc_fops->release;
-       if (pdeo) {
-               list_del(&pdeo->lh);
-               kfree(pdeo);
+       list_for_each_entry(pdeo, &pde->pde_openers, lh) {
+               if (pdeo->file == file) {
+                       close_pdeo(pde, pdeo);
+                       break;
+               }
        }
        spin_unlock(&pde->pde_unload_lock);
-
-       if (release)
-               rv = release(inode, file);
-
-       unuse_pde(pde);
-       return rv;
+       return 0;
 }
 
 static const struct file_operations proc_reg_file_ops = {
index c43d536f93b9df3e961a90e33cb0ac503f5c7e1c..e2fa9345a9a8b6986e6fa8d073fe7455ae43636a 100644 (file)
@@ -153,6 +153,8 @@ int proc_readdir_de(struct proc_dir_entry *de, struct file *filp, void *dirent,
 struct pde_opener {
        struct file *file;
        struct list_head lh;
+       int count;      /* number of threads in close_pdeo() */
+       struct mutex mutex;
 };
 
 ssize_t __proc_file_read(struct file *, char __user *, size_t, loff_t *);