linux-kernel - Re: [RFC PATCH] jbd2: avoid __GFP_ZERO with SLAB_TYPESAFE_BY

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <YgU1ONDsn9xgs/CG@mit.edu>
Date:   Thu, 10 Feb 2022 10:54:32 -0500
From:   "Theodore Ts'o" <tytso@....edu>
To:     "Paul E. McKenney" <paulmck@...nel.org>
Cc:     Jan Kara <jack@...e.cz>, Qian Cai <quic_qiancai@...cinc.com>,
        Jan Kara <jack@...e.com>,
        Neeraj Upadhyay <quic_neeraju@...cinc.com>,
        Joel Fernandes <joel@...lfernandes.org>,
        Boqun Feng <boqun.feng@...il.com>, linux-ext4@...r.kernel.org,
        rcu@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] jbd2: avoid __GFP_ZERO with SLAB_TYPESAFE_BY_RCU

On Wed, Feb 09, 2022 at 09:43:57PM -0800, Paul E. McKenney wrote:
> So the typical RCU approach (not involving SLAB_TYPESAFE_BY_RCU)
> is to take the grace period at the time of the free.  This can be
> done synchronously using synchronize_rcu(), but is often instead done
> asynchronously using call_rcu() or kfree_rcu().  So in this case,
> you don't need synchronize_rcu() on allocation because the required
> grace period already happened at *free() time.
> 
> But there are a few situations where it makes sense to free blocks that
> readers might still be referencing.  Readers must then add validity
> checks to detect this case, and also prevent freeing, for example,
> using a per-block spinlock for synchronization.  For example, a reader
> might acquire a spinlock in the block to prevent changes, recheck the
> lookup key, and if the key does not match, release the lock and pretend
> not to have found the block.  If the key does match, anything attempting
> to delete and free the block will be spinning on that same spinlock.
> 
> And so if you specify SLAB_TYPESAFE_BY_RCU, the slab allocator is
> guaranteeing type safety to RCU readers instead of the usual existence
> guarantee.  A memory block might be freed out from under an RCU reader,
> but its type will remain the same.  This means that the grace period
> happens internally to the slab allocator when a slab is returned to
> the system.
> 
> So either the validation checks are quite novel, the kmem_cache_zalloc()
> calls should be replaced by kmem_cache_alloc() plus validation checks,
> or the SLAB_TYPESAFE_BY_RCU should be removed.
> 
> Just out of curiosity, what is your mental model of SLAB_TYPESAFE_BY_RCU?

Hmm, so the code in question the flag was called SLAB_DESTROY_BY_RCU
in June 205 by commit de92c8caf16c ("jbd2: speedup
jbd2_journal_get_[write|undo]_access()"), and it was written by Jan.
I don't see anything to make sure the jh doesn't get freed until after
the grace period, and so that looks like a problem unless I'm missing
something.   Jan, what do you think?

					- Ted