From: "Mika Penttilä" <mika.penttila@kolumbus.fi>
To: Dave Peterson <dsp@llnl.gov>
Cc: linux-kernel@vger.kernel.org
Subject: Re: possible race condition in vfs code
Date: Wed, 30 Apr 2003 01:51:42 +0300 [thread overview]
Message-ID: <3EAF01FE.2040600@kolumbus.fi> (raw)
In-Reply-To: 200304150922.07003.dsp@llnl.gov
That piece of code looks wrong in other ways also..if we have unmounted
an active fs (which shouldn't be done but happens!) we shouldn't be at
least writing back to it anything! The !sb test isn't useful (we never
clear it in live inodes) and MS_ACTIVE handling is racy as hell wrt
umount...
--Mika
Dave Peterson wrote:
>I think I found a race condition in some of the inode handling
>code. Here's where I think it is:
>
> 1. Task A is inside kill_super(). It clears the MS_ACTIVE
> flag of the s_flags field of the super_block struct and
> calls invalidate_inodes(). In invalidate_inodes(), it
> attempts to acquire inode_lock and spins because task B,
> executing inside iput(), just decremented the reference
> count of some inode i to 0 and acquired inode_lock.
>
> 2. Then the "if (!inode->i_nlink)" test condition evaluates
> to false for task B so it executes the following chunk
> of code:
>
> 01 } else {
> 02 if (!list_empty(&inode->i_hash)) {
> 03 if (!(inode->i_state & (I_DIRTY|I_LOCK))) {
> 04 list_del(&inode->i_list);
> 05 list_add(&inode->i_list, &inode_unused);
> 06 }
> 07 inodes_stat.nr_unused++;
> 08 spin_unlock(&inode_lock);
> 09 if (!sb || (sb->s_flags & MS_ACTIVE))
> 10 return;
> 11 write_inode_now(inode, 1);
> 12 spin_lock(&inode_lock);
> 13 inodes_stat.nr_unused--;
> 14 list_del_init(&inode->i_hash);
> 15 }
> 16 list_del_init(&inode->i_list);
> 17 inode->i_state|=I_FREEING;
> 18 inodes_stat.nr_inodes--;
> 19 spin_unlock(&inode_lock);
> 20 if (inode->i_data.nrpages)
> 21 truncate_inode_pages(&inode->i_data, 0);
> 22 clear_inode(inode);
> 23 }
> 24 destroy_inode(inode);
>
> Now the test condition on line 02 evaluates to true, so
> task B adds the inode to the inode_unused list and then
> releases inode_lock on line 08.
>
> 3. Now A acquires inode_lock and B spins on inode_lock inside
> write_inode_now();
>
> 4. Task A calls invalidate_list(), transferring inode i from
> the inode_unused list to its own private throw_away list.
>
> 5. Task A releases inode_lock, allowing B to acquire inode_lock
> and continue executing.
>
> 6. A attempts to destroy inode i inside dispose_list() while B
> simultaneously attempts to destroy i, potentially causing
> all sorts of bad things to happen.
>
>So, did I find a bug or are my eyes playing tricks on me?
>
>-Dave Peterson
> dsp@llnl.gov
>
>P.S. Please CC my email address when responding to this message.
>
>-
>To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>Please read the FAQ at http://www.tux.org/lkml/
>
>
>
next prev parent reply other threads:[~2003-04-29 22:33 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2003-04-15 16:22 possible race condition in vfs code Dave Peterson
2003-04-29 22:51 ` Mika Penttilä [this message]
2003-04-29 23:03 ` viro
2003-04-30 6:52 ` Mika Penttilä
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=3EAF01FE.2040600@kolumbus.fi \
--to=mika.penttila@kolumbus.fi \
--cc=dsp@llnl.gov \
--cc=linux-kernel@vger.kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).