similar to: [PATCH v4 0/3] mm, treewide: Rename kzfree() to kfree_sensitive()

This patchset makes a global rename of the kzfree() to kfree_sensitive() to highlight the fact buffer clearing is only needed if the data objects contain sensitive information like encrpytion key. The fact that kzfree() uses memset() to do the clearing isn't totally safe either as compiler may compile out the clearing in their optimizer. Instead, the new kfree_sensitive() uses

v5: - Break the btrfs patch out as a separate patch to be processed independently. - Update the commit log of patch 1 to make it less scary. - Add a kzfree backward compatibility macro in patch 2. 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

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

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

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

Le 14/04/2020 ? 00:28, Waiman Long a ?crit?: > Since kfree_sensitive() will do an implicit memzero_explicit(), there > is no need to call memzero_explicit() before it. Eliminate those > memzero_explicit() and simplify the call sites. For better correctness, > the setting of keylen is also moved down after the key pointer check. > > Signed-off-by: Waiman Long <longman at

Le 14/04/2020 ? 00:28, Waiman Long a ?crit?: > Since kfree_sensitive() will do an implicit memzero_explicit(), there > is no need to call memzero_explicit() before it. Eliminate those > memzero_explicit() and simplify the call sites. For better correctness, > the setting of keylen is also moved down after the key pointer check. > > Signed-off-by: Waiman Long <longman at

On Mon, 2020-04-13 at 17:15 -0400, Waiman Long wrote: > Since kfree_sensitive() will do an implicit memzero_explicit(), there > is no need to call memzero_explicit() before it. Eliminate those > memzero_explicit() and simplify the call sites. 2 bits of trivia: > diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c []

On Mon, 2020-04-13 at 17:15 -0400, Waiman Long wrote: > Since kfree_sensitive() will do an implicit memzero_explicit(), there > is no need to call memzero_explicit() before it. Eliminate those > memzero_explicit() and simplify the call sites. 2 bits of trivia: > diff --git a/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c b/drivers/crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c []

On Tue, 16 Jun 2020 11:43:11 -0400 Waiman Long <longman at redhat.com> wrote: > 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

On Mon, Apr 13, 2020 at 05:15:49PM -0400, Waiman Long wrote: > diff --git a/mm/slab_common.c b/mm/slab_common.c > index 23c7500eea7d..c08bc7eb20bd 100644 > --- a/mm/slab_common.c > +++ b/mm/slab_common.c > @@ -1707,17 +1707,17 @@ void *krealloc(const void *p, size_t new_size, gfp_t flags) > EXPORT_SYMBOL(krealloc); > > /** > - * kzfree - like kfree but zero memory

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

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

On Mon, Jun 15, 2020 at 09:57:18PM -0400, Waiman Long wrote: > In btrfs_ioctl_get_subvol_info(), there is a classic case where kzalloc() > was incorrectly paired with kzfree(). According to David Sterba, there > isn't any sensitive information in the subvol_info that needs to be > cleared before freeing. So kfree_sensitive() isn't really needed, > use kfree() instead. >

On Mon 15-06-20 21:57:16, Waiman Long wrote: > The kzfree() function is normally used to clear some sensitive > information, like encryption keys, in the buffer before freeing it back > to the pool. Memset() is currently used for the buffer clearing. However, > it is entirely possible that the compiler may choose to optimize away the > memory clearing especially if LTO is being

Last time you sent this we couldn't decide which tree it should go through. Either the crypto tree or through Andrew seems like the right thing to me. Also the other issue is that it risks breaking things if people add new kzfree() instances while we are doing the transition. Could you just add a "#define kzfree kfree_sensitive" so that things continue to compile and we can remove

On Mon, Apr 13, 2020 at 05:15:49PM -0400, Waiman Long wrote: > fs/btrfs/ioctl.c | 2 +- > diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c > index 40b729dce91c..eab3f8510426 100644 > --- a/fs/btrfs/ioctl.c > +++ b/fs/btrfs/ioctl.c > @@ -2691,7 +2691,7 @@ static int btrfs_ioctl_get_subvol_info(struct file *file, void __user *argp) >

Since kfree_sensitive() will do an implicit memzero_explicit(), there is no need to call memzero_explicit() before it. Eliminate those memzero_explicit() and simplify the call sites. For better correctness, the setting of keylen is also moved down after the key pointer check. Signed-off-by: Waiman Long <longman at redhat.com> --- .../allwinner/sun8i-ce/sun8i-ce-cipher.c | 19

Since kfree_sensitive() will do an implicit memzero_explicit(), there is no need to call memzero_explicit() before it. Eliminate those memzero_explicit() and simplify the call sites. Signed-off-by: Waiman Long <longman at redhat.com> --- .../crypto/allwinner/sun8i-ce/sun8i-ce-cipher.c | 15 +++------------ .../crypto/allwinner/sun8i-ss/sun8i-ss-cipher.c | 16 +++-------------

v6: - add patch 1/2 to make sparc architecture happy. [Sam] - close created sessions previousely when rekeying. - convert the priority of virtio crypto algs from 4001 to 501 which is enough. v5: - add comments for algs_lock and table_lock. [Stefan] - use kzfree instead of kfree for key material security. [Stefan] - drop unnecessary spin_lock for struct virtio_crypto_ablkcipher_ctx. -