proc: always do ->release

Current two-stage scheme of removing PDE emphasizes one bug in proc:

		open
				rmmod
				remove_proc_entry
		close

->release won't be called because ->proc_fops were cleared.  In simple
cases it's small memory leak.

For every ->open, ->release has to be done.  List of openers is introduced
which is traversed at remove_proc_entry() if neeeded.

Discussions with Al long ago (sigh).

Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

1 files changed, 70 insertions, 4 deletions

diff --git a/fs/proc/inode.c b/fs/proc/inode.c
index b08d10017911..354c08485825 100644
--- a/fs/proc/inode.c
+++ b/fs/proc/inode.c
@@ -126,12 +126,17 @@ static const struct super_operations proc_sops = {
         .remount_fs     = proc_remount,
 };
 
-static void pde_users_dec(struct proc_dir_entry *pde)
+static void __pde_users_dec(struct proc_dir_entry *pde)
 {
-        spin_lock(&pde->pde_unload_lock);
         pde->pde_users--;
         if (pde->pde_unload_completion && pde->pde_users == 0)
                 complete(pde->pde_unload_completion);
+}
+
+static void pde_users_dec(struct proc_dir_entry *pde)
+{
+        spin_lock(&pde->pde_unload_lock);
+        __pde_users_dec(pde);
         spin_unlock(&pde->pde_unload_lock);
 }
 
@@ -318,36 +323,97 @@ static int proc_reg_open(struct inode *inode, struct file *file)
         struct proc_dir_entry *pde = PDE(inode);
         int rv = 0;
         int (*open)(struct inode *, struct file *);
+        int (*release)(struct inode *, struct file *);
+        struct pde_opener *pdeo;
+
+        /*
+         * What for, you ask? Well, we can have open, rmmod, remove_proc_entry
+         * sequence. ->release won't be called because ->proc_fops will be
+         * cleared. Depending on complexity of ->release, consequences vary.
+         *
+         * We can't wait for mercy when close will be done for real, it's
+         * deadlockable: rmmod foo </proc/foo . So, we're going to do ->release
+         * by hand in remove_proc_entry(). For this, save opener's credentials
+         * for later.
+         */
+        pdeo = kmalloc(sizeof(struct pde_opener), GFP_KERNEL);
+        if (!pdeo)
+                return -ENOMEM;
 
         spin_lock(&pde->pde_unload_lock);
         if (!pde->proc_fops) {
                 spin_unlock(&pde->pde_unload_lock);
+                kfree(pdeo);
                 return rv;
         }
         pde->pde_users++;
         open = pde->proc_fops->open;
+        release = pde->proc_fops->release;
         spin_unlock(&pde->pde_unload_lock);
 
         if (open)
                 rv = open(inode, file);
 
-        pde_users_dec(pde);
+        spin_lock(&pde->pde_unload_lock);
+        if (rv == 0 && release) {
+                /* To know what to release. */
+                pdeo->inode = inode;
+                pdeo->file = file;
+                /* Strictly for "too late" ->release in proc_reg_release(). */
+                pdeo->release = release;
+                list_add(&pdeo->lh, &pde->pde_openers);
+        } else
+                kfree(pdeo);
+        __pde_users_dec(pde);
+        spin_unlock(&pde->pde_unload_lock);
         return rv;
 }
 
+static struct pde_opener *find_pde_opener(struct proc_dir_entry *pde,
+                                        struct inode *inode, struct file *file)
+{
+        struct pde_opener *pdeo;
+
+        list_for_each_entry(pdeo, &pde->pde_openers, lh) {
+                if (pdeo->inode == inode && 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, inode, file);
         if (!pde->proc_fops) {
-                spin_unlock(&pde->pde_unload_lock);
+                /*
+                 * 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 = pdeo->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);
+        }
         spin_unlock(&pde->pde_unload_lock);
 
         if (release)