linux-kernel - Re: The bug of iput() removal from flusher thread?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20121121011111.GE10507@quack.suse.cz>
Date:	Wed, 21 Nov 2012 02:11:11 +0100
From:	Jan Kara <jack@...e.cz>
To:	OGAWA Hirofumi <hirofumi@...l.parknet.co.jp>
Cc:	Jan Kara <jack@...e.cz>, Al Viro <viro@...iv.linux.org.uk>,
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: The bug of iput() removal from flusher thread?

On Tue 20-11-12 06:53:12, OGAWA Hirofumi wrote:
> Jan Kara <jack@...e.cz> writes:
> 
> >> >  static void inode_sync_complete(struct inode *inode)
> >> >  {
> >> > +	/* If inode is clean an unused, put it into LRU now.  */
> >> > +	if (!(inode->i_state & I_DIRTY) && !atomic_read(&inode->i_count))
> >> > +		inode_lru_list_add(inode);
> >> 
> >> IMHO, open coding this would be bad idea.
> >   Do you mean creating a separate function for the above two lines?
> 
> Yes. And the intent is to consolidate "when adds inode to LRU" with
> iput_final()'s one.
> 
> >> And another one is I_REFERENCED. We really want to remove I_REFERENCED?
> >   We don't want I_REFERENCED set - noone used the inode. But looking into
> > the code with fresh eyes, the fix isn't as simple as I thought. First I
> > need to check MS_ACTIVE and second I need to check I_FREEING... So the
> > condition will be complex enough to warrant a separate function.
> 
> I can't see the issue (sync_filesystem() will wait I_DIRTY before
> MS_ACTIVE, and I_DIRTY prevents I_FREEING) though, it may be possible.
  E.g. when inode is deleted it can be both I_DIRTY (and flusher thread
can be working on it) while it is also marked as I_FREEING. In such case we
must avoid adding the inode to the LRU.

  Regarding MS_ACTIVE - you are right that sync_filesystem() should clean
all dirty inodes but some filesystems dirty their internal inodes during
umount so it's better to make flusher thread safe and not add such inodes
to the LRU during umount.

								Honza
-- 
Jan Kara <jack@...e.cz>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/