All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexey Dobriyan <adobriyan@openvz.org>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexey Dobriyan <adobriyan@gmail.com>,
	viro@ftp.linux.org.uk, linux-kernel@vger.kernel.org,
	duncan.sands@math.u-psud.fr
Subject: Re: [PATCH v4] Fix rmmod/read/write races in /proc entries
Date: Tue, 13 Feb 2007 19:16:27 +0300	[thread overview]
Message-ID: <20070213161627.GC6036@localhost.sw.ru> (raw)
In-Reply-To: <20070212223552.59d733b1.akpm@linux-foundation.org>

On Mon, Feb 12, 2007 at 10:35:52PM -0800, Andrew Morton wrote:
> On Sun, 11 Feb 2007 23:23:30 +0300 Alexey Dobriyan <adobriyan@gmail.com> wrote:
> 
> > [PATCH v4] Fix rmmod/read/write races in /proc entries
> 
> This:
> 
> static ssize_t
> proc_file_write(struct file *file, const char __user *buffer,
> 		size_t count, loff_t *ppos)
> {
> 	struct inode *inode = file->f_path.dentry->d_inode;
> 	struct proc_dir_entry * dp;
> 	ssize_t rv = -EIO;
> 	
> 	dp = PDE(inode);
> 
> 	if (!dp->write_proc)
> 		goto out;
> 
> 	spin_lock(&dp->pde_unload_lock);
> 	if (!dp->proc_fops)
> 		/*
> 		 * remove_proc_entry() marked PDE as "going away".
> 		 * No new writers allowed.
> 		 */
> 		goto out_unlock;
> 
> versus
> 
> 		spin_lock(&de->pde_unload_lock);
> 		/*
> 		 * Stop accepting new readers/writers. If you're dynamically
> 		 * allocating ->proc_fops, save a pointer somewhere.
> 		 */
> 		de->proc_fops = NULL;
> 		/* Wait until all existing readers/writers are done. */
> 		if (de->pde_users > 0) {
> 			struct completion c;
>
> 			init_completion(&c);
> 			if (!de->pde_unload_completion)
> 				de->pde_unload_completion = &c;
>
> 			spin_unlock(&de->pde_unload_lock);
> 			spin_unlock(&proc_subdir_lock);
>
> 			wait_for_completion(de->pde_unload_completion);
>
> 			spin_lock(&proc_subdir_lock);
> 			goto continue_removing;
> 		}
> 		spin_unlock(&de->pde_unload_lock);
>   <here>
> 	...
> 	<free de>
>
> What prevents proc_file_write() from looking up and playing with this de in
> <here>?

If I understood your two-column diagram correctly, scenario below can't
happen because of PDE's own refcount (->count) and existence of
->deleted (0/1)

remove_proc_entry() sees positive ->count and doesn't immediately free
PDE. remove_proc_entry() will at most a) lock b) access to check
->proc_fops which is NULL now, and c) unlock which is fine because
memory is in place.

->count is bumped in proc_get_inode after checking PDEs lists, but our
PDE was already removed from it.


      reply	other threads:[~2007-02-13 16:09 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-02-08 13:20 [PATCH v3] Fix rmmod/read/write races in /proc entries Alexey Dobriyan
2007-02-09  9:00 ` Andrew Morton
2007-02-11 20:23   ` [PATCH v4] " Alexey Dobriyan
2007-02-11 20:34     ` Al Viro
2007-02-13  6:35     ` Andrew Morton
2007-02-13 16:16       ` Alexey Dobriyan [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20070213161627.GC6036@localhost.sw.ru \
    --to=adobriyan@openvz.org \
    --cc=adobriyan@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=duncan.sands@math.u-psud.fr \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@ftp.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.