Virtualization - Jun 2020 - [PATCH v4 0/3] mm, treewide: Rename kzfree() to kfree_sensitive()

On Tue, Jun 16, 2020 at 11:53:50AM -0700, Joe Perches
wrote:
> On Mon, 2020-06-15 at 21:57 -0400, Waiman Long wrote:
> >  v4:
> >   - Break out the memzero_explicit() change as suggested by Dan
Carpenter
> >     so that it can be backported to stable.
> >   - Drop the "crypto: Remove unnecessary memzero_explicit()"
patch for
> >     now as there can be a bit more discussion on what is best. It will
be
> >     introduced as a separate patch later on after this one is merged.
> 
> To this larger audience and last week without reply:
> https://lore.kernel.org/lkml/573b3fbd5927c643920e1364230c296b23e7584d.camel
at perches.com/
> 
> Are there _any_ fastpath uses of kfree or vfree?
I'd consider kfree performance critical for cases where it is called
under locks. If possible the kfree is moved outside of the critical
section, but we have rbtrees or lists that get deleted under locks and
restructuring the code to do eg. splice and free it outside of the lock
is not always possible.

On Wed, Jun 17, 2020 at 01:01:30AM +0200, David Sterba
wrote:
> On Tue, Jun 16, 2020 at 11:53:50AM -0700, Joe Perches wrote:
> > On Mon, 2020-06-15 at 21:57 -0400, Waiman Long wrote:
> > >  v4:
> > >   - Break out the memzero_explicit() change as suggested by Dan
Carpenter
> > >     so that it can be backported to stable.
> > >   - Drop the "crypto: Remove unnecessary
memzero_explicit()" patch for
> > >     now as there can be a bit more discussion on what is best. It
will be
> > >     introduced as a separate patch later on after this one is
merged.
> > 
> > To this larger audience and last week without reply:
> >
https://lore.kernel.org/lkml/573b3fbd5927c643920e1364230c296b23e7584d.camel at
perches.com/
> > 
> > Are there _any_ fastpath uses of kfree or vfree?
> 
> I'd consider kfree performance critical for cases where it is called
> under locks. If possible the kfree is moved outside of the critical
> section, but we have rbtrees or lists that get deleted under locks and
> restructuring the code to do eg. splice and free it outside of the lock
> is not always possible.
Not just performance critical, but correctness critical.  Since kvfree()
may allocate from the vmalloc allocator, I really think that kvfree()
should assert that it's !in_atomic().  Otherwise we can get into trouble
if we end up calling vfree() and have to take the mutex.

On Tue 16-06-20 17:37:11, Matthew Wilcox wrote:
> On Wed, Jun 17, 2020 at 01:01:30AM +0200, David Sterba wrote:
> > On Tue, Jun 16, 2020 at 11:53:50AM -0700, Joe Perches wrote:
> > > On Mon, 2020-06-15 at 21:57 -0400, Waiman Long wrote:
> > > >  v4:
> > > >   - Break out the memzero_explicit() change as suggested by
Dan Carpenter
> > > >     so that it can be backported to stable.
> > > >   - Drop the "crypto: Remove unnecessary
memzero_explicit()" patch for
> > > >     now as there can be a bit more discussion on what is
best. It will be
> > > >     introduced as a separate patch later on after this one
is merged.
> > > 
> > > To this larger audience and last week without reply:
> > >
https://lore.kernel.org/lkml/573b3fbd5927c643920e1364230c296b23e7584d.camel at
perches.com/
> > > 
> > > Are there _any_ fastpath uses of kfree or vfree?
> > 
> > I'd consider kfree performance critical for cases where it is
called
> > under locks. If possible the kfree is moved outside of the critical
> > section, but we have rbtrees or lists that get deleted under locks and
> > restructuring the code to do eg. splice and free it outside of the
lock
> > is not always possible.
> 
> Not just performance critical, but correctness critical.  Since kvfree()
> may allocate from the vmalloc allocator, I really think that kvfree()
> should assert that it's !in_atomic().  Otherwise we can get into
trouble
> if we end up calling vfree() and have to take the mutex.
FWIW __vfree already checks for atomic context and put the work into a
deferred context. So this should be safe. It should be used as a last
resort, though.

-- 
Michal Hocko
SUSE Labs