Re: [PATCH] Staging: rtl8712: Addressed checkpatch.pl issues related to macro parameter wrapping in parentheses.
On Sun, 14 Jun 2020 at 15:05, Greg Kroah-Hartman
wrote:
>
> On Sun, Jun 14, 2020 at 02:51:25PM +0100, Ricardo Ferreira wrote:
> > #define init_h2fwcmd_w_parm_no_rsp(pcmd, pparm, code) \
> > do {\
> > - INIT_LIST_HEAD(&pcmd->list);\
> > - pcmd->cmdcode = code;\
> > - pcmd->parmbuf = (u8 *)(pparm);\
> > - pcmd->cmdsz = sizeof(*pparm);\
> > - pcmd->rsp = NULL;\
> > - pcmd->rspsz = 0;\
> > + INIT_LIST_HEAD(&(pcmd)->list);\
> > + (pcmd)->cmdcode = code;\
> > + (pcmd)->parmbuf = (u8 *)((pparm));\
> > + (pcmd)->cmdsz = sizeof(*(pparm));\
> > + (pcmd)->rsp = NULL;\
> > + (pcmd)->rspsz = 0;\
> > } while (0)
>
> Does that change really make any sense? checkpatch is a nice hint,
> sometimes it is not correct...
(Replying again since I mistakenly sent my comments only to Greg...)
Yeah I was over-eager and applied some of checkpatche's patches
without thinking twice... I guess the parenthesis wrapping only makes
sense when you have an operator (either binary or unary). I've
rechecked each macro identified by checkpatch to see if there is a
need for parenthesis wrapping in their current usage.
Regards,
Ricardo Ferreira.
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] Staging: rtl8712: Addressed checkpatch.pl issues related to macro parameter wrapping in parentheses.
On Mon, Jun 15, 2020 at 10:28:51AM +0100, Ricardo Ferreira wrote:
> On Sun, 14 Jun 2020 at 15:05, Greg Kroah-Hartman
> wrote:
> >
> > On Sun, Jun 14, 2020 at 02:51:25PM +0100, Ricardo Ferreira wrote:
> > > #define init_h2fwcmd_w_parm_no_rsp(pcmd, pparm, code) \
> > > do {\
> > > - INIT_LIST_HEAD(&pcmd->list);\
> > > - pcmd->cmdcode = code;\
> > > - pcmd->parmbuf = (u8 *)(pparm);\
> > > - pcmd->cmdsz = sizeof(*pparm);\
> > > - pcmd->rsp = NULL;\
> > > - pcmd->rspsz = 0;\
> > > + INIT_LIST_HEAD(&(pcmd)->list);\
> > > + (pcmd)->cmdcode = code;\
> > > + (pcmd)->parmbuf = (u8 *)((pparm));\
> > > + (pcmd)->cmdsz = sizeof(*(pparm));\
> > > + (pcmd)->rsp = NULL;\
> > > + (pcmd)->rspsz = 0;\
> > > } while (0)
> >
> > Does that change really make any sense? checkpatch is a nice hint,
> > sometimes it is not correct...
>
> (Replying again since I mistakenly sent my comments only to Greg...)
>
> Yeah I was over-eager and applied some of checkpatche's patches
> without thinking twice... I guess the parenthesis wrapping only makes
> sense when you have an operator (either binary or unary). I've
> rechecked each macro identified by checkpatch to see if there is a
> need for parenthesis wrapping in their current usage.
Yes, please do that, and also test-build your patches. Sending patches
that break the build are a sure way to make maintainers grumpy :)
thanks,
greg k-h
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v2] Staging: rtl8712: Addressed checkpatch.pl issues related to macro parameter wrapping in parentheses.
On Sun, Jun 14, 2020 at 06:31:46PM +0100, Ricardo Ferreira wrote: > Attempting to wet my feet in kernel patch submission by submitting a > checkstyle > fix for the rtl8712 driver. > > Signed-off-by: Ricardo Ferreira > --- What changed from v1? Always put that below the --- line, so please fix this up and do a v3. Also, your changelog text is very odd, do you see other commits in the kernel for these files that say things that way? Please try to follow how others do things. Also read the documentation for how to make a good changelog text, that will help out a lot. thanks, greg k-h ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH] staging: comedi: fix up missing whitespace in comment
From: Garrit Franke I noticed this missing whitespace in a comment inside ni_mio_common.c Signed-off-by: Garrit Franke --- drivers/staging/comedi/drivers/ni_mio_common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/comedi/drivers/ni_mio_common.c b/drivers/staging/comedi/drivers/ni_mio_common.c index d99f4065b96d..9266e13f6271 100644 --- a/drivers/staging/comedi/drivers/ni_mio_common.c +++ b/drivers/staging/comedi/drivers/ni_mio_common.c @@ -2390,7 +2390,7 @@ static int ni_ai_cmd(struct comedi_device *dev, struct comedi_subdevice *s) #endif break; case AIMODE_SAMPLE: - /*generate FIFO interrupts on non-empty */ + /* generate FIFO interrupts on non-empty */ ni_stc_writew(dev, NISTC_AI_MODE3_FIFO_MODE_NE, NISTC_AI_MODE3_REG); break; -- 2.25.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] mm, treewide: Rename kzfree() to kfree_sensitive()
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
> + * kfree_sensitive - Clear sensitive information in memory before freeing
> * @p: object to free memory of
> *
> * The memory of the object @p points to is zeroed before freed.
> - * If @p is %NULL, kzfree() does nothing.
> + * If @p is %NULL, kfree_sensitive() does nothing.
> *
> * Note: this function zeroes the whole allocated buffer which can be a good
> * deal bigger than the requested buffer size passed to kmalloc(). So be
> * careful when using this function in performance sensitive code.
> */
> -void kzfree(const void *p)
> +void kfree_sensitive(const void *p)
> {
> size_t ks;
> void *mem = (void *)p;
> @@ -1725,10 +1725,10 @@ void kzfree(const void *p)
> if (unlikely(ZERO_OR_NULL_PTR(mem)))
> return;
> ks = ksize(mem);
> - memset(mem, 0, ks);
> + memzero_explicit(mem, ks);
^
This is an unrelated bug fix. It really needs to be pulled into a
separate patch by itself and back ported to stable kernels.
> kfree(mem);
> }
> -EXPORT_SYMBOL(kzfree);
> +EXPORT_SYMBOL(kfree_sensitive);
>
> /**
> * ksize - get the actual amount of memory allocated for a given object
regards,
dan carpenter
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH 1/2] mm, treewide: Rename kzfree() to kfree_sensitive()
On 6/15/20 2:07 PM, Dan Carpenter wrote:
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
+ * kfree_sensitive - Clear sensitive information in memory before freeing
* @p: object to free memory of
*
* The memory of the object @p points to is zeroed before freed.
- * If @p is %NULL, kzfree() does nothing.
+ * If @p is %NULL, kfree_sensitive() does nothing.
*
* Note: this function zeroes the whole allocated buffer which can be a good
* deal bigger than the requested buffer size passed to kmalloc(). So be
* careful when using this function in performance sensitive code.
*/
-void kzfree(const void *p)
+void kfree_sensitive(const void *p)
{
size_t ks;
void *mem = (void *)p;
@@ -1725,10 +1725,10 @@ void kzfree(const void *p)
if (unlikely(ZERO_OR_NULL_PTR(mem)))
return;
ks = ksize(mem);
- memset(mem, 0, ks);
+ memzero_explicit(mem, ks);
^
This is an unrelated bug fix. It really needs to be pulled into a
separate patch by itself and back ported to stable kernels.
kfree(mem);
}
-EXPORT_SYMBOL(kzfree);
+EXPORT_SYMBOL(kfree_sensitive);
/**
* ksize - get the actual amount of memory allocated for a given object
regards,
dan carpenter
Thanks for the suggestion. I will break it out and post a version soon.
Cheers,
Longman
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[driver-core:readfile] BUILD SUCCESS fd5cf2d6f921ca0f8f777d8833a0a3e5100bc515
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git readfile branch HEAD: fd5cf2d6f921ca0f8f777d8833a0a3e5100bc515 selftests: add readfile(2) selftests elapsed time: 483m configs tested: 98 configs skipped: 1 The following configs have been built successfully. More configs may be tested in the coming days. arm defconfig arm allyesconfig arm allmodconfig arm allnoconfig arm64allyesconfig arm64 defconfig arm64allmodconfig arm64 allnoconfig i386 allnoconfig i386 allyesconfig i386defconfig i386 debian-10.3 ia64 allmodconfig ia64defconfig ia64 allnoconfig ia64 allyesconfig m68k allmodconfig m68k allnoconfig m68k sun3_defconfig m68kdefconfig m68k allyesconfig nios2 defconfig nios2allyesconfig openriscdefconfig c6x allyesconfig c6x allnoconfig openrisc allyesconfig nds32 defconfig nds32 allnoconfig csky allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig h8300allmodconfig xtensa defconfig arc defconfig arc allyesconfig sh allmodconfig shallnoconfig microblazeallnoconfig mips allyesconfig mips allnoconfig mips allmodconfig pariscallnoconfig parisc defconfig parisc allyesconfig parisc allmodconfig powerpc defconfig powerpc allyesconfig powerpc rhel-kconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a006-20200615 i386 randconfig-a002-20200615 i386 randconfig-a001-20200615 i386 randconfig-a004-20200615 i386 randconfig-a005-20200615 i386 randconfig-a003-20200615 x86_64 randconfig-a015-20200615 x86_64 randconfig-a011-20200615 x86_64 randconfig-a016-20200615 x86_64 randconfig-a012-20200615 x86_64 randconfig-a014-20200615 x86_64 randconfig-a013-20200615 i386 randconfig-a015-20200615 i386 randconfig-a011-20200615 i386 randconfig-a014-20200615 i386 randconfig-a013-20200615 i386 randconfig-a016-20200615 i386 randconfig-a012-20200615 riscvallyesconfig riscv allnoconfig riscv defconfig riscvallmodconfig s390 allyesconfig s390 allnoconfig s390 allmodconfig s390defconfig sparcallyesconfig sparc defconfig sparc64 defconfig sparc64 allnoconfig sparc64 allyesconfig sparc64 allmodconfig um allmodconfig umallnoconfig um defconfig um allyesconfig x86_64 rhel-7.6 x86_64rhel-7.6-kselftests x86_64 rhel-8.3 x86_64 kexec x86_64 rhel x86_64 rhel-7.2-clear x86_64lkp x86_64 fedora-25 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
[driver-core:debugfs_cleanup] BUILD SUCCESS 8b905726a5dd443ac67f92fe9d4768335f19332d
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/driver-core.git debugfs_cleanup branch HEAD: 8b905726a5dd443ac67f92fe9d4768335f19332d debugfs: remove return value of debugfs_create_devm_seqfile() elapsed time: 487m configs tested: 103 configs skipped: 1 The following configs have been built successfully. More configs may be tested in the coming days. arm64allyesconfig arm64 defconfig arm64allmodconfig arm64 allnoconfig arm defconfig arm allyesconfig arm allmodconfig arm allnoconfig arm pxa_defconfig um x86_64_defconfig mips pic32mzda_defconfig arm versatile_defconfig ia64 gensparse_defconfig i386 allnoconfig i386 allyesconfig i386defconfig i386 debian-10.3 ia64 allmodconfig ia64defconfig ia64 allnoconfig ia64 allyesconfig m68k allmodconfig m68k allnoconfig m68k sun3_defconfig m68kdefconfig m68k allyesconfig nios2 defconfig nios2allyesconfig openriscdefconfig c6x allyesconfig c6x allnoconfig openrisc allyesconfig nds32 defconfig nds32 allnoconfig csky allyesconfig cskydefconfig alpha defconfig alphaallyesconfig xtensa allyesconfig h8300allyesconfig h8300allmodconfig xtensa defconfig arc defconfig arc allyesconfig sh allmodconfig shallnoconfig microblazeallnoconfig mips allyesconfig mips allnoconfig mips allmodconfig pariscallnoconfig parisc defconfig parisc allyesconfig parisc allmodconfig powerpc defconfig powerpc allyesconfig powerpc rhel-kconfig powerpc allmodconfig powerpc allnoconfig i386 randconfig-a006-20200615 i386 randconfig-a002-20200615 i386 randconfig-a001-20200615 i386 randconfig-a004-20200615 i386 randconfig-a005-20200615 i386 randconfig-a003-20200615 x86_64 randconfig-a015-20200615 x86_64 randconfig-a011-20200615 x86_64 randconfig-a016-20200615 x86_64 randconfig-a012-20200615 x86_64 randconfig-a014-20200615 x86_64 randconfig-a013-20200615 i386 randconfig-a015-20200615 i386 randconfig-a011-20200615 i386 randconfig-a014-20200615 i386 randconfig-a013-20200615 i386 randconfig-a016-20200615 i386 randconfig-a012-20200615 riscvallyesconfig riscv allnoconfig riscv defconfig riscvallmodconfig s390 allyesconfig s390 allnoconfig s390 allmodconfig s390defconfig sparcallyesconfig sparc defconfig sparc64 defconfig sparc64 allnoconfig sparc64 allyesconfig sparc64 allmodconfig um allmodconfig umallnoconfig um defconfig um allyesconfig x86_64 rhel-7.6 x86_64rhel-7.6-kselftests x86_64 rhel-8.3 x86_64 kexec x86_64 rhel x86_64
[PATCH][next] staging: rts5208: Use array_size() helper in vmalloc()
and memset()
The vmalloc() function has no 2-factor argument form, so multiplication
factors need to be wrapped in array_size(). Also, while there, use
array_size() in memset().
This issue was found with the help of Coccinelle and, audited and fixed
manually.
Addresses-KSPP-ID: https://github.com/KSPP/linux/issues/83
Signed-off-by: Gustavo A. R. Silva
---
drivers/staging/rts5208/ms.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/staging/rts5208/ms.c b/drivers/staging/rts5208/ms.c
index d53dd138a356..9001570a8c94 100644
--- a/drivers/staging/rts5208/ms.c
+++ b/drivers/staging/rts5208/ms.c
@@ -2306,14 +2306,14 @@ static int ms_build_l2p_tbl(struct rtsx_chip *chip, int
seg_no)
if (!segment->l2p_table)
goto BUILD_FAIL;
}
- memset((u8 *)(segment->l2p_table), 0xff, table_size * 2);
+ memset((u8 *)(segment->l2p_table), 0xff, array_size(table_size, 2));
if (!segment->free_table) {
- segment->free_table = vmalloc(MS_FREE_TABLE_CNT * 2);
+ segment->free_table = vmalloc(array_size(MS_FREE_TABLE_CNT, 2));
if (!segment->free_table)
goto BUILD_FAIL;
}
- memset((u8 *)(segment->free_table), 0xff, MS_FREE_TABLE_CNT * 2);
+ memset((u8 *)(segment->free_table), 0xff, array_size(MS_FREE_TABLE_CNT,
2));
start = (u16)seg_no << 9;
end = (u16)(seg_no + 1) << 9;
--
2.27.0
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 0/3] mm, treewide: Rename kzfree() to kfree_sensitive()
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. 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 especially if LTO is used. Instead, the new kfree_sensitive() uses memzero_explicit() which won't get compiled out. Waiman Long (3): mm/slab: Use memzero_explicit() in kzfree() mm, treewide: Rename kzfree() to kfree_sensitive() btrfs: Use kfree() in btrfs_ioctl_get_subvol_info() arch/s390/crypto/prng.c | 4 +-- arch/x86/power/hibernate.c| 2 +- crypto/adiantum.c | 2 +- crypto/ahash.c| 4 +-- crypto/api.c | 2 +- crypto/asymmetric_keys/verify_pefile.c| 4 +-- crypto/deflate.c | 2 +- crypto/drbg.c | 10 +++--- crypto/ecc.c | 8 ++--- crypto/ecdh.c | 2 +- crypto/gcm.c | 2 +- crypto/gf128mul.c | 4 +-- crypto/jitterentropy-kcapi.c | 2 +- crypto/rng.c | 2 +- crypto/rsa-pkcs1pad.c | 6 ++-- crypto/seqiv.c| 2 +- crypto/shash.c| 2 +- crypto/skcipher.c | 2 +- crypto/testmgr.c | 6 ++-- crypto/zstd.c | 2 +- .../allwinner/sun8i-ce/sun8i-ce-cipher.c | 2 +- .../allwinner/sun8i-ss/sun8i-ss-cipher.c | 2 +- drivers/crypto/amlogic/amlogic-gxl-cipher.c | 4 +-- drivers/crypto/atmel-ecc.c| 2 +- drivers/crypto/caam/caampkc.c | 28 +++ drivers/crypto/cavium/cpt/cptvf_main.c| 6 ++-- drivers/crypto/cavium/cpt/cptvf_reqmanager.c | 12 +++ drivers/crypto/cavium/nitrox/nitrox_lib.c | 4 +-- drivers/crypto/cavium/zip/zip_crypto.c| 6 ++-- drivers/crypto/ccp/ccp-crypto-rsa.c | 6 ++-- drivers/crypto/ccree/cc_aead.c| 4 +-- drivers/crypto/ccree/cc_buffer_mgr.c | 4 +-- drivers/crypto/ccree/cc_cipher.c | 6 ++-- drivers/crypto/ccree/cc_hash.c| 8 ++--- drivers/crypto/ccree/cc_request_mgr.c | 2 +- drivers/crypto/marvell/cesa/hash.c| 2 +- .../crypto/marvell/octeontx/otx_cptvf_main.c | 6 ++-- .../marvell/octeontx/otx_cptvf_reqmgr.h | 2 +- drivers/crypto/mediatek/mtk-aes.c | 2 +- drivers/crypto/nx/nx.c| 4 +-- drivers/crypto/virtio/virtio_crypto_algs.c| 12 +++ drivers/crypto/virtio/virtio_crypto_core.c| 2 +- drivers/md/dm-crypt.c | 32 - drivers/md/dm-integrity.c | 6 ++-- drivers/misc/ibmvmc.c | 6 ++-- .../hisilicon/hns3/hns3pf/hclge_mbx.c | 2 +- .../net/ethernet/intel/ixgbe/ixgbe_ipsec.c| 6 ++-- drivers/net/ppp/ppp_mppe.c| 6 ++-- drivers/net/wireguard/noise.c | 4 +-- drivers/net/wireguard/peer.c | 2 +- drivers/net/wireless/intel/iwlwifi/pcie/rx.c | 2 +- .../net/wireless/intel/iwlwifi/pcie/tx-gen2.c | 6 ++-- drivers/net/wireless/intel/iwlwifi/pcie/tx.c | 6 ++-- drivers/net/wireless/intersil/orinoco/wext.c | 4 +-- drivers/s390/crypto/ap_bus.h | 4 +-- drivers/staging/ks7010/ks_hostif.c| 2 +- drivers/staging/rtl8723bs/core/rtw_security.c | 2 +- drivers/staging/wlan-ng/p80211netdev.c| 2 +- drivers/target/iscsi/iscsi_target_auth.c | 2 +- fs/btrfs/ioctl.c | 2 +- fs/cifs/cifsencrypt.c | 2 +- fs/cifs/connect.c | 10 +++--- fs/cifs/dfs_cache.c | 2 +- fs/cifs/misc.c| 8 ++--- fs/crypto/keyring.c | 6 ++-- fs/crypto/keysetup_v1.c | 4 +-- fs/ecryptfs/keystore.c| 4 +-- fs/ecryptfs/messaging.c | 2 +- include/crypto/aead.h | 2 +- include/crypto/akcipher.h | 2 +- include/crypto/gf128mul.h | 2 +- include/crypto/hash.h
[PATCH v4 1/3] mm/slab: Use memzero_explicit() in kzfree()
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 used. To make sure that this
optimization will not happen, memzero_explicit(), which is introduced
in v3.18, is now used in kzfree() to do the clearing.
Fixes: 3ef0e5ba4673 ("slab: introduce kzfree()")
Cc: sta...@vger.kernel.org
Signed-off-by: Waiman Long
---
mm/slab_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 9e72ba224175..37d48a56431d 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1726,7 +1726,7 @@ void kzfree(const void *p)
if (unlikely(ZERO_OR_NULL_PTR(mem)))
return;
ks = ksize(mem);
- memset(mem, 0, ks);
+ memzero_explicit(mem, ks);
kfree(mem);
}
EXPORT_SYMBOL(kzfree);
--
2.18.1
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
[PATCH v4 2/3] mm, treewide: Rename kzfree() to kfree_sensitive()
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 the use of memzero_explicit() instead of memset(). Suggested-by: Joe Perches Acked-by: David Howells Acked-by: Michal Hocko Acked-by: Johannes Weiner Signed-off-by: Waiman Long --- arch/s390/crypto/prng.c | 4 +-- arch/x86/power/hibernate.c| 2 +- crypto/adiantum.c | 2 +- crypto/ahash.c| 4 +-- crypto/api.c | 2 +- crypto/asymmetric_keys/verify_pefile.c| 4 +-- crypto/deflate.c | 2 +- crypto/drbg.c | 10 +++--- crypto/ecc.c | 8 ++--- crypto/ecdh.c | 2 +- crypto/gcm.c | 2 +- crypto/gf128mul.c | 4 +-- crypto/jitterentropy-kcapi.c | 2 +- crypto/rng.c | 2 +- crypto/rsa-pkcs1pad.c | 6 ++-- crypto/seqiv.c| 2 +- crypto/shash.c| 2 +- crypto/skcipher.c | 2 +- crypto/testmgr.c | 6 ++-- crypto/zstd.c | 2 +- .../allwinner/sun8i-ce/sun8i-ce-cipher.c | 2 +- .../allwinner/sun8i-ss/sun8i-ss-cipher.c | 2 +- drivers/crypto/amlogic/amlogic-gxl-cipher.c | 4 +-- drivers/crypto/atmel-ecc.c| 2 +- drivers/crypto/caam/caampkc.c | 28 +++ drivers/crypto/cavium/cpt/cptvf_main.c| 6 ++-- drivers/crypto/cavium/cpt/cptvf_reqmanager.c | 12 +++ drivers/crypto/cavium/nitrox/nitrox_lib.c | 4 +-- drivers/crypto/cavium/zip/zip_crypto.c| 6 ++-- drivers/crypto/ccp/ccp-crypto-rsa.c | 6 ++-- drivers/crypto/ccree/cc_aead.c| 4 +-- drivers/crypto/ccree/cc_buffer_mgr.c | 4 +-- drivers/crypto/ccree/cc_cipher.c | 6 ++-- drivers/crypto/ccree/cc_hash.c| 8 ++--- drivers/crypto/ccree/cc_request_mgr.c | 2 +- drivers/crypto/marvell/cesa/hash.c| 2 +- .../crypto/marvell/octeontx/otx_cptvf_main.c | 6 ++-- .../marvell/octeontx/otx_cptvf_reqmgr.h | 2 +- drivers/crypto/mediatek/mtk-aes.c | 2 +- drivers/crypto/nx/nx.c| 4 +-- drivers/crypto/virtio/virtio_crypto_algs.c| 12 +++ drivers/crypto/virtio/virtio_crypto_core.c| 2 +- drivers/md/dm-crypt.c | 32 - drivers/md/dm-integrity.c | 6 ++-- drivers/misc/ibmvmc.c | 6 ++-- .../hisilicon/hns3/hns3pf/hclge_mbx.c | 2 +- .../net/ethernet/intel/ixgbe/ixgbe_ipsec.c| 6 ++-- drivers/net/ppp/ppp_mppe.c| 6 ++-- drivers/net/wireguard/noise.c | 4 +-- drivers/net/wireguard/peer.c | 2 +- drivers/net/wireless/intel/iwlwifi/pcie/rx.c | 2 +- .../net/wireless/intel/iwlwifi/pcie/tx-gen2.c | 6 ++-- drivers/net/wireless/intel/iwlwifi/pcie/tx.c | 6 ++-- drivers/net/wireless/intersil/orinoco/wext.c | 4 +-- drivers/s390/crypto/ap_bus.h | 4 +-- drivers/staging/ks7010/ks_hostif.c| 2 +- drivers/staging/rtl8723bs/core/rtw_security.c | 2 +- drivers/staging/wlan-ng/p80211netdev.c| 2 +- drivers/target/iscsi/iscsi_target_auth.c | 2 +- fs/btrfs/ioctl.c | 2 +- fs/cifs/cifsencrypt.c | 2 +- fs/cifs/connect.c | 10 +++--- fs/cifs/dfs_cache.c | 2 +- fs/cifs/misc.c| 8 ++--- fs/crypto/keyring.c | 6 ++-- fs/crypto/keysetup_v1.c | 4 +-- fs/ecryptfs/keystore.c
[PATCH v4 3/3] btrfs: Use kfree() in btrfs_ioctl_get_subvol_info()
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. Reported-by: David Sterba Signed-off-by: Waiman Long --- fs/btrfs/ioctl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index f1dd9e4271e9..e8f7c5f00894 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -2692,7 +2692,7 @@ static int btrfs_ioctl_get_subvol_info(struct file *file, void __user *argp) btrfs_put_root(root); out_free: btrfs_free_path(path); - kfree_sensitive(subvol_info); + kfree(subvol_info); return ret; } -- 2.18.1 ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH v4 1/3] mm/slab: Use memzero_explicit() in kzfree()
On Mon, Jun 15, 2020 at 09:57:16PM -0400, 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 used. To make sure that this
> optimization will not happen, memzero_explicit(), which is introduced
> in v3.18, is now used in kzfree() to do the clearing.
>
> Fixes: 3ef0e5ba4673 ("slab: introduce kzfree()")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Waiman Long
> ---
> mm/slab_common.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 9e72ba224175..37d48a56431d 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1726,7 +1726,7 @@ void kzfree(const void *p)
> if (unlikely(ZERO_OR_NULL_PTR(mem)))
> return;
> ks = ksize(mem);
> - memset(mem, 0, ks);
> + memzero_explicit(mem, ks);
> kfree(mem);
> }
> EXPORT_SYMBOL(kzfree);
This is a good change, but the commit message isn't really accurate. AFAIK, no
one has found any case where this memset() gets optimized out. And even with
LTO, it would be virtually impossible due to all the synchronization and global
data structures that kfree() uses. (Remember that this isn't the C standard
function "free()", so the compiler can't assign it any special meaning.)
Not to mention that LTO support isn't actually upstream yet.
I still agree with the change, but it might be helpful if the commit message
were honest that this is really a hardening measure and about properly conveying
the intent. As-is this sounds like a critical fix, which might confuse people.
- Eric
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCH] staging: android: ashmem.c: Cleanup
Hi Dio,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on staging/staging-testing]
[cannot apply to aa5af974127d317071d6225a0f3678c5f520e7ce]
url:
https://github.com/0day-ci/linux/commits/Dio-Putra/staging-android-ashmem-c-Cleanup/20200614-013821
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git
af7b4801030c07637840191c69eb666917e4135d
:: branch date: 3 hours ago
:: commit date: 3 hours ago
config: i386-randconfig-s002-20200614 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-13) 9.3.0
reproduce:
# apt-get install sparse
# sparse version: v0.6.1-250-g42323db3-dirty
# save the attached .config to linux build tree
make W=1 C=1 ARCH=i386 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
sparse warnings: (new ones prefixed by >>)
>> drivers/staging/android/ashmem.c:418:25: sparse: sparse: assignment to const
>> expression
drivers/staging/android/ashmem.c:419:36: sparse: sparse: assignment to const
expression
drivers/staging/android/ashmem.c:420:36: sparse: sparse: assignment to const
expression
#
https://github.com/0day-ci/linux/commit/7fe32ab69ec805d9e6b68fc7871f9de384429a48
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout 7fe32ab69ec805d9e6b68fc7871f9de384429a48
vim +418 drivers/staging/android/ashmem.c
6d67b0290b4b84 Suren Baghdasaryan 2020-01-27 367
11980c2ac4ccfa Robert Love2011-12-20 368 static int
ashmem_mmap(struct file *file, struct vm_area_struct *vma)
11980c2ac4ccfa Robert Love2011-12-20 369 {
7fe32ab69ec805 Dio Putra 2020-06-14 370 static const struct
file_operations vmfile_fops;
11980c2ac4ccfa Robert Love2011-12-20 371 struct ashmem_area
*asma = file->private_data;
11980c2ac4ccfa Robert Love2011-12-20 372 int ret = 0;
11980c2ac4ccfa Robert Love2011-12-20 373
11980c2ac4ccfa Robert Love2011-12-20 374
mutex_lock(&ashmem_mutex);
11980c2ac4ccfa Robert Love2011-12-20 375
11980c2ac4ccfa Robert Love2011-12-20 376 /* user needs to
SET_SIZE before mapping */
59848d6aded59a Alistair Strachan 2018-06-19 377 if (!asma->size) {
11980c2ac4ccfa Robert Love2011-12-20 378 ret = -EINVAL;
11980c2ac4ccfa Robert Love2011-12-20 379 goto out;
11980c2ac4ccfa Robert Love2011-12-20 380 }
11980c2ac4ccfa Robert Love2011-12-20 381
8632c614565d0c Alistair Strachan 2018-06-19 382 /* requested mapping
size larger than object size */
8632c614565d0c Alistair Strachan 2018-06-19 383 if (vma->vm_end -
vma->vm_start > PAGE_ALIGN(asma->size)) {
11980c2ac4ccfa Robert Love2011-12-20 384 ret = -EINVAL;
11980c2ac4ccfa Robert Love2011-12-20 385 goto out;
11980c2ac4ccfa Robert Love2011-12-20 386 }
11980c2ac4ccfa Robert Love2011-12-20 387
11980c2ac4ccfa Robert Love2011-12-20 388 /* requested protection
bits must match our allowed protection mask */
59848d6aded59a Alistair Strachan 2018-06-19 389 if ((vma->vm_flags &
~calc_vm_prot_bits(asma->prot_mask, 0)) &
59848d6aded59a Alistair Strachan 2018-06-19 390
calc_vm_prot_bits(PROT_MASK, 0)) {
11980c2ac4ccfa Robert Love2011-12-20 391 ret = -EPERM;
11980c2ac4ccfa Robert Love2011-12-20 392 goto out;
11980c2ac4ccfa Robert Love2011-12-20 393 }
56f76fc68492af Arve Hjønnevåg 2011-12-20 394 vma->vm_flags &=
~calc_vm_may_flags(~asma->prot_mask);
11980c2ac4ccfa Robert Love2011-12-20 395
11980c2ac4ccfa Robert Love2011-12-20 396 if (!asma->file) {
11980c2ac4ccfa Robert Love2011-12-20 397 char *name =
ASHMEM_NAME_DEF;
11980c2ac4ccfa Robert Love2011-12-20 398 struct file
*vmfile;
11980c2ac4ccfa Robert Love2011-12-20 399
11980c2ac4ccfa Robert Love2011-12-20 400 if
(asma->name[ASHMEM_NAME_PREFIX_LEN] != '\0')
11980c2ac4ccfa Robert Love2011-12-20 401 name =
asma->name;
11980c2ac4ccfa Robert Love2011-12-20 402
11980c2ac4ccfa Robert Love2011-12-20 403 /* ... and
allocate the backing shmem file */
11980c2ac4ccfa Robert Love2011-12-20 404 vmfile =
shmem_file_setup(name, asma->size, vma->vm_flags);
7f44cb0ba88b40 Viresh Kumar 2015-07-31 405 if
(IS_ERR(vmfile)) {
11980c2ac4ccfa Robert Love2011-12-20 406 ret =
PTR_ERR(vmfile);
11980c2ac4ccfa Robert Love2011-12-20 407 goto
out;
11980c2ac4ccfa Robert Love2011-12-20 408 }
97fbfef6bd5978 Shuxiao Zhang 2
Re: [PATCH v4 1/3] mm/slab: Use memzero_explicit() in kzfree()
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 used. To make sure that this
> optimization will not happen, memzero_explicit(), which is introduced
> in v3.18, is now used in kzfree() to do the clearing.
>
> Fixes: 3ef0e5ba4673 ("slab: introduce kzfree()")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Waiman Long
Acked-by: Michal Hocko
Although I am not really sure this is a stable material. Is there any
known instance where the memset was optimized out from kzfree?
> ---
> mm/slab_common.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 9e72ba224175..37d48a56431d 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1726,7 +1726,7 @@ void kzfree(const void *p)
> if (unlikely(ZERO_OR_NULL_PTR(mem)))
> return;
> ks = ksize(mem);
> - memset(mem, 0, ks);
> + memzero_explicit(mem, ks);
> kfree(mem);
> }
> EXPORT_SYMBOL(kzfree);
> --
> 2.18.1
>
--
Michal Hocko
SUSE Labs
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
