Thread (5 messages) 5 messages, 2 authors, 2020-06-16

Re: [PATCH v5 2/2] mm, treewide: Rename kzfree() to kfree_sensitive()

From: Waiman Long <longman@redhat.com>
Date: 2020-06-16 18:36:41
Also in: keyrings, linux-amlogic, linux-bluetooth, linux-cifs, linux-crypto, linux-fscrypt, linux-integrity, linux-mediatek, linux-mm, linux-nfs, linux-pm, linux-scsi, linux-sctp, linux-security-module, linux-wireless, linuxppc-dev, lkml, netdev, target-devel

On 6/16/20 2:09 PM, Andrew Morton wrote:
On Tue, 16 Jun 2020 11:43:11 -0400 Waiman Long [off-list ref] wrote:
quoted
As said by Linus:

   A symmetric naming is only helpful if it implies symmetries in use.
   Otherwise it's actively misleading.

   In "kzalloc()", the z is meaningful and an important part of what the
   caller wants.

   In "kzfree()", the z is actively detrimental, because maybe in the
   future we really _might_ want to use that "memfill(0xdeadbeef)" or
   something. The "zero" part of the interface isn't even _relevant_.

The main reason that kzfree() exists is to clear sensitive information
that should not be leaked to other future users of the same memory
objects.

Rename kzfree() to kfree_sensitive() to follow the example of the
recently added kvfree_sensitive() and make the intention of the API
more explicit. In addition, memzero_explicit() is used to clear the
memory to make sure that it won't get optimized away by the compiler.

The renaming is done by using the command sequence:

   git grep -w --name-only kzfree |\
   xargs sed -i 's/\bkzfree\b/kfree_sensitive/'

followed by some editing of the kfree_sensitive() kerneldoc and adding
a kzfree backward compatibility macro in slab.h.

...
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -186,10 +186,12 @@ void memcg_deactivate_kmem_caches(struct mem_cgroup *, struct mem_cgroup *);
   */
  void * __must_check krealloc(const void *, size_t, gfp_t);
  void kfree(const void *);
-void kzfree(const void *);
+void kfree_sensitive(const void *);
  size_t __ksize(const void *);
  size_t ksize(const void *);
  
+#define kzfree(x)	kfree_sensitive(x)	/* For backward compatibility */
+
What was the thinking here?  Is this really necessary?

I suppose we could keep this around for a while to ease migration.  But
not for too long, please.
It should be there just for 1 release cycle. I have broken out the btrfs 
patch to the btrfs list and I didn't make the kzfree to kfree_sensitive 
conversion there as that patch was in front in my patch list. So 
depending on which one lands first, there can be a window where the 
compilation may fail without this workaround. I am going to send out 
another patch in the next release cycle to remove it.

Cheers,
Longman
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help