linux-kernel - Re: [PATCH RFC] ext4: fix potential race between online resizing and write operations

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: <20200221131455.GA4904@pc636>
Date:   Fri, 21 Feb 2020 14:14:55 +0100
From:   Uladzislau Rezki <urezki@...il.com>
To:     "Paul E. McKenney" <paulmck@...nel.org>,
        "Theodore Y. Ts'o" <tytso@....edu>,
        Joel Fernandes <joel@...lfernandes.org>
Cc:     "Theodore Y. Ts'o" <tytso@....edu>,
        Uladzislau Rezki <urezki@...il.com>,
        Joel Fernandes <joel@...lfernandes.org>,
        Ext4 Developers List <linux-ext4@...r.kernel.org>,
        Suraj Jitindar Singh <surajjs@...zon.com>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH RFC] ext4: fix potential race between online resizing and
 write operations

On Thu, Feb 20, 2020 at 04:30:35PM -0800, Paul E. McKenney wrote:
> On Wed, Feb 19, 2020 at 11:52:33PM -0500, Theodore Y. Ts'o wrote:
> > On Tue, Feb 18, 2020 at 06:08:57PM +0100, Uladzislau Rezki wrote:
> > > now it becomes possible to use it like: 
> > > 	...
> > > 	void *p = kvmalloc(PAGE_SIZE);
> > > 	kvfree_rcu(p);
> > > 	...
> > > also have a look at the example in the mm/list_lru.c diff.
> > 
> > I certainly like the interface, thanks!  I'm going to be pushing
> > patches to fix this using ext4_kvfree_array_rcu() since there are a
> > number of bugs in ext4's online resizing which appear to be hitting
> > multiple cloud providers (with reports from both AWS and GCP) and I
> > want something which can be easily backported to stable kernels.
> > 
> > But once kvfree_rcu() hits mainline, I'll switch ext4 to use it, since
> > your kvfree_rcu() is definitely more efficient than my expedient
> > jury-rig.
> > 
> > I don't feel entirely competent to review the implementation, but I do
> > have one question.  It looks like the rcutiny implementation of
> > kfree_call_rcu() isn't going to do the right thing with kvfree_rcu(p).
> > Am I missing something?
> 
> Good catch!  I believe that rcu_reclaim_tiny() would need to do
> kvfree() instead of its current kfree().
> 
> Vlad, anything I am missing here?
>
Yes something like that. There are some open questions about
realization, when it comes to tiny RCU. Since we are talking
about "headless" kvfree_rcu() interface, i mean we can not link
freed "objects" between each other, instead we should place a
pointer directly into array that will be drained later on.

It would be much more easier to achieve that if we were talking
about the interface like: kvfree_rcu(p, rcu), but that is not our
case :)

So, for CONFIG_TINY_RCU we should implement very similar what we
have done for CONFIG_TREE_RCU or just simply do like Ted has done
with his

void ext4_kvfree_array_rcu(void *to_free)

i mean:

   local_irq_save(flags);
   struct foo *ptr = kzalloc(sizeof(*ptr), GFP_ATOMIC);

   if (ptr) {
           ptr->ptr = to_free;
           call_rcu(&ptr->rcu, kvfree_callback);
   }
   local_irq_restore(flags);

Also there is one more open question what to do if GFP_ATOMIC
gets failed in case of having low memory condition. Probably
we can make use of "mempool interface" that allows to have
min_nr guaranteed pre-allocated pages. 

Thanks!

--
Vlad Rezki