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
| ||
|
Message-ID: <8105.1556207127@warthog.procyon.org.uk> Date: Thu, 25 Apr 2019 16:45:27 +0100 From: David Howells <dhowells@...hat.com> To: Al Viro <viro@...iv.linux.org.uk> Cc: dhowells@...hat.com, linux-afs@...ts.infradead.org, linux-ext4@...r.kernel.org, linux-ntfs-dev@...ts.sourceforge.net, linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH 3/6] vfs: Allow searching of the icache under RCU conditions [ver #2] Al Viro <viro@...iv.linux.org.uk> wrote: > Hmm... Why do these stores to ->i_state need WRITE_ONCE, while an arseload > of similar in fs/fs-writeback.c does not? Because what matters in find_inode_rcu() are the I_WILL_FREE and I_FREEING flags - and there's a gap during iput_final() where neither is set. if (!drop) { inode->i_state |= I_WILL_FREE; spin_unlock(&inode->i_lock); write_inode_now(inode, 1); spin_lock(&inode->i_lock); WARN_ON(inode->i_state & I_NEW); inode->i_state &= ~I_WILL_FREE; ---> } inode->i_state |= I_FREEING; It's normally covered by i_lock, but it's a problem if anyone looks at the pair without taking i_lock. Even flipping the order: if (!drop) { inode->i_state |= I_WILL_FREE; spin_unlock(&inode->i_lock); write_inode_now(inode, 1); spin_lock(&inode->i_lock); WARN_ON(inode->i_state & I_NEW); inode->i_state |= I_FREEING; inode->i_state &= ~I_WILL_FREE; } else { inode->i_state |= I_FREEING; } isn't a guarantee of the order in which the compiler will do things AIUI. Maybe I've been listening to Paul McKenney too much. So the WRITE_ONCE() should guarantee that both bits will change atomically. Note that ocfs2_drop_inode() looks a tad suspicious: int ocfs2_drop_inode(struct inode *inode) { struct ocfs2_inode_info *oi = OCFS2_I(inode); trace_ocfs2_drop_inode((unsigned long long)oi->ip_blkno, inode->i_nlink, oi->ip_flags); assert_spin_locked(&inode->i_lock); inode->i_state |= I_WILL_FREE; spin_unlock(&inode->i_lock); write_inode_now(inode, 1); spin_lock(&inode->i_lock); WARN_ON(inode->i_state & I_NEW); inode->i_state &= ~I_WILL_FREE; return 1; } David
Powered by blists - more mailing lists