linux-ext4 - 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 for Android: free password hash cracker in your pocket

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

Date:   Wed, 9 Feb 2022 12:11:37 -0800
From:   "Paul E. McKenney" <paulmck@...nel.org>
To:     Jan Kara <jack@...e.cz>
Cc:     Qian Cai <quic_qiancai@...cinc.com>, Theodore Ts'o <tytso@....edu>,
        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 07:10:10PM +0100, Jan Kara wrote:
> On Wed 09-02-22 11:57:42, Qian Cai wrote:
> > Since the linux-next commit 120aa5e57479 (mm: Check for
> > SLAB_TYPESAFE_BY_RCU and __GFP_ZERO slab allocation), we will get a
> > boot warning. Avoid it by calling synchronize_rcu() before the zeroing.
> > 
> > Signed-off-by: Qian Cai <quic_qiancai@...cinc.com>
> 
> No, the performance impact of this would be just horrible. Can you
> ellaborate a bit why SLAB_TYPESAFE_BY_RCU + __GFP_ZERO is a problem and why
> synchronize_rcu() would be needed here before the memset() please? I mean
> how is zeroing here any different from the memory just being used?

Suppose a reader picks up a pointer to a memory block, then that memory
is freed.  No problem, given that this is a SLAB_TYPESAFE_BY_RCU slab,
so the memory won't be freed while the reader is accessing it.  But while
the reader is in the process of validating the block, it is zeroed.

How does the validation step handle this in all cases?

If you have a way of handling this, I will of course drop the patch.
And learn something new, which is always a good thing.  ;-)

							Thanx, Paul

> > ---
> >  fs/jbd2/journal.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> > index c2cf74b01ddb..323112de5921 100644
> > --- a/fs/jbd2/journal.c
> > +++ b/fs/jbd2/journal.c
> > @@ -2861,15 +2861,18 @@ static struct journal_head *journal_alloc_journal_head(void)
> >  #ifdef CONFIG_JBD2_DEBUG
> >  	atomic_inc(&nr_journal_heads);
> >  #endif
> > -	ret = kmem_cache_zalloc(jbd2_journal_head_cache, GFP_NOFS);
> > +	ret = kmem_cache_alloc(jbd2_journal_head_cache, GFP_NOFS);
> >  	if (!ret) {
> >  		jbd_debug(1, "out of memory for journal_head\n");
> >  		pr_notice_ratelimited("ENOMEM in %s, retrying.\n", __func__);
> > -		ret = kmem_cache_zalloc(jbd2_journal_head_cache,
> > +		ret = kmem_cache_alloc(jbd2_journal_head_cache,
> >  				GFP_NOFS | __GFP_NOFAIL);
> >  	}
> > -	if (ret)
> > +	if (ret) {
> > +		synchronize_rcu();
> > +		memset(ret, 0, sizeof(*ret));
> >  		spin_lock_init(&ret->b_state_lock);
> > +	}
> >  	return ret;
> >  }
> >  
> > -- 
> > 2.30.2
> > 
> -- 
> Jan Kara <jack@...e.com>
> SUSE Labs, CR