linux-ext4 - Re: [RFC] inode.i_opflags - Usage of two different locking schemes

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 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]

Date:   Tue, 16 Mar 2021 18:14:29 +0100
From:   Jan Kara <jack@...e.cz>
To:     Alexander Lochmann <alexander.lochmann@...dortmund.de>
Cc:     Theodore Ts'o <tytso@....edu>,
        Horst Schirmeier <horst.schirmeier@...dortmund.de>,
        Jan Kara <jack@...e.cz>, Jan Kara <jack@...e.com>,
        linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC] inode.i_opflags - Usage of two different locking schemes

On Mon 08-03-21 15:05:33, Alexander Lochmann wrote:
> On 05.03.21 17:04, Theodore Ts'o wrote:
> > On Fri, Mar 05, 2021 at 04:35:47PM +0100, Alexander Lochmann wrote:
> > > 
> > > 
> > > On 05.03.21 16:18, Theodore Ts'o wrote:
> > > > 1)  I don't see where i_opflags is being read in ipc/mqueue.c at all,
> > > > either with or without i_rwsem.
> > > > 
> > > It is read in fs/dcache.c
> > 
> > So why is this unique to the mqueue inode then?  It might be helpful
> > to have explicit call stacks in the e-mail, in text form, when you
> > resend to LKML.
> It is unique to mqeue inode, because the control flow goes through
> ipc/mqueue.c where almost always the i_rwsem is taken.
> Hence, we see more memory accesses to an mqueue inode with the i_rwsem.
> The i_lock is less often hold compared to the i_rwsem.
> We conclude the i_rwsem is needed. So it might not be a contradiction at
> all. It rather could be a flaw in our approach. :-/
> 
> Besides from our current discussion:
> Does the i_lock protect i_opflags for both reading and writing?

So i_lock is supposed to protect i_opflags for writing AFAICT. For reading
we don't seem to bother in some cases and I agree that is potentially
problematic. It is *mostly* OK because we initialize i_opflags when loading
inode into memory / adding it to dcache. But sometimes we also update them
while the inode is alive. Now this is fine for the particular flag we
update but in theory, if the compiler wants to screw us and stores
temporarily some nonsensical value in i_opflags we'd have a problem. This
is mostly a theoretical issue but eventually we probably want to fix this.

								Honza
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR