* [PATCH 0/3] introduce rte_memset_sensative
@ 2024-11-14 1:10 Stephen Hemminger
2024-11-14 1:10 ` [PATCH 1/3] eal: " Stephen Hemminger
` (5 more replies)
0 siblings, 6 replies; 42+ messages in thread
From: Stephen Hemminger @ 2024-11-14 1:10 UTC (permalink / raw)
To: dev; +Cc: Stephen Hemminger
The PVS bug list pointed out a number of places where code is
trying to do memset() on sensative data which can be optimized
away. Introduce som helpers and example usage.
Stephen Hemminger (3):
eal: introduce rte_memset_sensative
crypto/qat: use rte_memset_sensative
eal: add rte_free_sensative
drivers/crypto/qat/qat_sym_session.c | 17 ++++++++--------
lib/eal/common/rte_malloc.c | 30 ++++++++++++++++++++++------
lib/eal/include/rte_malloc.h | 18 +++++++++++++++++
lib/eal/include/rte_string_fns.h | 27 +++++++++++++++++++++++++
lib/eal/version.map | 3 +++
5 files changed, 81 insertions(+), 14 deletions(-)
--
2.45.2
^ permalink raw reply [flat|nested] 42+ messages in thread* [PATCH 1/3] eal: introduce rte_memset_sensative 2024-11-14 1:10 [PATCH 0/3] introduce rte_memset_sensative Stephen Hemminger @ 2024-11-14 1:10 ` Stephen Hemminger 2024-11-14 1:10 ` [PATCH 2/3] crypto/qat: use rte_memset_sensative Stephen Hemminger ` (4 subsequent siblings) 5 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 1:10 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Tyler Retzlaff When memset() is used before a release function such as free, the compiler if allowed to optimize the memset away under the as-if rules. This is normally ok, but in certain cases such as passwords or security keys it is problematic. Introduce a DPDK wrapper which is equivalent to the C++ memset_s function. Naming chosen to be similar to kernel. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- lib/eal/include/rte_string_fns.h | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/lib/eal/include/rte_string_fns.h b/lib/eal/include/rte_string_fns.h index 702bd81251..6f7dd85cbf 100644 --- a/lib/eal/include/rte_string_fns.h +++ b/lib/eal/include/rte_string_fns.h @@ -15,6 +15,7 @@ #include <stdio.h> #include <string.h> +#include <rte_atomic.h> #include <rte_common.h> #include <rte_compat.h> @@ -149,6 +150,32 @@ rte_str_skip_leading_spaces(const char *src) return p; } +/** + * @warning + * @b EXPERIMENTAL: this API may change without prior notice. + * + * Fill memory with constant byte but can not be optimized away. + * Use as a replacement for memset() for sensitive information. + * + * @param dst + * target buffer + * @param ch + * byte to fill + * @param + * number of bytes to fill + * + * @return + * like memset() returns a pointer th the memory area dst. + */ +__rte_experimental +static inline void * +rte_memset_sensative(void *dst, int ch, size_t sz) +{ + void *ret = memset(dst, ch, sz); + rte_compiler_barrier(); + return ret; +} + #ifdef __cplusplus } #endif -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 2/3] crypto/qat: use rte_memset_sensative 2024-11-14 1:10 [PATCH 0/3] introduce rte_memset_sensative Stephen Hemminger 2024-11-14 1:10 ` [PATCH 1/3] eal: " Stephen Hemminger @ 2024-11-14 1:10 ` Stephen Hemminger 2024-11-14 1:10 ` [PATCH 3/3] eal: add rte_free_sensative Stephen Hemminger ` (3 subsequent siblings) 5 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 1:10 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Kai Ji Just doing memset() on keys is not enough, compiler can optimize it away. Need something with a barrier. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/crypto/qat/qat_sym_session.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/crypto/qat/qat_sym_session.c b/drivers/crypto/qat/qat_sym_session.c index 50d687fd37..4b4e9ccbab 100644 --- a/drivers/crypto/qat/qat_sym_session.c +++ b/drivers/crypto/qat/qat_sym_session.c @@ -26,6 +26,7 @@ #include <rte_crypto_sym.h> #include <rte_security_driver.h> #include <rte_ether.h> +#include <rte_string_fns.h> #include "qat_logs.h" #include "qat_sym_session.h" @@ -1633,7 +1634,7 @@ static int qat_sym_do_precomputes(enum icp_qat_hw_auth_algo hash_alg, aes_cmac_key_derive(k0, k1); aes_cmac_key_derive(k1, k2); - memset(k0, 0, ICP_QAT_HW_AES_128_KEY_SZ); + rte_memset_sensative(k0, 0, ICP_QAT_HW_AES_128_KEY_SZ); *p_state_len = ICP_QAT_HW_AES_XCBC_MAC_STATE2_SZ; rte_free(in); goto out; @@ -1668,7 +1669,7 @@ static int qat_sym_do_precomputes(enum icp_qat_hw_auth_algo hash_alg, &enc_key) != 0) { rte_free(in - (x * ICP_QAT_HW_AES_XCBC_MAC_KEY_SZ)); - memset(out - + rte_memset_sensative(out - (x * ICP_QAT_HW_AES_XCBC_MAC_KEY_SZ), 0, ICP_QAT_HW_AES_XCBC_MAC_STATE2_SZ); return -EFAULT; @@ -1698,7 +1699,7 @@ static int qat_sym_do_precomputes(enum icp_qat_hw_auth_algo hash_alg, return -ENOMEM; } - memset(in, 0, ICP_QAT_HW_GALOIS_H_SZ); + rte_memset_sensative(in, 0, ICP_QAT_HW_GALOIS_H_SZ); if (AES_set_encrypt_key(auth_key, auth_keylen << 3, &enc_key) != 0) { return -EFAULT; @@ -1757,8 +1758,8 @@ static int qat_sym_do_precomputes(enum icp_qat_hw_auth_algo hash_alg, } /* don't leave data lying around */ - memset(ipad, 0, block_size); - memset(opad, 0, block_size); + rte_memset_sensative(ipad, 0, block_size); + rte_memset_sensative(opad, 0, block_size); out: return 0; } @@ -2006,8 +2007,8 @@ static int qat_sym_do_precomputes_ipsec_mb(enum icp_qat_hw_auth_algo hash_alg, out: /* don't leave data lying around */ - memset(ipad, 0, block_size); - memset(opad, 0, block_size); + rte_memset_sensative(ipad, 0, block_size); + rte_memset_sensative(opad, 0, block_size); free_mb_mgr(m); return ret; } @@ -3232,7 +3233,7 @@ qat_security_session_destroy(void *dev __rte_unused, if (s->mb_mgr) free_mb_mgr(s->mb_mgr); #endif - memset(s, 0, qat_sym_session_get_private_size(dev)); + rte_memset_sensative(s, 0, qat_sym_session_get_private_size(dev)); } return 0; -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 3/3] eal: add rte_free_sensative 2024-11-14 1:10 [PATCH 0/3] introduce rte_memset_sensative Stephen Hemminger 2024-11-14 1:10 ` [PATCH 1/3] eal: " Stephen Hemminger 2024-11-14 1:10 ` [PATCH 2/3] crypto/qat: use rte_memset_sensative Stephen Hemminger @ 2024-11-14 1:10 ` Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 0/8] memset security handling Stephen Hemminger ` (2 subsequent siblings) 5 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 1:10 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Anatoly Burakov, Tyler Retzlaff Although internally rte_free does poison the buffer in most cases, it is useful to have function that explicitly does this to avoid any security issues. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- lib/eal/common/rte_malloc.c | 30 ++++++++++++++++++++++++------ lib/eal/include/rte_malloc.h | 18 ++++++++++++++++++ lib/eal/version.map | 3 +++ 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/lib/eal/common/rte_malloc.c b/lib/eal/common/rte_malloc.c index 3eed4d4be6..bd543b46e8 100644 --- a/lib/eal/common/rte_malloc.c +++ b/lib/eal/common/rte_malloc.c @@ -15,6 +15,7 @@ #include <rte_eal_memconfig.h> #include <rte_common.h> #include <rte_spinlock.h> +#include <rte_string_fns.h> #include <eal_trace_internal.h> @@ -27,27 +28,44 @@ /* Free the memory space back to heap */ -static void -mem_free(void *addr, const bool trace_ena) +static inline void +mem_free(void *addr, const bool trace_ena, bool zero) { + struct malloc_elem *elem; + if (trace_ena) rte_eal_trace_mem_free(addr); - if (addr == NULL) return; - if (malloc_heap_free(malloc_elem_from_data(addr)) < 0) + if (addr == NULL) + return; + + elem = malloc_elem_from_data(addr); + if (zero) { + size_t data_len = elem->size - MALLOC_ELEM_OVERHEAD; + + rte_memset_sensative(addr, 0, data_len); + } + + if (malloc_heap_free(elem) < 0) EAL_LOG(ERR, "Error: Invalid memory"); } void rte_free(void *addr) { - mem_free(addr, true); + mem_free(addr, true, false); +} + +void +rte_free_sensative(void *addr) +{ + mem_free(addr, true, true); } void eal_free_no_trace(void *addr) { - mem_free(addr, false); + mem_free(addr, false, false); } static void * diff --git a/lib/eal/include/rte_malloc.h b/lib/eal/include/rte_malloc.h index c8836de67c..6675c3f405 100644 --- a/lib/eal/include/rte_malloc.h +++ b/lib/eal/include/rte_malloc.h @@ -51,6 +51,24 @@ struct rte_malloc_socket_stats { void rte_free(void *ptr); + +/** + * Frees the memory space pointed to by the provided pointer + * and guarantees it will be zero'd before reuse. + * + * This pointer must have been returned by a previous call to + * rte_malloc(), rte_zmalloc(), rte_calloc() or rte_realloc(). The behaviour of + * rte_free() is undefined if the pointer does not match this requirement. + * + * If the pointer is NULL, the function does nothing. + * + * @param ptr + * The pointer to memory to be freed. + */ +__rte_experimental +void +rte_free_sensative(void *ptr); + /** * This function allocates memory from the huge-page area of memory. The memory * is not cleared. In NUMA systems, the memory allocated resides on the same diff --git a/lib/eal/version.map b/lib/eal/version.map index a20c713eb1..4e0203053a 100644 --- a/lib/eal/version.map +++ b/lib/eal/version.map @@ -398,6 +398,9 @@ EXPERIMENTAL { # added in 24.11 rte_bitset_to_str; rte_lcore_var_alloc; + + # added in 25.03 + rte_free_sensative; }; INTERNAL { -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 0/8] memset security handling 2024-11-14 1:10 [PATCH 0/3] introduce rte_memset_sensative Stephen Hemminger ` (2 preceding siblings ...) 2024-11-14 1:10 ` [PATCH 3/3] eal: add rte_free_sensative Stephen Hemminger @ 2024-11-14 1:52 ` Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 1/8] eal: introduce new secure memory fill Stephen Hemminger ` (7 more replies) 2024-11-14 2:35 ` [PATCH v3 00/11] memset security handling Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 00/12] memset security fixes Stephen Hemminger 5 siblings, 8 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 1:52 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger This series handles memset related bugs indentified by PVS Studio. The root cause is that Gcc and other compilers are free to optimize away memset called before free. Most of the places memset was being used like this were bogus; probably some developer debug habit, and can be safely removed. Stephen Hemminger (8): eal: introduce new secure memory fill eal: add new secure free function crypto/qat: force zero of keys crypto/qat: fix size calculation for memset crypto/qat: use secure memset bus/uacce: remove memset before free compress/octeontx: remove unnecessary memset test: remove unneeded memset app/test/test_cmdline_cirbuf.c | 2 -- drivers/bus/uacce/uacce.c | 1 - drivers/compress/octeontx/otx_zip.c | 1 - drivers/compress/octeontx/otx_zip_pmd.c | 2 -- drivers/crypto/qat/qat_asym.c | 5 +---- drivers/crypto/qat/qat_sym_session.c | 27 +++++++++++----------- lib/eal/common/rte_malloc.c | 30 ++++++++++++++++++++----- lib/eal/include/rte_malloc.h | 18 +++++++++++++++ lib/eal/include/rte_string_fns.h | 27 ++++++++++++++++++++++ lib/eal/version.map | 3 +++ 10 files changed, 87 insertions(+), 29 deletions(-) -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 1/8] eal: introduce new secure memory fill 2024-11-14 1:52 ` [PATCH v2 0/8] memset security handling Stephen Hemminger @ 2024-11-14 1:52 ` Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 2/8] eal: add new secure free function Stephen Hemminger ` (6 subsequent siblings) 7 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 1:52 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Tyler Retzlaff When memset() is used before a release function such as free, the compiler if allowed to optimize the memset away under the as-if rules. This is normally ok, but in certain cases such as passwords or security keys it is problematic. Introduce a DPDK wrapper which is equivalent to the C++ memset_s function. Naming chosen to be similar to kernel. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- lib/eal/include/rte_string_fns.h | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/lib/eal/include/rte_string_fns.h b/lib/eal/include/rte_string_fns.h index 702bd81251..bf6052c547 100644 --- a/lib/eal/include/rte_string_fns.h +++ b/lib/eal/include/rte_string_fns.h @@ -15,6 +15,7 @@ #include <stdio.h> #include <string.h> +#include <rte_atomic.h> #include <rte_common.h> #include <rte_compat.h> @@ -149,6 +150,32 @@ rte_str_skip_leading_spaces(const char *src) return p; } +/** + * @warning + * @b EXPERIMENTAL: this API may change without prior notice. + * + * Fill memory with constant byte but can not be optimized away. + * Use as a replacement for memset() for sensitive information. + * + * @param dst + * target buffer + * @param ch + * byte to fill + * @param sz + * number of bytes to fill + * + * @return + * like memset() returns a pointer th the memory area dst. + */ +__rte_experimental +static inline void * +rte_memset_sensative(void *dst, int ch, size_t sz) +{ + void *ret = memset(dst, ch, sz); + rte_compiler_barrier(); + return ret; +} + #ifdef __cplusplus } #endif -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 2/8] eal: add new secure free function 2024-11-14 1:52 ` [PATCH v2 0/8] memset security handling Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 1/8] eal: introduce new secure memory fill Stephen Hemminger @ 2024-11-14 1:52 ` Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 3/8] crypto/qat: force zero of keys Stephen Hemminger ` (5 subsequent siblings) 7 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 1:52 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Anatoly Burakov, Tyler Retzlaff Although internally rte_free does poison the buffer in most cases, it is useful to have function that explicitly does this to avoid any security issues. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- lib/eal/common/rte_malloc.c | 30 ++++++++++++++++++++++++------ lib/eal/include/rte_malloc.h | 18 ++++++++++++++++++ lib/eal/version.map | 3 +++ 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/lib/eal/common/rte_malloc.c b/lib/eal/common/rte_malloc.c index 3eed4d4be6..bd543b46e8 100644 --- a/lib/eal/common/rte_malloc.c +++ b/lib/eal/common/rte_malloc.c @@ -15,6 +15,7 @@ #include <rte_eal_memconfig.h> #include <rte_common.h> #include <rte_spinlock.h> +#include <rte_string_fns.h> #include <eal_trace_internal.h> @@ -27,27 +28,44 @@ /* Free the memory space back to heap */ -static void -mem_free(void *addr, const bool trace_ena) +static inline void +mem_free(void *addr, const bool trace_ena, bool zero) { + struct malloc_elem *elem; + if (trace_ena) rte_eal_trace_mem_free(addr); - if (addr == NULL) return; - if (malloc_heap_free(malloc_elem_from_data(addr)) < 0) + if (addr == NULL) + return; + + elem = malloc_elem_from_data(addr); + if (zero) { + size_t data_len = elem->size - MALLOC_ELEM_OVERHEAD; + + rte_memset_sensative(addr, 0, data_len); + } + + if (malloc_heap_free(elem) < 0) EAL_LOG(ERR, "Error: Invalid memory"); } void rte_free(void *addr) { - mem_free(addr, true); + mem_free(addr, true, false); +} + +void +rte_free_sensative(void *addr) +{ + mem_free(addr, true, true); } void eal_free_no_trace(void *addr) { - mem_free(addr, false); + mem_free(addr, false, false); } static void * diff --git a/lib/eal/include/rte_malloc.h b/lib/eal/include/rte_malloc.h index c8836de67c..6675c3f405 100644 --- a/lib/eal/include/rte_malloc.h +++ b/lib/eal/include/rte_malloc.h @@ -51,6 +51,24 @@ struct rte_malloc_socket_stats { void rte_free(void *ptr); + +/** + * Frees the memory space pointed to by the provided pointer + * and guarantees it will be zero'd before reuse. + * + * This pointer must have been returned by a previous call to + * rte_malloc(), rte_zmalloc(), rte_calloc() or rte_realloc(). The behaviour of + * rte_free() is undefined if the pointer does not match this requirement. + * + * If the pointer is NULL, the function does nothing. + * + * @param ptr + * The pointer to memory to be freed. + */ +__rte_experimental +void +rte_free_sensative(void *ptr); + /** * This function allocates memory from the huge-page area of memory. The memory * is not cleared. In NUMA systems, the memory allocated resides on the same diff --git a/lib/eal/version.map b/lib/eal/version.map index a20c713eb1..4e0203053a 100644 --- a/lib/eal/version.map +++ b/lib/eal/version.map @@ -398,6 +398,9 @@ EXPERIMENTAL { # added in 24.11 rte_bitset_to_str; rte_lcore_var_alloc; + + # added in 25.03 + rte_free_sensative; }; INTERNAL { -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 3/8] crypto/qat: force zero of keys 2024-11-14 1:52 ` [PATCH v2 0/8] memset security handling Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 1/8] eal: introduce new secure memory fill Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 2/8] eal: add new secure free function Stephen Hemminger @ 2024-11-14 1:52 ` Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 4/8] crypto/qat: fix size calculation for memset Stephen Hemminger ` (4 subsequent siblings) 7 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 1:52 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Kai Ji Just doing memset() on keys is not enough, compiler can optimize it away. Need something with a barrier. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/crypto/qat/qat_sym_session.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/crypto/qat/qat_sym_session.c b/drivers/crypto/qat/qat_sym_session.c index 50d687fd37..4b4e9ccbab 100644 --- a/drivers/crypto/qat/qat_sym_session.c +++ b/drivers/crypto/qat/qat_sym_session.c @@ -26,6 +26,7 @@ #include <rte_crypto_sym.h> #include <rte_security_driver.h> #include <rte_ether.h> +#include <rte_string_fns.h> #include "qat_logs.h" #include "qat_sym_session.h" @@ -1633,7 +1634,7 @@ static int qat_sym_do_precomputes(enum icp_qat_hw_auth_algo hash_alg, aes_cmac_key_derive(k0, k1); aes_cmac_key_derive(k1, k2); - memset(k0, 0, ICP_QAT_HW_AES_128_KEY_SZ); + rte_memset_sensative(k0, 0, ICP_QAT_HW_AES_128_KEY_SZ); *p_state_len = ICP_QAT_HW_AES_XCBC_MAC_STATE2_SZ; rte_free(in); goto out; @@ -1668,7 +1669,7 @@ static int qat_sym_do_precomputes(enum icp_qat_hw_auth_algo hash_alg, &enc_key) != 0) { rte_free(in - (x * ICP_QAT_HW_AES_XCBC_MAC_KEY_SZ)); - memset(out - + rte_memset_sensative(out - (x * ICP_QAT_HW_AES_XCBC_MAC_KEY_SZ), 0, ICP_QAT_HW_AES_XCBC_MAC_STATE2_SZ); return -EFAULT; @@ -1698,7 +1699,7 @@ static int qat_sym_do_precomputes(enum icp_qat_hw_auth_algo hash_alg, return -ENOMEM; } - memset(in, 0, ICP_QAT_HW_GALOIS_H_SZ); + rte_memset_sensative(in, 0, ICP_QAT_HW_GALOIS_H_SZ); if (AES_set_encrypt_key(auth_key, auth_keylen << 3, &enc_key) != 0) { return -EFAULT; @@ -1757,8 +1758,8 @@ static int qat_sym_do_precomputes(enum icp_qat_hw_auth_algo hash_alg, } /* don't leave data lying around */ - memset(ipad, 0, block_size); - memset(opad, 0, block_size); + rte_memset_sensative(ipad, 0, block_size); + rte_memset_sensative(opad, 0, block_size); out: return 0; } @@ -2006,8 +2007,8 @@ static int qat_sym_do_precomputes_ipsec_mb(enum icp_qat_hw_auth_algo hash_alg, out: /* don't leave data lying around */ - memset(ipad, 0, block_size); - memset(opad, 0, block_size); + rte_memset_sensative(ipad, 0, block_size); + rte_memset_sensative(opad, 0, block_size); free_mb_mgr(m); return ret; } @@ -3232,7 +3233,7 @@ qat_security_session_destroy(void *dev __rte_unused, if (s->mb_mgr) free_mb_mgr(s->mb_mgr); #endif - memset(s, 0, qat_sym_session_get_private_size(dev)); + rte_memset_sensative(s, 0, qat_sym_session_get_private_size(dev)); } return 0; -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 4/8] crypto/qat: fix size calculation for memset 2024-11-14 1:52 ` [PATCH v2 0/8] memset security handling Stephen Hemminger ` (2 preceding siblings ...) 2024-11-14 1:52 ` [PATCH v2 3/8] crypto/qat: force zero of keys Stephen Hemminger @ 2024-11-14 1:52 ` Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 5/8] crypto/qat: use secure memset Stephen Hemminger ` (3 subsequent siblings) 7 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 1:52 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, stable, Kai Ji, Ciara Power, Fan Zhang The memset was always doing 0 bytes since size computed later. Link: https://pvs-studio.com/en/blog/posts/cpp/1179/ Fixes: 3a80d7fb2ecd ("crypto/qat: support SHA3 plain hash") Cc: stable@dpdk.org Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/crypto/qat/qat_sym_session.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/crypto/qat/qat_sym_session.c b/drivers/crypto/qat/qat_sym_session.c index 4b4e9ccbab..8915307c1d 100644 --- a/drivers/crypto/qat/qat_sym_session.c +++ b/drivers/crypto/qat/qat_sym_session.c @@ -2347,7 +2347,7 @@ int qat_sym_cd_cipher_set(struct qat_sym_session *cdesc, return 0; } -int qat_sym_cd_auth_set(struct qat_sym_session *cdesc, +static int qat_sym_cd_auth_set(struct qat_sym_session *cdesc, const uint8_t *authkey, uint32_t authkeylen, uint32_t aad_length, @@ -2620,27 +2620,27 @@ int qat_sym_cd_auth_set(struct qat_sym_session *cdesc, break; case ICP_QAT_HW_AUTH_ALGO_SHA3_224: /* Plain SHA3-224 */ - memset(cdesc->cd_cur_ptr, 0, state1_size); state1_size = qat_hash_get_state1_size( cdesc->qat_hash_alg); + memset(cdesc->cd_cur_ptr, 0, state1_size); break; case ICP_QAT_HW_AUTH_ALGO_SHA3_256: /* Plain SHA3-256 */ - memset(cdesc->cd_cur_ptr, 0, state1_size); state1_size = qat_hash_get_state1_size( cdesc->qat_hash_alg); + memset(cdesc->cd_cur_ptr, 0, state1_size); break; case ICP_QAT_HW_AUTH_ALGO_SHA3_384: /* Plain SHA3-384 */ - memset(cdesc->cd_cur_ptr, 0, state1_size); state1_size = qat_hash_get_state1_size( cdesc->qat_hash_alg); + memset(cdesc->cd_cur_ptr, 0, state1_size); break; case ICP_QAT_HW_AUTH_ALGO_SHA3_512: /* Plain SHA3-512 */ - memset(cdesc->cd_cur_ptr, 0, state1_size); state1_size = qat_hash_get_state1_size( cdesc->qat_hash_alg); + memset(cdesc->cd_cur_ptr, 0, state1_size); break; case ICP_QAT_HW_AUTH_ALGO_AES_XCBC_MAC: state1_size = ICP_QAT_HW_AES_XCBC_MAC_STATE1_SZ; -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 5/8] crypto/qat: use secure memset 2024-11-14 1:52 ` [PATCH v2 0/8] memset security handling Stephen Hemminger ` (3 preceding siblings ...) 2024-11-14 1:52 ` [PATCH v2 4/8] crypto/qat: fix size calculation for memset Stephen Hemminger @ 2024-11-14 1:52 ` Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 6/8] bus/uacce: remove memset before free Stephen Hemminger ` (2 subsequent siblings) 7 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 1:52 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Kai Ji Regular memset maybe removed by compiler if done before a free function. Use new rte_free_sensative instead. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/crypto/qat/qat_asym.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/crypto/qat/qat_asym.c b/drivers/crypto/qat/qat_asym.c index f5b56b2f71..336e7ec0e5 100644 --- a/drivers/crypto/qat/qat_asym.c +++ b/drivers/crypto/qat/qat_asym.c @@ -102,10 +102,7 @@ static const struct rte_driver cryptodev_qat_asym_driver = { curve.p.data, curve.bytesize) #define PARAM_CLR(what) \ - do { \ - memset(what.data, 0, what.length); \ - rte_free(what.data); \ - } while (0) + rte_free_sensative(what.data) static void request_init(struct icp_qat_fw_pke_request *qat_req) -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 6/8] bus/uacce: remove memset before free 2024-11-14 1:52 ` [PATCH v2 0/8] memset security handling Stephen Hemminger ` (4 preceding siblings ...) 2024-11-14 1:52 ` [PATCH v2 5/8] crypto/qat: use secure memset Stephen Hemminger @ 2024-11-14 1:52 ` Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 7/8] compress/octeontx: remove unnecessary memset Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 8/8] test: remove unneeded memset Stephen Hemminger 7 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 1:52 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Chengwen Feng Doing memset before free maybe removed by compiler, and is flagged by security scanning tools as potential problem. In this case the memset is unnecessary. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/bus/uacce/uacce.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/bus/uacce/uacce.c b/drivers/bus/uacce/uacce.c index c1529c38c0..35c1027245 100644 --- a/drivers/bus/uacce/uacce.c +++ b/drivers/bus/uacce/uacce.c @@ -454,7 +454,6 @@ uacce_cleanup(void) dev->device.driver = NULL; free: - memset(dev, 0, sizeof(*dev)); free(dev); } -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 7/8] compress/octeontx: remove unnecessary memset 2024-11-14 1:52 ` [PATCH v2 0/8] memset security handling Stephen Hemminger ` (5 preceding siblings ...) 2024-11-14 1:52 ` [PATCH v2 6/8] bus/uacce: remove memset before free Stephen Hemminger @ 2024-11-14 1:52 ` Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 8/8] test: remove unneeded memset Stephen Hemminger 7 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 1:52 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Ashish Gupta, Fan Zhang Calling memset before rte_free not necessary, and could be removed by the compiler. In this case, the data is not security sensitive so the memset can be removed. Some security scanning tools will flag this. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/compress/octeontx/otx_zip.c | 1 - drivers/compress/octeontx/otx_zip_pmd.c | 2 -- 2 files changed, 3 deletions(-) diff --git a/drivers/compress/octeontx/otx_zip.c b/drivers/compress/octeontx/otx_zip.c index 11471dcbb4..331d2d9475 100644 --- a/drivers/compress/octeontx/otx_zip.c +++ b/drivers/compress/octeontx/otx_zip.c @@ -81,7 +81,6 @@ zipvf_q_term(struct zipvf_qp *qp) struct zip_vf *vf = qp->vf; if (cmdq->va != NULL) { - memset(cmdq->va, 0, ZIP_MAX_CMDQ_SIZE); rte_free(cmdq->va); } diff --git a/drivers/compress/octeontx/otx_zip_pmd.c b/drivers/compress/octeontx/otx_zip_pmd.c index c8f456b319..74e3e942ad 100644 --- a/drivers/compress/octeontx/otx_zip_pmd.c +++ b/drivers/compress/octeontx/otx_zip_pmd.c @@ -479,8 +479,6 @@ zip_pmd_stream_free(struct rte_compressdev *dev, void *stream) (void *)&(z_stream->bufs[0]), (MAX_BUFS_PER_STREAM * ZIP_BURST_SIZE)); - /* Zero out the whole structure */ - memset(stream, 0, sizeof(struct zip_stream)); rte_free(stream); return 0; -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v2 8/8] test: remove unneeded memset 2024-11-14 1:52 ` [PATCH v2 0/8] memset security handling Stephen Hemminger ` (6 preceding siblings ...) 2024-11-14 1:52 ` [PATCH v2 7/8] compress/octeontx: remove unnecessary memset Stephen Hemminger @ 2024-11-14 1:52 ` Stephen Hemminger 7 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 1:52 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Since tmp is not used later in the function, this memset is unnecessary. Even though this is harmless, it causes tools that look for security issues around memset to flag this a bug. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- app/test/test_cmdline_cirbuf.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/test/test_cmdline_cirbuf.c b/app/test/test_cmdline_cirbuf.c index 8ac326cb02..1be357abf6 100644 --- a/app/test/test_cmdline_cirbuf.c +++ b/app/test/test_cmdline_cirbuf.c @@ -281,8 +281,6 @@ test_cirbuf_string_add_del_reverse(void) printf("Error: buffer should have been empty!\n"); return -1; } - /* clear tmp buffer */ - memset(tmp, 0, sizeof(tmp)); /* * reinitialize circular buffer -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 00/11] memset security handling 2024-11-14 1:10 [PATCH 0/3] introduce rte_memset_sensative Stephen Hemminger ` (3 preceding siblings ...) 2024-11-14 1:52 ` [PATCH v2 0/8] memset security handling Stephen Hemminger @ 2024-11-14 2:35 ` Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 01/11] eal: introduce new secure memory fill Stephen Hemminger ` (10 more replies) 2024-11-14 18:43 ` [PATCH v4 00/12] memset security fixes Stephen Hemminger 5 siblings, 11 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 2:35 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger This series handles memset related bugs indentified by PVS Studio. The root cause is that Gcc and other compilers are free to optimize away memset called before free. Most of the places memset was being used like this were bogus; probably some developer debug habit, and can be safely removed. v3 - fix spelling - fix issues reported in Napatech NIC Stephen Hemminger (11): eal: introduce new secure memory fill eal: add new secure free function crypto/qat: force zero of keys crypto/qat: fix size calculation for memset crypto/qat: use secure memset bus/uacce: remove memset before free compress/octeontx: remove unnecessary memset test: remove unneeded memset net/ntnic: remove unnecessary void cast net/ntnic: check result of malloc net/ntnic: remove unnecessary memset app/test/test_cmdline_cirbuf.c | 2 -- drivers/bus/uacce/uacce.c | 1 - drivers/compress/octeontx/otx_zip.c | 1 - drivers/compress/octeontx/otx_zip_pmd.c | 2 -- drivers/crypto/qat/qat_asym.c | 5 +--- drivers/crypto/qat/qat_sym_session.c | 27 +++++++++-------- drivers/net/ntnic/nim/i2c_nim.c | 2 +- drivers/net/ntnic/nthw/core/nthw_hif.c | 5 +--- drivers/net/ntnic/nthw/core/nthw_iic.c | 5 +--- drivers/net/ntnic/nthw/core/nthw_pcie3.c | 5 +--- drivers/net/ntnic/nthw/core/nthw_rpf.c | 5 +--- drivers/net/ntnic/nthw/core/nthw_sdc.c | 5 +--- drivers/net/ntnic/nthw/core/nthw_si5340.c | 5 +--- .../ntnic/nthw/flow_filter/flow_nthw_cat.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_csu.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_flm.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_hfu.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_hsh.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_ifr.c | 2 +- .../ntnic/nthw/flow_filter/flow_nthw_info.c | 7 ++--- .../net/ntnic/nthw/flow_filter/flow_nthw_km.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_pdb.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_qsl.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_slc_lr.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c | 3 +- .../ntnic/nthw/flow_filter/flow_nthw_tx_ins.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c | 7 ++--- drivers/net/ntnic/nthw/nthw_rac.c | 4 ++- drivers/net/ntnic/ntnic_ethdev.c | 2 +- lib/eal/common/rte_malloc.c | 30 +++++++++++++++---- lib/eal/include/rte_malloc.h | 18 +++++++++++ lib/eal/include/rte_string_fns.h | 27 +++++++++++++++++ lib/eal/version.map | 3 ++ 34 files changed, 126 insertions(+), 124 deletions(-) -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 01/11] eal: introduce new secure memory fill 2024-11-14 2:35 ` [PATCH v3 00/11] memset security handling Stephen Hemminger @ 2024-11-14 2:35 ` Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 02/11] eal: add new secure free function Stephen Hemminger ` (9 subsequent siblings) 10 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 2:35 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Tyler Retzlaff When memset() is used before a release function such as free, the compiler if allowed to optimize the memset away under the as-if rules. This is normally ok, but in certain cases such as passwords or security keys it is problematic. Introduce a DPDK wrapper which is equivalent to the C++ memset_s function. Naming chosen to be similar to kernel. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- lib/eal/include/rte_string_fns.h | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/lib/eal/include/rte_string_fns.h b/lib/eal/include/rte_string_fns.h index 702bd81251..4874703957 100644 --- a/lib/eal/include/rte_string_fns.h +++ b/lib/eal/include/rte_string_fns.h @@ -15,6 +15,7 @@ #include <stdio.h> #include <string.h> +#include <rte_atomic.h> #include <rte_common.h> #include <rte_compat.h> @@ -149,6 +150,32 @@ rte_str_skip_leading_spaces(const char *src) return p; } +/** + * @warning + * @b EXPERIMENTAL: this API may change without prior notice. + * + * Fill memory with constant byte but can not be optimized away. + * Use as a replacement for memset() for sensitive information. + * + * @param dst + * target buffer + * @param ch + * byte to fill + * @param sz + * number of bytes to fill + * + * @return + * like memset() returns a pointer th the memory area dst. + */ +__rte_experimental +static inline void * +rte_memset_sensitive(void *dst, int ch, size_t sz) +{ + void *ret = memset(dst, ch, sz); + rte_compiler_barrier(); + return ret; +} + #ifdef __cplusplus } #endif -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 02/11] eal: add new secure free function 2024-11-14 2:35 ` [PATCH v3 00/11] memset security handling Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 01/11] eal: introduce new secure memory fill Stephen Hemminger @ 2024-11-14 2:35 ` Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 03/11] crypto/qat: force zero of keys Stephen Hemminger ` (8 subsequent siblings) 10 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 2:35 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Anatoly Burakov, Tyler Retzlaff Although internally rte_free does poison the buffer in most cases, it is useful to have function that explicitly does this to avoid any security issues. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- lib/eal/common/rte_malloc.c | 30 ++++++++++++++++++++++++------ lib/eal/include/rte_malloc.h | 18 ++++++++++++++++++ lib/eal/version.map | 3 +++ 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/lib/eal/common/rte_malloc.c b/lib/eal/common/rte_malloc.c index 3eed4d4be6..c9e0f4724f 100644 --- a/lib/eal/common/rte_malloc.c +++ b/lib/eal/common/rte_malloc.c @@ -15,6 +15,7 @@ #include <rte_eal_memconfig.h> #include <rte_common.h> #include <rte_spinlock.h> +#include <rte_string_fns.h> #include <eal_trace_internal.h> @@ -27,27 +28,44 @@ /* Free the memory space back to heap */ -static void -mem_free(void *addr, const bool trace_ena) +static inline void +mem_free(void *addr, const bool trace_ena, bool zero) { + struct malloc_elem *elem; + if (trace_ena) rte_eal_trace_mem_free(addr); - if (addr == NULL) return; - if (malloc_heap_free(malloc_elem_from_data(addr)) < 0) + if (addr == NULL) + return; + + elem = malloc_elem_from_data(addr); + if (zero) { + size_t data_len = elem->size - MALLOC_ELEM_OVERHEAD; + + rte_memset_sensitive(addr, 0, data_len); + } + + if (malloc_heap_free(elem) < 0) EAL_LOG(ERR, "Error: Invalid memory"); } void rte_free(void *addr) { - mem_free(addr, true); + mem_free(addr, true, false); +} + +void +rte_free_sensitive(void *addr) +{ + mem_free(addr, true, true); } void eal_free_no_trace(void *addr) { - mem_free(addr, false); + mem_free(addr, false, false); } static void * diff --git a/lib/eal/include/rte_malloc.h b/lib/eal/include/rte_malloc.h index c8836de67c..d472ebb7db 100644 --- a/lib/eal/include/rte_malloc.h +++ b/lib/eal/include/rte_malloc.h @@ -51,6 +51,24 @@ struct rte_malloc_socket_stats { void rte_free(void *ptr); + +/** + * Frees the memory space pointed to by the provided pointer + * and guarantees it will be zero'd before reuse. + * + * This pointer must have been returned by a previous call to + * rte_malloc(), rte_zmalloc(), rte_calloc() or rte_realloc(). The behaviour of + * rte_free() is undefined if the pointer does not match this requirement. + * + * If the pointer is NULL, the function does nothing. + * + * @param ptr + * The pointer to memory to be freed. + */ +__rte_experimental +void +rte_free_sensitive(void *ptr); + /** * This function allocates memory from the huge-page area of memory. The memory * is not cleared. In NUMA systems, the memory allocated resides on the same diff --git a/lib/eal/version.map b/lib/eal/version.map index a20c713eb1..fa67ff44d5 100644 --- a/lib/eal/version.map +++ b/lib/eal/version.map @@ -398,6 +398,9 @@ EXPERIMENTAL { # added in 24.11 rte_bitset_to_str; rte_lcore_var_alloc; + + # added in 25.03 + rte_free_sensitive; }; INTERNAL { -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 03/11] crypto/qat: force zero of keys 2024-11-14 2:35 ` [PATCH v3 00/11] memset security handling Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 01/11] eal: introduce new secure memory fill Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 02/11] eal: add new secure free function Stephen Hemminger @ 2024-11-14 2:35 ` Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 04/11] crypto/qat: fix size calculation for memset Stephen Hemminger ` (7 subsequent siblings) 10 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 2:35 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Kai Ji Just doing memset() on keys is not enough, compiler can optimize it away. Need something with a barrier. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/crypto/qat/qat_sym_session.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/crypto/qat/qat_sym_session.c b/drivers/crypto/qat/qat_sym_session.c index 50d687fd37..2fce8fcb16 100644 --- a/drivers/crypto/qat/qat_sym_session.c +++ b/drivers/crypto/qat/qat_sym_session.c @@ -26,6 +26,7 @@ #include <rte_crypto_sym.h> #include <rte_security_driver.h> #include <rte_ether.h> +#include <rte_string_fns.h> #include "qat_logs.h" #include "qat_sym_session.h" @@ -1633,7 +1634,7 @@ static int qat_sym_do_precomputes(enum icp_qat_hw_auth_algo hash_alg, aes_cmac_key_derive(k0, k1); aes_cmac_key_derive(k1, k2); - memset(k0, 0, ICP_QAT_HW_AES_128_KEY_SZ); + rte_memset_sensitive(k0, 0, ICP_QAT_HW_AES_128_KEY_SZ); *p_state_len = ICP_QAT_HW_AES_XCBC_MAC_STATE2_SZ; rte_free(in); goto out; @@ -1668,7 +1669,7 @@ static int qat_sym_do_precomputes(enum icp_qat_hw_auth_algo hash_alg, &enc_key) != 0) { rte_free(in - (x * ICP_QAT_HW_AES_XCBC_MAC_KEY_SZ)); - memset(out - + rte_memset_sensitive(out - (x * ICP_QAT_HW_AES_XCBC_MAC_KEY_SZ), 0, ICP_QAT_HW_AES_XCBC_MAC_STATE2_SZ); return -EFAULT; @@ -1698,7 +1699,7 @@ static int qat_sym_do_precomputes(enum icp_qat_hw_auth_algo hash_alg, return -ENOMEM; } - memset(in, 0, ICP_QAT_HW_GALOIS_H_SZ); + rte_memset_sensitive(in, 0, ICP_QAT_HW_GALOIS_H_SZ); if (AES_set_encrypt_key(auth_key, auth_keylen << 3, &enc_key) != 0) { return -EFAULT; @@ -1757,8 +1758,8 @@ static int qat_sym_do_precomputes(enum icp_qat_hw_auth_algo hash_alg, } /* don't leave data lying around */ - memset(ipad, 0, block_size); - memset(opad, 0, block_size); + rte_memset_sensitive(ipad, 0, block_size); + rte_memset_sensitive(opad, 0, block_size); out: return 0; } @@ -2006,8 +2007,8 @@ static int qat_sym_do_precomputes_ipsec_mb(enum icp_qat_hw_auth_algo hash_alg, out: /* don't leave data lying around */ - memset(ipad, 0, block_size); - memset(opad, 0, block_size); + rte_memset_sensitive(ipad, 0, block_size); + rte_memset_sensitive(opad, 0, block_size); free_mb_mgr(m); return ret; } @@ -3232,7 +3233,7 @@ qat_security_session_destroy(void *dev __rte_unused, if (s->mb_mgr) free_mb_mgr(s->mb_mgr); #endif - memset(s, 0, qat_sym_session_get_private_size(dev)); + rte_memset_sensitive(s, 0, qat_sym_session_get_private_size(dev)); } return 0; -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 04/11] crypto/qat: fix size calculation for memset 2024-11-14 2:35 ` [PATCH v3 00/11] memset security handling Stephen Hemminger ` (2 preceding siblings ...) 2024-11-14 2:35 ` [PATCH v3 03/11] crypto/qat: force zero of keys Stephen Hemminger @ 2024-11-14 2:35 ` Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 05/11] crypto/qat: use secure memset Stephen Hemminger ` (6 subsequent siblings) 10 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 2:35 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, stable, Kai Ji, Ciara Power, Fan Zhang The memset was always doing 0 bytes since size computed later. Link: https://pvs-studio.com/en/blog/posts/cpp/1179/ Fixes: 3a80d7fb2ecd ("crypto/qat: support SHA3 plain hash") Cc: stable@dpdk.org Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/crypto/qat/qat_sym_session.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/crypto/qat/qat_sym_session.c b/drivers/crypto/qat/qat_sym_session.c index 2fce8fcb16..6f3eacaf43 100644 --- a/drivers/crypto/qat/qat_sym_session.c +++ b/drivers/crypto/qat/qat_sym_session.c @@ -2347,7 +2347,7 @@ int qat_sym_cd_cipher_set(struct qat_sym_session *cdesc, return 0; } -int qat_sym_cd_auth_set(struct qat_sym_session *cdesc, +static int qat_sym_cd_auth_set(struct qat_sym_session *cdesc, const uint8_t *authkey, uint32_t authkeylen, uint32_t aad_length, @@ -2620,27 +2620,27 @@ int qat_sym_cd_auth_set(struct qat_sym_session *cdesc, break; case ICP_QAT_HW_AUTH_ALGO_SHA3_224: /* Plain SHA3-224 */ - memset(cdesc->cd_cur_ptr, 0, state1_size); state1_size = qat_hash_get_state1_size( cdesc->qat_hash_alg); + memset(cdesc->cd_cur_ptr, 0, state1_size); break; case ICP_QAT_HW_AUTH_ALGO_SHA3_256: /* Plain SHA3-256 */ - memset(cdesc->cd_cur_ptr, 0, state1_size); state1_size = qat_hash_get_state1_size( cdesc->qat_hash_alg); + memset(cdesc->cd_cur_ptr, 0, state1_size); break; case ICP_QAT_HW_AUTH_ALGO_SHA3_384: /* Plain SHA3-384 */ - memset(cdesc->cd_cur_ptr, 0, state1_size); state1_size = qat_hash_get_state1_size( cdesc->qat_hash_alg); + memset(cdesc->cd_cur_ptr, 0, state1_size); break; case ICP_QAT_HW_AUTH_ALGO_SHA3_512: /* Plain SHA3-512 */ - memset(cdesc->cd_cur_ptr, 0, state1_size); state1_size = qat_hash_get_state1_size( cdesc->qat_hash_alg); + memset(cdesc->cd_cur_ptr, 0, state1_size); break; case ICP_QAT_HW_AUTH_ALGO_AES_XCBC_MAC: state1_size = ICP_QAT_HW_AES_XCBC_MAC_STATE1_SZ; -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 05/11] crypto/qat: use secure memset 2024-11-14 2:35 ` [PATCH v3 00/11] memset security handling Stephen Hemminger ` (3 preceding siblings ...) 2024-11-14 2:35 ` [PATCH v3 04/11] crypto/qat: fix size calculation for memset Stephen Hemminger @ 2024-11-14 2:35 ` Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 06/11] bus/uacce: remove memset before free Stephen Hemminger ` (5 subsequent siblings) 10 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 2:35 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Kai Ji Regular memset maybe removed by compiler if done before a free function. Use new rte_free_sensitive instead. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/crypto/qat/qat_asym.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/crypto/qat/qat_asym.c b/drivers/crypto/qat/qat_asym.c index f5b56b2f71..d8a1406819 100644 --- a/drivers/crypto/qat/qat_asym.c +++ b/drivers/crypto/qat/qat_asym.c @@ -102,10 +102,7 @@ static const struct rte_driver cryptodev_qat_asym_driver = { curve.p.data, curve.bytesize) #define PARAM_CLR(what) \ - do { \ - memset(what.data, 0, what.length); \ - rte_free(what.data); \ - } while (0) + rte_free_sensitive(what.data) static void request_init(struct icp_qat_fw_pke_request *qat_req) -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 06/11] bus/uacce: remove memset before free 2024-11-14 2:35 ` [PATCH v3 00/11] memset security handling Stephen Hemminger ` (4 preceding siblings ...) 2024-11-14 2:35 ` [PATCH v3 05/11] crypto/qat: use secure memset Stephen Hemminger @ 2024-11-14 2:35 ` Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 07/11] compress/octeontx: remove unnecessary memset Stephen Hemminger ` (4 subsequent siblings) 10 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 2:35 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Chengwen Feng Doing memset before free maybe removed by compiler, and is flagged by security scanning tools as potential problem. In this case the memset is unnecessary. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/bus/uacce/uacce.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/bus/uacce/uacce.c b/drivers/bus/uacce/uacce.c index c1529c38c0..35c1027245 100644 --- a/drivers/bus/uacce/uacce.c +++ b/drivers/bus/uacce/uacce.c @@ -454,7 +454,6 @@ uacce_cleanup(void) dev->device.driver = NULL; free: - memset(dev, 0, sizeof(*dev)); free(dev); } -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 07/11] compress/octeontx: remove unnecessary memset 2024-11-14 2:35 ` [PATCH v3 00/11] memset security handling Stephen Hemminger ` (5 preceding siblings ...) 2024-11-14 2:35 ` [PATCH v3 06/11] bus/uacce: remove memset before free Stephen Hemminger @ 2024-11-14 2:35 ` Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 08/11] test: remove unneeded memset Stephen Hemminger ` (3 subsequent siblings) 10 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 2:35 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Ashish Gupta, Fan Zhang Calling memset before rte_free not necessary, and could be removed by the compiler. In this case, the data is not security sensitive so the memset can be removed. Some security scanning tools will flag this. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/compress/octeontx/otx_zip.c | 1 - drivers/compress/octeontx/otx_zip_pmd.c | 2 -- 2 files changed, 3 deletions(-) diff --git a/drivers/compress/octeontx/otx_zip.c b/drivers/compress/octeontx/otx_zip.c index 11471dcbb4..331d2d9475 100644 --- a/drivers/compress/octeontx/otx_zip.c +++ b/drivers/compress/octeontx/otx_zip.c @@ -81,7 +81,6 @@ zipvf_q_term(struct zipvf_qp *qp) struct zip_vf *vf = qp->vf; if (cmdq->va != NULL) { - memset(cmdq->va, 0, ZIP_MAX_CMDQ_SIZE); rte_free(cmdq->va); } diff --git a/drivers/compress/octeontx/otx_zip_pmd.c b/drivers/compress/octeontx/otx_zip_pmd.c index c8f456b319..74e3e942ad 100644 --- a/drivers/compress/octeontx/otx_zip_pmd.c +++ b/drivers/compress/octeontx/otx_zip_pmd.c @@ -479,8 +479,6 @@ zip_pmd_stream_free(struct rte_compressdev *dev, void *stream) (void *)&(z_stream->bufs[0]), (MAX_BUFS_PER_STREAM * ZIP_BURST_SIZE)); - /* Zero out the whole structure */ - memset(stream, 0, sizeof(struct zip_stream)); rte_free(stream); return 0; -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 08/11] test: remove unneeded memset 2024-11-14 2:35 ` [PATCH v3 00/11] memset security handling Stephen Hemminger ` (6 preceding siblings ...) 2024-11-14 2:35 ` [PATCH v3 07/11] compress/octeontx: remove unnecessary memset Stephen Hemminger @ 2024-11-14 2:35 ` Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 09/11] net/ntnic: remove unnecessary void cast Stephen Hemminger ` (2 subsequent siblings) 10 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 2:35 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Since tmp is not used later in the function, this memset is unnecessary. Even though this is harmless, it causes tools that look for security issues around memset to flag this a bug. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- app/test/test_cmdline_cirbuf.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/test/test_cmdline_cirbuf.c b/app/test/test_cmdline_cirbuf.c index 8ac326cb02..1be357abf6 100644 --- a/app/test/test_cmdline_cirbuf.c +++ b/app/test/test_cmdline_cirbuf.c @@ -281,8 +281,6 @@ test_cirbuf_string_add_del_reverse(void) printf("Error: buffer should have been empty!\n"); return -1; } - /* clear tmp buffer */ - memset(tmp, 0, sizeof(tmp)); /* * reinitialize circular buffer -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 09/11] net/ntnic: remove unnecessary void cast 2024-11-14 2:35 ` [PATCH v3 00/11] memset security handling Stephen Hemminger ` (7 preceding siblings ...) 2024-11-14 2:35 ` [PATCH v3 08/11] test: remove unneeded memset Stephen Hemminger @ 2024-11-14 2:35 ` Stephen Hemminger 2024-11-17 18:16 ` Serhii Iliushyk 2024-11-14 2:35 ` [PATCH v3 10/11] net/ntnic: check result of malloc Stephen Hemminger 2024-11-14 2:36 ` [PATCH v3 11/11] net/ntnic: remove unnecessary memset Stephen Hemminger 10 siblings, 1 reply; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 2:35 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Christian Koue Muf, Serhii Iliushyk There is no need to cast memset to void. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/net/ntnic/nim/i2c_nim.c | 2 +- drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_ifr.c | 2 +- drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c | 4 ++-- drivers/net/ntnic/ntnic_ethdev.c | 2 +- 17 files changed, 31 insertions(+), 31 deletions(-) diff --git a/drivers/net/ntnic/nim/i2c_nim.c b/drivers/net/ntnic/nim/i2c_nim.c index e6f7755ded..f9fec5f767 100644 --- a/drivers/net/ntnic/nim/i2c_nim.c +++ b/drivers/net/ntnic/nim/i2c_nim.c @@ -292,7 +292,7 @@ static int qsfp_nim_state_build(nim_i2c_ctx_t *ctx, sfp_nim_state_t *state) assert(ctx && state); assert(ctx->nim_id != NT_NIM_UNKNOWN && "Nim is not initialized"); - (void)memset(state, 0, sizeof(*state)); + memset(state, 0, sizeof(*state)); switch (ctx->nim_id) { case 12U: diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c index d3213593e1..649a060682 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c @@ -18,7 +18,7 @@ struct cat_nthw *cat_nthw_new(void) struct cat_nthw *p = malloc(sizeof(struct cat_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -26,7 +26,7 @@ struct cat_nthw *cat_nthw_new(void) void cat_nthw_delete(struct cat_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c index c1b73179a8..dc3582c9f5 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c @@ -22,7 +22,7 @@ struct csu_nthw *csu_nthw_new(void) struct csu_nthw *p = malloc(sizeof(struct csu_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -30,7 +30,7 @@ struct csu_nthw *csu_nthw_new(void) void csu_nthw_delete(struct csu_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c index 8855978349..7647949bc7 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c @@ -19,7 +19,7 @@ struct flm_nthw *flm_nthw_new(void) struct flm_nthw *p = malloc(sizeof(struct flm_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -27,7 +27,7 @@ struct flm_nthw *flm_nthw_new(void) void flm_nthw_delete(struct flm_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c index 4a8b17101d..d0e65c071d 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c @@ -22,7 +22,7 @@ struct hfu_nthw *hfu_nthw_new(void) struct hfu_nthw *p = malloc(sizeof(struct hfu_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -30,7 +30,7 @@ struct hfu_nthw *hfu_nthw_new(void) void hfu_nthw_delete(struct hfu_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c index 80ead2729a..caf84790cd 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c @@ -23,7 +23,7 @@ struct hsh_nthw *hsh_nthw_new(void) struct hsh_nthw *p = malloc(sizeof(struct hsh_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -31,7 +31,7 @@ struct hsh_nthw *hsh_nthw_new(void) void hsh_nthw_delete(struct hsh_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_ifr.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_ifr.c index 11b6b5e5b8..df2bd1cc5c 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_ifr.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_ifr.c @@ -22,7 +22,7 @@ struct ifr_nthw *ifr_nthw_new(void) struct ifr_nthw *p = malloc(sizeof(struct ifr_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c index 8e0b24dd9a..007c71d1b0 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c @@ -23,7 +23,7 @@ struct info_nthw *info_nthw_new(void) struct info_nthw *p = malloc(sizeof(struct info_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -31,7 +31,7 @@ struct info_nthw *info_nthw_new(void) void info_nthw_delete(struct info_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c index edc8f58759..3909194214 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c @@ -32,7 +32,7 @@ struct km_nthw *km_nthw_new(void) struct km_nthw *p = malloc(sizeof(struct km_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -40,7 +40,7 @@ struct km_nthw *km_nthw_new(void) void km_nthw_delete(struct km_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c index 4a9713965b..700d868cef 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c @@ -23,7 +23,7 @@ struct pdb_nthw *pdb_nthw_new(void) struct pdb_nthw *p = malloc(sizeof(struct pdb_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -31,7 +31,7 @@ struct pdb_nthw *pdb_nthw_new(void) void pdb_nthw_delete(struct pdb_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c index c887fe25e2..47fea6553f 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c @@ -23,7 +23,7 @@ struct qsl_nthw *qsl_nthw_new(void) struct qsl_nthw *p = malloc(sizeof(struct qsl_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -31,7 +31,7 @@ struct qsl_nthw *qsl_nthw_new(void) void qsl_nthw_delete(struct qsl_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c index 28c7a05fe2..2f141acbde 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c @@ -22,7 +22,7 @@ struct rpp_lr_nthw *rpp_lr_nthw_new(void) struct rpp_lr_nthw *p = malloc(sizeof(struct rpp_lr_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -30,7 +30,7 @@ struct rpp_lr_nthw *rpp_lr_nthw_new(void) void rpp_lr_nthw_delete(struct rpp_lr_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c index e062a700eb..e278ad0a41 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c @@ -23,7 +23,7 @@ struct slc_lr_nthw *slc_lr_nthw_new(void) struct slc_lr_nthw *p = malloc(sizeof(struct slc_lr_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -31,7 +31,7 @@ struct slc_lr_nthw *slc_lr_nthw_new(void) void slc_lr_nthw_delete(struct slc_lr_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c index ee85a1c61b..58e5672462 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c @@ -22,7 +22,7 @@ struct tx_cpy_nthw *tx_cpy_nthw_new(void) struct tx_cpy_nthw *p = malloc(sizeof(struct tx_cpy_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -31,7 +31,7 @@ void tx_cpy_nthw_delete(struct tx_cpy_nthw *p) { if (p) { free(p->m_writers); - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c index 47af44945f..0ff9af22c7 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c @@ -22,7 +22,7 @@ struct tx_ins_nthw *tx_ins_nthw_new(void) struct tx_ins_nthw *p = malloc(sizeof(struct tx_ins_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -30,7 +30,7 @@ struct tx_ins_nthw *tx_ins_nthw_new(void) void tx_ins_nthw_delete(struct tx_ins_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c index c40857db0f..221ea94eed 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c @@ -22,7 +22,7 @@ struct tx_rpl_nthw *tx_rpl_nthw_new(void) struct tx_rpl_nthw *p = malloc(sizeof(struct tx_rpl_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -30,7 +30,7 @@ struct tx_rpl_nthw *tx_rpl_nthw_new(void) void tx_rpl_nthw_delete(struct tx_rpl_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/ntnic_ethdev.c b/drivers/net/ntnic/ntnic_ethdev.c index 2a2643a106..9f3e9440dd 100644 --- a/drivers/net/ntnic/ntnic_ethdev.c +++ b/drivers/net/ntnic/ntnic_ethdev.c @@ -1251,7 +1251,7 @@ eth_set_mc_addr_list(struct rte_eth_dev *eth_dev, mc_addrs[i] = mc_addr_set[i]; else - (void)memset(&mc_addrs[i], 0, sizeof(mc_addrs[i])); + memset(&mc_addrs[i], 0, sizeof(mc_addrs[i])); return 0; } -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 09/11] net/ntnic: remove unnecessary void cast 2024-11-14 2:35 ` [PATCH v3 09/11] net/ntnic: remove unnecessary void cast Stephen Hemminger @ 2024-11-17 18:16 ` Serhii Iliushyk 0 siblings, 0 replies; 42+ messages in thread From: Serhii Iliushyk @ 2024-11-17 18:16 UTC (permalink / raw) To: Stephen Hemminger, dev; +Cc: Christian Koue Muf >On 14.11.2024, 04:38, "Stephen Hemminger" wrote: > > >There is no need to cast memset to void. > > >Signed-off-by: Stephen Hemminger <stephen@networkplumber.org <mailto:stephen@networkplumber.org>> >--- >drivers/net/ntnic/nim/i2c_nim.c | 2 +- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c | 4 ++-- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c | 4 ++-- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c | 4 ++-- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c | 4 ++-- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c | 4 ++-- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_ifr.c | 2 +- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c | 4 ++-- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c | 4 ++-- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c | 4 ++-- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c | 4 ++-- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c | 4 ++-- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c | 4 ++-- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c | 4 ++-- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c | 4 ++-- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c | 4 ++-- >drivers/net/ntnic/ntnic_ethdev.c | 2 +- >17 files changed, 31 insertions(+), 31 deletions(-) > > >diff --git a/drivers/net/ntnic/nim/i2c_nim.c b/drivers/net/ntnic/nim/i2c_nim.c >index e6f7755ded..f9fec5f767 100644 >--- a/drivers/net/ntnic/nim/i2c_nim.c >+++ b/drivers/net/ntnic/nim/i2c_nim.c >@@ -292,7 +292,7 @@ static int qsfp_nim_state_build(nim_i2c_ctx_t *ctx, sfp_nim_state_t *state) >assert(ctx && state); >assert(ctx->nim_id != NT_NIM_UNKNOWN && "Nim is not initialized"); > > >- (void)memset(state, 0, sizeof(*state)); >+ memset(state, 0, sizeof(*state)); > > >switch (ctx->nim_id) { >case 12U: >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c >index d3213593e1..649a060682 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c >@@ -18,7 +18,7 @@ struct cat_nthw *cat_nthw_new(void) >struct cat_nthw *p = malloc(sizeof(struct cat_nthw)); > > >if (p) >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); > > >return p; >} >@@ -26,7 +26,7 @@ struct cat_nthw *cat_nthw_new(void) >void cat_nthw_delete(struct cat_nthw *p) >{ >if (p) { >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); >free(p); >} >} >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c >index c1b73179a8..dc3582c9f5 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c >@@ -22,7 +22,7 @@ struct csu_nthw *csu_nthw_new(void) >struct csu_nthw *p = malloc(sizeof(struct csu_nthw)); > > >if (p) >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); > > >return p; >} >@@ -30,7 +30,7 @@ struct csu_nthw *csu_nthw_new(void) >void csu_nthw_delete(struct csu_nthw *p) >{ >if (p) { >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); >free(p); >} >} >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c >index 8855978349..7647949bc7 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c >@@ -19,7 +19,7 @@ struct flm_nthw *flm_nthw_new(void) >struct flm_nthw *p = malloc(sizeof(struct flm_nthw)); > > >if (p) >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); > > >return p; >} >@@ -27,7 +27,7 @@ struct flm_nthw *flm_nthw_new(void) >void flm_nthw_delete(struct flm_nthw *p) >{ >if (p) { >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); >free(p); >} >} >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c >index 4a8b17101d..d0e65c071d 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c >@@ -22,7 +22,7 @@ struct hfu_nthw *hfu_nthw_new(void) >struct hfu_nthw *p = malloc(sizeof(struct hfu_nthw)); > > >if (p) >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); > > >return p; >} >@@ -30,7 +30,7 @@ struct hfu_nthw *hfu_nthw_new(void) >void hfu_nthw_delete(struct hfu_nthw *p) >{ >if (p) { >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); >free(p); >} >} >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c >index 80ead2729a..caf84790cd 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c >@@ -23,7 +23,7 @@ struct hsh_nthw *hsh_nthw_new(void) >struct hsh_nthw *p = malloc(sizeof(struct hsh_nthw)); > > >if (p) >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); > > >return p; >} >@@ -31,7 +31,7 @@ struct hsh_nthw *hsh_nthw_new(void) >void hsh_nthw_delete(struct hsh_nthw *p) >{ >if (p) { >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); >free(p); >} >} >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_ifr.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_ifr.c >index 11b6b5e5b8..df2bd1cc5c 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_ifr.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_ifr.c >@@ -22,7 +22,7 @@ struct ifr_nthw *ifr_nthw_new(void) >struct ifr_nthw *p = malloc(sizeof(struct ifr_nthw)); > > >if (p) >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); > > >return p; >} >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c >index 8e0b24dd9a..007c71d1b0 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c >@@ -23,7 +23,7 @@ struct info_nthw *info_nthw_new(void) >struct info_nthw *p = malloc(sizeof(struct info_nthw)); > > >if (p) >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); > > >return p; >} >@@ -31,7 +31,7 @@ struct info_nthw *info_nthw_new(void) >void info_nthw_delete(struct info_nthw *p) >{ >if (p) { >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); >free(p); >} >} >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c >index edc8f58759..3909194214 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c >@@ -32,7 +32,7 @@ struct km_nthw *km_nthw_new(void) >struct km_nthw *p = malloc(sizeof(struct km_nthw)); > > >if (p) >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); > > >return p; >} >@@ -40,7 +40,7 @@ struct km_nthw *km_nthw_new(void) >void km_nthw_delete(struct km_nthw *p) >{ >if (p) { >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); >free(p); >} >} >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c >index 4a9713965b..700d868cef 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c >@@ -23,7 +23,7 @@ struct pdb_nthw *pdb_nthw_new(void) >struct pdb_nthw *p = malloc(sizeof(struct pdb_nthw)); > > >if (p) >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); > > >return p; >} >@@ -31,7 +31,7 @@ struct pdb_nthw *pdb_nthw_new(void) >void pdb_nthw_delete(struct pdb_nthw *p) >{ >if (p) { >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); >free(p); >} >} >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c >index c887fe25e2..47fea6553f 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c >@@ -23,7 +23,7 @@ struct qsl_nthw *qsl_nthw_new(void) >struct qsl_nthw *p = malloc(sizeof(struct qsl_nthw)); > > >if (p) >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); > > >return p; >} >@@ -31,7 +31,7 @@ struct qsl_nthw *qsl_nthw_new(void) >void qsl_nthw_delete(struct qsl_nthw *p) >{ >if (p) { >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); >free(p); >} >} >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c >index 28c7a05fe2..2f141acbde 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c >@@ -22,7 +22,7 @@ struct rpp_lr_nthw *rpp_lr_nthw_new(void) >struct rpp_lr_nthw *p = malloc(sizeof(struct rpp_lr_nthw)); > > >if (p) >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); > > >return p; >} >@@ -30,7 +30,7 @@ struct rpp_lr_nthw *rpp_lr_nthw_new(void) >void rpp_lr_nthw_delete(struct rpp_lr_nthw *p) >{ >if (p) { >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); >free(p); >} >} >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c >index e062a700eb..e278ad0a41 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c >@@ -23,7 +23,7 @@ struct slc_lr_nthw *slc_lr_nthw_new(void) >struct slc_lr_nthw *p = malloc(sizeof(struct slc_lr_nthw)); > > >if (p) >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); > > >return p; >} >@@ -31,7 +31,7 @@ struct slc_lr_nthw *slc_lr_nthw_new(void) >void slc_lr_nthw_delete(struct slc_lr_nthw *p) >{ >if (p) { >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); >free(p); >} >} >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c >index ee85a1c61b..58e5672462 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c >@@ -22,7 +22,7 @@ struct tx_cpy_nthw *tx_cpy_nthw_new(void) >struct tx_cpy_nthw *p = malloc(sizeof(struct tx_cpy_nthw)); > > >if (p) >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); > > >return p; >} >@@ -31,7 +31,7 @@ void tx_cpy_nthw_delete(struct tx_cpy_nthw *p) >{ >if (p) { >free(p->m_writers); >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); >free(p); >} >} >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c >index 47af44945f..0ff9af22c7 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c >@@ -22,7 +22,7 @@ struct tx_ins_nthw *tx_ins_nthw_new(void) >struct tx_ins_nthw *p = malloc(sizeof(struct tx_ins_nthw)); > > >if (p) >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); > > >return p; >} >@@ -30,7 +30,7 @@ struct tx_ins_nthw *tx_ins_nthw_new(void) >void tx_ins_nthw_delete(struct tx_ins_nthw *p) >{ >if (p) { >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); >free(p); >} >} >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c >index c40857db0f..221ea94eed 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c >@@ -22,7 +22,7 @@ struct tx_rpl_nthw *tx_rpl_nthw_new(void) >struct tx_rpl_nthw *p = malloc(sizeof(struct tx_rpl_nthw)); > > >if (p) >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); > > >return p; >} >@@ -30,7 +30,7 @@ struct tx_rpl_nthw *tx_rpl_nthw_new(void) >void tx_rpl_nthw_delete(struct tx_rpl_nthw *p) >{ >if (p) { >- (void)memset(p, 0, sizeof(*p)); >+ memset(p, 0, sizeof(*p)); >free(p); >} >} >diff --git a/drivers/net/ntnic/ntnic_ethdev.c b/drivers/net/ntnic/ntnic_ethdev.c >index 2a2643a106..9f3e9440dd 100644 >--- a/drivers/net/ntnic/ntnic_ethdev.c >+++ b/drivers/net/ntnic/ntnic_ethdev.c >@@ -1251,7 +1251,7 @@ eth_set_mc_addr_list(struct rte_eth_dev *eth_dev, >mc_addrs[i] = mc_addr_set[i]; > > >else >- (void)memset(&mc_addrs[i], 0, sizeof(mc_addrs[i])); >+ memset(&mc_addrs[i], 0, sizeof(mc_addrs[i])); > > >return 0; >} >-- >2.45.2 > > Hi Stephen, Thanks a lot for fixing this issue. We appreciate it. Reviewed-by: Serhii Iliushyk <sil-plv@napatech.com> ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 10/11] net/ntnic: check result of malloc 2024-11-14 2:35 ` [PATCH v3 00/11] memset security handling Stephen Hemminger ` (8 preceding siblings ...) 2024-11-14 2:35 ` [PATCH v3 09/11] net/ntnic: remove unnecessary void cast Stephen Hemminger @ 2024-11-14 2:35 ` Stephen Hemminger 2024-11-17 18:24 ` Serhii Iliushyk 2024-11-14 2:36 ` [PATCH v3 11/11] net/ntnic: remove unnecessary memset Stephen Hemminger 10 siblings, 1 reply; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 2:35 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Christian Koue Muf, Serhii Iliushyk Need to check the result of malloc() before calling memset. This is only place in this driver that forgot, other code does check. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/net/ntnic/nthw/nthw_rac.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/ntnic/nthw/nthw_rac.c b/drivers/net/ntnic/nthw/nthw_rac.c index ca6aba6db2..f275e64da3 100644 --- a/drivers/net/ntnic/nthw/nthw_rac.c +++ b/drivers/net/ntnic/nthw/nthw_rac.c @@ -31,7 +31,9 @@ nthw_rac_t *nthw_rac_new(void) { nthw_rac_t *p = malloc(sizeof(nthw_rac_t)); - memset(p, 0, sizeof(nthw_rac_t)); + + if (p) + memset(p, 0, sizeof(nthw_rac_t)); return p; } -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 10/11] net/ntnic: check result of malloc 2024-11-14 2:35 ` [PATCH v3 10/11] net/ntnic: check result of malloc Stephen Hemminger @ 2024-11-17 18:24 ` Serhii Iliushyk 0 siblings, 0 replies; 42+ messages in thread From: Serhii Iliushyk @ 2024-11-17 18:24 UTC (permalink / raw) To: Stephen Hemminger, dev; +Cc: Christian Koue Muf On 14.11.2024, 04:38, "Stephen Hemminger" wrote: > > >Need to check the result of malloc() before calling memset. >This is only place in this driver that forgot, other code >does check. > > >Signed-off-by: Stephen Hemminger <stephen@networkplumber.org <mailto:stephen@networkplumber.org>> >--- >drivers/net/ntnic/nthw/nthw_rac.c | 4 +++- >1 file changed, 3 insertions(+), 1 deletion(-) > > >diff --git a/drivers/net/ntnic/nthw/nthw_rac.c b/drivers/net/ntnic/nthw/nthw_rac.c >index ca6aba6db2..f275e64da3 100644 >--- a/drivers/net/ntnic/nthw/nthw_rac.c >+++ b/drivers/net/ntnic/nthw/nthw_rac.c >@@ -31,7 +31,9 @@ >nthw_rac_t *nthw_rac_new(void) >{ >nthw_rac_t *p = malloc(sizeof(nthw_rac_t)); >- memset(p, 0, sizeof(nthw_rac_t)); >+ >+ if (p) >+ memset(p, 0, sizeof(nthw_rac_t)); >return p; >} > > >-- >2.45.2 > > Hi Stephen, Thanks a lot for noticing and fixing this issue. We appreciate it. Reviewed-by: Serhii Iliushyk <sil-plv@napatech.com> ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v3 11/11] net/ntnic: remove unnecessary memset 2024-11-14 2:35 ` [PATCH v3 00/11] memset security handling Stephen Hemminger ` (9 preceding siblings ...) 2024-11-14 2:35 ` [PATCH v3 10/11] net/ntnic: check result of malloc Stephen Hemminger @ 2024-11-14 2:36 ` Stephen Hemminger 2024-11-17 18:26 ` Serhii Iliushyk 10 siblings, 1 reply; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 2:36 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Christian Koue Muf, Serhii Iliushyk Calling memset before free() has no effect and will be flagged by security parsing tools as a potential bug. None of these data structures have sensitive information. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/net/ntnic/nthw/core/nthw_hif.c | 5 +---- drivers/net/ntnic/nthw/core/nthw_iic.c | 5 +---- drivers/net/ntnic/nthw/core/nthw_pcie3.c | 5 +---- drivers/net/ntnic/nthw/core/nthw_rpf.c | 5 +---- drivers/net/ntnic/nthw/core/nthw_sdc.c | 5 +---- drivers/net/ntnic/nthw/core/nthw_si5340.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c | 1 - drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c | 5 +---- 20 files changed, 19 insertions(+), 77 deletions(-) diff --git a/drivers/net/ntnic/nthw/core/nthw_hif.c b/drivers/net/ntnic/nthw/core/nthw_hif.c index 9f699e4f94..d702257d76 100644 --- a/drivers/net/ntnic/nthw/core/nthw_hif.c +++ b/drivers/net/ntnic/nthw/core/nthw_hif.c @@ -23,10 +23,7 @@ nthw_hif_t *nthw_hif_new(void) void nthw_hif_delete(nthw_hif_t *p) { - if (p) { - memset(p, 0, sizeof(nthw_hif_t)); - free(p); - } + free(p); } int nthw_hif_init(nthw_hif_t *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/core/nthw_iic.c b/drivers/net/ntnic/nthw/core/nthw_iic.c index 269754c24a..a98ec659c4 100644 --- a/drivers/net/ntnic/nthw/core/nthw_iic.c +++ b/drivers/net/ntnic/nthw/core/nthw_iic.c @@ -253,10 +253,7 @@ int nthw_iic_init(nthw_iic_t *p, nthw_fpga_t *p_fpga, int n_iic_instance, void nthw_iic_delete(nthw_iic_t *p) { - if (p) { - memset(p, 0, sizeof(nthw_iic_t)); - free(p); - } + free(p); } int nthw_iic_set_retry_params(nthw_iic_t *p, const int n_poll_delay, const int n_bus_ready_retry, diff --git a/drivers/net/ntnic/nthw/core/nthw_pcie3.c b/drivers/net/ntnic/nthw/core/nthw_pcie3.c index 5997ebb419..a5833e166c 100644 --- a/drivers/net/ntnic/nthw/core/nthw_pcie3.c +++ b/drivers/net/ntnic/nthw/core/nthw_pcie3.c @@ -24,10 +24,7 @@ nthw_pcie3_t *nthw_pcie3_new(void) void nthw_pcie3_delete(nthw_pcie3_t *p) { - if (p) { - memset(p, 0, sizeof(nthw_pcie3_t)); - free(p); - } + free(p); } int nthw_pcie3_init(nthw_pcie3_t *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/core/nthw_rpf.c b/drivers/net/ntnic/nthw/core/nthw_rpf.c index 1ed4d7b4e0..d5c19e312b 100644 --- a/drivers/net/ntnic/nthw/core/nthw_rpf.c +++ b/drivers/net/ntnic/nthw/core/nthw_rpf.c @@ -22,10 +22,7 @@ nthw_rpf_t *nthw_rpf_new(void) void nthw_rpf_delete(nthw_rpf_t *p) { - if (p) { - memset(p, 0, sizeof(nthw_rpf_t)); - free(p); - } + free(p); } int nthw_rpf_init(nthw_rpf_t *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/core/nthw_sdc.c b/drivers/net/ntnic/nthw/core/nthw_sdc.c index fc73e6957c..e32d87b967 100644 --- a/drivers/net/ntnic/nthw/core/nthw_sdc.c +++ b/drivers/net/ntnic/nthw/core/nthw_sdc.c @@ -22,10 +22,7 @@ nthw_sdc_t *nthw_sdc_new(void) void nthw_sdc_delete(nthw_sdc_t *p) { - if (p) { - memset(p, 0, sizeof(nthw_sdc_t)); - free(p); - } + free(p); } int nthw_sdc_init(nthw_sdc_t *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/core/nthw_si5340.c b/drivers/net/ntnic/nthw/core/nthw_si5340.c index 05cadc0bf4..ceaa58e0f7 100644 --- a/drivers/net/ntnic/nthw/core/nthw_si5340.c +++ b/drivers/net/ntnic/nthw/core/nthw_si5340.c @@ -44,10 +44,7 @@ int nthw_si5340_init(nthw_si5340_t *p, nthw_iic_t *p_nthw_iic, uint8_t n_iic_add void nthw_si5340_delete(nthw_si5340_t *p) { - if (p) { - memset(p, 0, sizeof(nthw_si5340_t)); - free(p); - } + free(p); } /* diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c index 649a060682..776d4986f7 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c @@ -25,10 +25,7 @@ struct cat_nthw *cat_nthw_new(void) void cat_nthw_delete(struct cat_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } void cat_nthw_set_debug_mode(struct cat_nthw *p, unsigned int n_debug_mode) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c index dc3582c9f5..036a25d06c 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c @@ -29,10 +29,7 @@ struct csu_nthw *csu_nthw_new(void) void csu_nthw_delete(struct csu_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int csu_nthw_init(struct csu_nthw *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c index 7647949bc7..e016c80bc9 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c @@ -26,10 +26,7 @@ struct flm_nthw *flm_nthw_new(void) void flm_nthw_delete(struct flm_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } void flm_nthw_set_debug_mode(struct flm_nthw *p, unsigned int n_debug_mode) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c index d0e65c071d..02064f1bc6 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c @@ -29,10 +29,7 @@ struct hfu_nthw *hfu_nthw_new(void) void hfu_nthw_delete(struct hfu_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int hfu_nthw_init(struct hfu_nthw *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c index caf84790cd..10f982ea76 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c @@ -30,10 +30,7 @@ struct hsh_nthw *hsh_nthw_new(void) void hsh_nthw_delete(struct hsh_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int hsh_nthw_init(struct hsh_nthw *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c index 007c71d1b0..acdc40b8b1 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c @@ -30,10 +30,7 @@ struct info_nthw *info_nthw_new(void) void info_nthw_delete(struct info_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int info_nthw_init(struct info_nthw *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c index 3909194214..7acdd6d36f 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c @@ -39,10 +39,7 @@ struct km_nthw *km_nthw_new(void) void km_nthw_delete(struct km_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int km_nthw_init(struct km_nthw *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c index 700d868cef..850bb1d22d 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c @@ -30,10 +30,7 @@ struct pdb_nthw *pdb_nthw_new(void) void pdb_nthw_delete(struct pdb_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int pdb_nthw_init(struct pdb_nthw *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c index 47fea6553f..ccc5f46f02 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c @@ -30,10 +30,7 @@ struct qsl_nthw *qsl_nthw_new(void) void qsl_nthw_delete(struct qsl_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int qsl_nthw_init(struct qsl_nthw *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c index 2f141acbde..11f691dcb5 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c @@ -29,10 +29,7 @@ struct rpp_lr_nthw *rpp_lr_nthw_new(void) void rpp_lr_nthw_delete(struct rpp_lr_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int rpp_lr_nthw_init(struct rpp_lr_nthw *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c index e278ad0a41..8b51fa18ad 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c @@ -30,10 +30,7 @@ struct slc_lr_nthw *slc_lr_nthw_new(void) void slc_lr_nthw_delete(struct slc_lr_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int slc_lr_nthw_init(struct slc_lr_nthw *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c index 58e5672462..f7987b9c8c 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c @@ -31,7 +31,6 @@ void tx_cpy_nthw_delete(struct tx_cpy_nthw *p) { if (p) { free(p->m_writers); - memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c index 0ff9af22c7..c21fb5413b 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c @@ -29,10 +29,7 @@ struct tx_ins_nthw *tx_ins_nthw_new(void) void tx_ins_nthw_delete(struct tx_ins_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int tx_ins_nthw_init(struct tx_ins_nthw *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c index 221ea94eed..62bed95be1 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c @@ -29,10 +29,7 @@ struct tx_rpl_nthw *tx_rpl_nthw_new(void) void tx_rpl_nthw_delete(struct tx_rpl_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int tx_rpl_nthw_init(struct tx_rpl_nthw *p, nthw_fpga_t *p_fpga, int n_instance) -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v3 11/11] net/ntnic: remove unnecessary memset 2024-11-14 2:36 ` [PATCH v3 11/11] net/ntnic: remove unnecessary memset Stephen Hemminger @ 2024-11-17 18:26 ` Serhii Iliushyk 0 siblings, 0 replies; 42+ messages in thread From: Serhii Iliushyk @ 2024-11-17 18:26 UTC (permalink / raw) To: Stephen Hemminger, dev; +Cc: Christian Koue Muf >On 14.11.2024, 04:38, "Stephen Hemminger" wrote: > > >Calling memset before free() has no effect and will be flagged >by security parsing tools as a potential bug. None of these data >structures have sensitive information. > > >Signed-off-by: Stephen Hemminger <stephen@networkplumber.org <mailto:stephen@networkplumber.org>> >--- >drivers/net/ntnic/nthw/core/nthw_hif.c | 5 +---- >drivers/net/ntnic/nthw/core/nthw_iic.c | 5 +---- >drivers/net/ntnic/nthw/core/nthw_pcie3.c | 5 +---- >drivers/net/ntnic/nthw/core/nthw_rpf.c | 5 +---- >drivers/net/ntnic/nthw/core/nthw_sdc.c | 5 +---- >drivers/net/ntnic/nthw/core/nthw_si5340.c | 5 +---- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c | 5 +---- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c | 5 +---- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c | 5 +---- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c | 5 +---- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c | 5 +---- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c | 5 +---- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c | 5 +---- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c | 5 +---- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c | 5 +---- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c | 5 +---- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c | 5 +---- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c | 1 - >drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c | 5 +---- >drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c | 5 +---- >20 files changed, 19 insertions(+), 77 deletions(-) > > >diff --git a/drivers/net/ntnic/nthw/core/nthw_hif.c b/drivers/net/ntnic/nthw/core/nthw_hif.c >index 9f699e4f94..d702257d76 100644 >--- a/drivers/net/ntnic/nthw/core/nthw_hif.c >+++ b/drivers/net/ntnic/nthw/core/nthw_hif.c >@@ -23,10 +23,7 @@ nthw_hif_t *nthw_hif_new(void) > > >void nthw_hif_delete(nthw_hif_t *p) >{ >- if (p) { >- memset(p, 0, sizeof(nthw_hif_t)); >- free(p); >- } >+ free(p); >} > > >int nthw_hif_init(nthw_hif_t *p, nthw_fpga_t *p_fpga, int n_instance) >diff --git a/drivers/net/ntnic/nthw/core/nthw_iic.c b/drivers/net/ntnic/nthw/core/nthw_iic.c >index 269754c24a..a98ec659c4 100644 >--- a/drivers/net/ntnic/nthw/core/nthw_iic.c >+++ b/drivers/net/ntnic/nthw/core/nthw_iic.c >@@ -253,10 +253,7 @@ int nthw_iic_init(nthw_iic_t *p, nthw_fpga_t *p_fpga, int n_iic_instance, > > >void nthw_iic_delete(nthw_iic_t *p) >{ >- if (p) { >- memset(p, 0, sizeof(nthw_iic_t)); >- free(p); >- } >+ free(p); >} > > >int nthw_iic_set_retry_params(nthw_iic_t *p, const int n_poll_delay, const int n_bus_ready_retry, >diff --git a/drivers/net/ntnic/nthw/core/nthw_pcie3.c b/drivers/net/ntnic/nthw/core/nthw_pcie3.c >index 5997ebb419..a5833e166c 100644 >--- a/drivers/net/ntnic/nthw/core/nthw_pcie3.c >+++ b/drivers/net/ntnic/nthw/core/nthw_pcie3.c >@@ -24,10 +24,7 @@ nthw_pcie3_t *nthw_pcie3_new(void) > > >void nthw_pcie3_delete(nthw_pcie3_t *p) >{ >- if (p) { >- memset(p, 0, sizeof(nthw_pcie3_t)); >- free(p); >- } >+ free(p); >} > > >int nthw_pcie3_init(nthw_pcie3_t *p, nthw_fpga_t *p_fpga, int n_instance) >diff --git a/drivers/net/ntnic/nthw/core/nthw_rpf.c b/drivers/net/ntnic/nthw/core/nthw_rpf.c >index 1ed4d7b4e0..d5c19e312b 100644 >--- a/drivers/net/ntnic/nthw/core/nthw_rpf.c >+++ b/drivers/net/ntnic/nthw/core/nthw_rpf.c >@@ -22,10 +22,7 @@ nthw_rpf_t *nthw_rpf_new(void) > > >void nthw_rpf_delete(nthw_rpf_t *p) >{ >- if (p) { >- memset(p, 0, sizeof(nthw_rpf_t)); >- free(p); >- } >+ free(p); >} > > >int nthw_rpf_init(nthw_rpf_t *p, nthw_fpga_t *p_fpga, int n_instance) >diff --git a/drivers/net/ntnic/nthw/core/nthw_sdc.c b/drivers/net/ntnic/nthw/core/nthw_sdc.c >index fc73e6957c..e32d87b967 100644 >--- a/drivers/net/ntnic/nthw/core/nthw_sdc.c >+++ b/drivers/net/ntnic/nthw/core/nthw_sdc.c >@@ -22,10 +22,7 @@ nthw_sdc_t *nthw_sdc_new(void) > > >void nthw_sdc_delete(nthw_sdc_t *p) >{ >- if (p) { >- memset(p, 0, sizeof(nthw_sdc_t)); >- free(p); >- } >+ free(p); >} > > >int nthw_sdc_init(nthw_sdc_t *p, nthw_fpga_t *p_fpga, int n_instance) >diff --git a/drivers/net/ntnic/nthw/core/nthw_si5340.c b/drivers/net/ntnic/nthw/core/nthw_si5340.c >index 05cadc0bf4..ceaa58e0f7 100644 >--- a/drivers/net/ntnic/nthw/core/nthw_si5340.c >+++ b/drivers/net/ntnic/nthw/core/nthw_si5340.c >@@ -44,10 +44,7 @@ int nthw_si5340_init(nthw_si5340_t *p, nthw_iic_t *p_nthw_iic, uint8_t n_iic_add > > >void nthw_si5340_delete(nthw_si5340_t *p) >{ >- if (p) { >- memset(p, 0, sizeof(nthw_si5340_t)); >- free(p); >- } >+ free(p); >} > > >/* >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c >index 649a060682..776d4986f7 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c >@@ -25,10 +25,7 @@ struct cat_nthw *cat_nthw_new(void) > > >void cat_nthw_delete(struct cat_nthw *p) >{ >- if (p) { >- memset(p, 0, sizeof(*p)); >- free(p); >- } >+ free(p); >} > > >void cat_nthw_set_debug_mode(struct cat_nthw *p, unsigned int n_debug_mode) >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c >index dc3582c9f5..036a25d06c 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c >@@ -29,10 +29,7 @@ struct csu_nthw *csu_nthw_new(void) > > >void csu_nthw_delete(struct csu_nthw *p) >{ >- if (p) { >- memset(p, 0, sizeof(*p)); >- free(p); >- } >+ free(p); >} > > >int csu_nthw_init(struct csu_nthw *p, nthw_fpga_t *p_fpga, int n_instance) >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c >index 7647949bc7..e016c80bc9 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c >@@ -26,10 +26,7 @@ struct flm_nthw *flm_nthw_new(void) > > >void flm_nthw_delete(struct flm_nthw *p) >{ >- if (p) { >- memset(p, 0, sizeof(*p)); >- free(p); >- } >+ free(p); >} > > >void flm_nthw_set_debug_mode(struct flm_nthw *p, unsigned int n_debug_mode) >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c >index d0e65c071d..02064f1bc6 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c >@@ -29,10 +29,7 @@ struct hfu_nthw *hfu_nthw_new(void) > > >void hfu_nthw_delete(struct hfu_nthw *p) >{ >- if (p) { >- memset(p, 0, sizeof(*p)); >- free(p); >- } >+ free(p); >} > > >int hfu_nthw_init(struct hfu_nthw *p, nthw_fpga_t *p_fpga, int n_instance) >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c >index caf84790cd..10f982ea76 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c >@@ -30,10 +30,7 @@ struct hsh_nthw *hsh_nthw_new(void) > > >void hsh_nthw_delete(struct hsh_nthw *p) >{ >- if (p) { >- memset(p, 0, sizeof(*p)); >- free(p); >- } >+ free(p); >} > > >int hsh_nthw_init(struct hsh_nthw *p, nthw_fpga_t *p_fpga, int n_instance) >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c >index 007c71d1b0..acdc40b8b1 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c >@@ -30,10 +30,7 @@ struct info_nthw *info_nthw_new(void) > > >void info_nthw_delete(struct info_nthw *p) >{ >- if (p) { >- memset(p, 0, sizeof(*p)); >- free(p); >- } >+ free(p); >} > > >int info_nthw_init(struct info_nthw *p, nthw_fpga_t *p_fpga, int n_instance) >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c >index 3909194214..7acdd6d36f 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c >@@ -39,10 +39,7 @@ struct km_nthw *km_nthw_new(void) > > >void km_nthw_delete(struct km_nthw *p) >{ >- if (p) { >- memset(p, 0, sizeof(*p)); >- free(p); >- } >+ free(p); >} > > >int km_nthw_init(struct km_nthw *p, nthw_fpga_t *p_fpga, int n_instance) >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c >index 700d868cef..850bb1d22d 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c >@@ -30,10 +30,7 @@ struct pdb_nthw *pdb_nthw_new(void) > > >void pdb_nthw_delete(struct pdb_nthw *p) >{ >- if (p) { >- memset(p, 0, sizeof(*p)); >- free(p); >- } >+ free(p); >} > > >int pdb_nthw_init(struct pdb_nthw *p, nthw_fpga_t *p_fpga, int n_instance) >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c >index 47fea6553f..ccc5f46f02 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c >@@ -30,10 +30,7 @@ struct qsl_nthw *qsl_nthw_new(void) > > >void qsl_nthw_delete(struct qsl_nthw *p) >{ >- if (p) { >- memset(p, 0, sizeof(*p)); >- free(p); >- } >+ free(p); >} > > >int qsl_nthw_init(struct qsl_nthw *p, nthw_fpga_t *p_fpga, int n_instance) >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c >index 2f141acbde..11f691dcb5 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c >@@ -29,10 +29,7 @@ struct rpp_lr_nthw *rpp_lr_nthw_new(void) > > >void rpp_lr_nthw_delete(struct rpp_lr_nthw *p) >{ >- if (p) { >- memset(p, 0, sizeof(*p)); >- free(p); >- } >+ free(p); >} > > >int rpp_lr_nthw_init(struct rpp_lr_nthw *p, nthw_fpga_t *p_fpga, int n_instance) >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c >index e278ad0a41..8b51fa18ad 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c >@@ -30,10 +30,7 @@ struct slc_lr_nthw *slc_lr_nthw_new(void) > > >void slc_lr_nthw_delete(struct slc_lr_nthw *p) >{ >- if (p) { >- memset(p, 0, sizeof(*p)); >- free(p); >- } >+ free(p); >} > > >int slc_lr_nthw_init(struct slc_lr_nthw *p, nthw_fpga_t *p_fpga, int n_instance) >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c >index 58e5672462..f7987b9c8c 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c >@@ -31,7 +31,6 @@ void tx_cpy_nthw_delete(struct tx_cpy_nthw *p) >{ >if (p) { >free(p->m_writers); >- memset(p, 0, sizeof(*p)); >free(p); >} >} >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c >index 0ff9af22c7..c21fb5413b 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c >@@ -29,10 +29,7 @@ struct tx_ins_nthw *tx_ins_nthw_new(void) > > >void tx_ins_nthw_delete(struct tx_ins_nthw *p) >{ >- if (p) { >- memset(p, 0, sizeof(*p)); >- free(p); >- } >+ free(p); >} > > >int tx_ins_nthw_init(struct tx_ins_nthw *p, nthw_fpga_t *p_fpga, int n_instance) >diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c >index 221ea94eed..62bed95be1 100644 >--- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c >+++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c >@@ -29,10 +29,7 @@ struct tx_rpl_nthw *tx_rpl_nthw_new(void) > > >void tx_rpl_nthw_delete(struct tx_rpl_nthw *p) >{ >- if (p) { >- memset(p, 0, sizeof(*p)); >- free(p); >- } >+ free(p); >} > > >int tx_rpl_nthw_init(struct tx_rpl_nthw *p, nthw_fpga_t *p_fpga, int n_instance) >-- >2.45.2 > > Hi Stephen, Thanks a lot for noticing and fixing this issue. We appreciate it. Reviewed-by: Serhii Iliushyk <sil-plv@napatech.com> ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 00/12] memset security fixes 2024-11-14 1:10 [PATCH 0/3] introduce rte_memset_sensative Stephen Hemminger ` (4 preceding siblings ...) 2024-11-14 2:35 ` [PATCH v3 00/11] memset security handling Stephen Hemminger @ 2024-11-14 18:43 ` Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 01/12] eal: introduce new secure memory fill Stephen Hemminger ` (11 more replies) 5 siblings, 12 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 18:43 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger This series handles memset related bugs indentified by PVS Studio. The root cause is that Gcc and other compilers are free to optimize away memset called before free. Most of the places memset was being used like this were bogus; probably some developer debug habit, and can be safely removed. v4 - review feedback - remove more Napatech NIC unnecessary memset Stephen Hemminger (12): eal: introduce new secure memory fill eal: add new secure free function crypto/qat: force zero of keys crypto/qat: fix size calculation for memset crypto/qat: use secure memset bus/uacce: remove memset before free compress/octeontx: remove unnecessary memset test: remove unneeded memset net/ntnic: remove unnecessary void cast net/ntnic: check result of malloc net/ntnic: remove unnecessary memset devtools/cocci: add script to find problematic memset app/test/test_cmdline_cirbuf.c | 2 -- devtools/cocci/memset_free.cocci | 20 +++++++++++++ drivers/bus/uacce/uacce.c | 1 - drivers/compress/octeontx/otx_zip.c | 1 - drivers/compress/octeontx/otx_zip_pmd.c | 2 -- drivers/crypto/qat/qat_asym.c | 5 +--- drivers/crypto/qat/qat_sym_session.c | 27 +++++++++-------- drivers/net/ntnic/nim/i2c_nim.c | 2 +- drivers/net/ntnic/nthw/core/nthw_hif.c | 5 +--- drivers/net/ntnic/nthw/core/nthw_iic.c | 5 +--- drivers/net/ntnic/nthw/core/nthw_pcie3.c | 5 +--- drivers/net/ntnic/nthw/core/nthw_rpf.c | 5 +--- drivers/net/ntnic/nthw/core/nthw_sdc.c | 5 +--- drivers/net/ntnic/nthw/core/nthw_si5340.c | 5 +--- .../ntnic/nthw/flow_filter/flow_nthw_cat.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_csu.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_flm.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_hfu.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_hsh.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_ifr.c | 2 +- .../ntnic/nthw/flow_filter/flow_nthw_info.c | 7 ++--- .../net/ntnic/nthw/flow_filter/flow_nthw_km.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_pdb.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_qsl.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_slc_lr.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c | 3 +- .../ntnic/nthw/flow_filter/flow_nthw_tx_ins.c | 7 ++--- .../ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c | 7 ++--- .../net/ntnic/nthw/model/nthw_fpga_model.c | 1 - drivers/net/ntnic/nthw/nthw_rac.c | 4 ++- drivers/net/ntnic/ntnic_ethdev.c | 2 +- lib/eal/common/rte_malloc.c | 30 +++++++++++++++---- lib/eal/include/rte_malloc.h | 18 +++++++++++ lib/eal/include/rte_string_fns.h | 27 +++++++++++++++++ lib/eal/version.map | 3 ++ 36 files changed, 146 insertions(+), 125 deletions(-) create mode 100644 devtools/cocci/memset_free.cocci -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 01/12] eal: introduce new secure memory fill 2024-11-14 18:43 ` [PATCH v4 00/12] memset security fixes Stephen Hemminger @ 2024-11-14 18:43 ` Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 02/12] eal: add new secure free function Stephen Hemminger ` (10 subsequent siblings) 11 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 18:43 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Tyler Retzlaff When memset() is used before a release function such as free, the compiler if allowed to optimize the memset away under the as-if rules. This is normally ok, but in certain cases such as passwords or security keys it is problematic. Introduce a DPDK wrapper which is equivalent to the C++ memset_s function. Naming chosen to be similar to kernel. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- lib/eal/include/rte_string_fns.h | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/lib/eal/include/rte_string_fns.h b/lib/eal/include/rte_string_fns.h index 702bd81251..4874703957 100644 --- a/lib/eal/include/rte_string_fns.h +++ b/lib/eal/include/rte_string_fns.h @@ -15,6 +15,7 @@ #include <stdio.h> #include <string.h> +#include <rte_atomic.h> #include <rte_common.h> #include <rte_compat.h> @@ -149,6 +150,32 @@ rte_str_skip_leading_spaces(const char *src) return p; } +/** + * @warning + * @b EXPERIMENTAL: this API may change without prior notice. + * + * Fill memory with constant byte but can not be optimized away. + * Use as a replacement for memset() for sensitive information. + * + * @param dst + * target buffer + * @param ch + * byte to fill + * @param sz + * number of bytes to fill + * + * @return + * like memset() returns a pointer th the memory area dst. + */ +__rte_experimental +static inline void * +rte_memset_sensitive(void *dst, int ch, size_t sz) +{ + void *ret = memset(dst, ch, sz); + rte_compiler_barrier(); + return ret; +} + #ifdef __cplusplus } #endif -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 02/12] eal: add new secure free function 2024-11-14 18:43 ` [PATCH v4 00/12] memset security fixes Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 01/12] eal: introduce new secure memory fill Stephen Hemminger @ 2024-11-14 18:43 ` Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 03/12] crypto/qat: force zero of keys Stephen Hemminger ` (9 subsequent siblings) 11 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 18:43 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Anatoly Burakov, Tyler Retzlaff Although internally rte_free does poison the buffer in most cases, it is useful to have function that explicitly does this to avoid any security issues. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- lib/eal/common/rte_malloc.c | 30 ++++++++++++++++++++++++------ lib/eal/include/rte_malloc.h | 18 ++++++++++++++++++ lib/eal/version.map | 3 +++ 3 files changed, 45 insertions(+), 6 deletions(-) diff --git a/lib/eal/common/rte_malloc.c b/lib/eal/common/rte_malloc.c index 3eed4d4be6..c9e0f4724f 100644 --- a/lib/eal/common/rte_malloc.c +++ b/lib/eal/common/rte_malloc.c @@ -15,6 +15,7 @@ #include <rte_eal_memconfig.h> #include <rte_common.h> #include <rte_spinlock.h> +#include <rte_string_fns.h> #include <eal_trace_internal.h> @@ -27,27 +28,44 @@ /* Free the memory space back to heap */ -static void -mem_free(void *addr, const bool trace_ena) +static inline void +mem_free(void *addr, const bool trace_ena, bool zero) { + struct malloc_elem *elem; + if (trace_ena) rte_eal_trace_mem_free(addr); - if (addr == NULL) return; - if (malloc_heap_free(malloc_elem_from_data(addr)) < 0) + if (addr == NULL) + return; + + elem = malloc_elem_from_data(addr); + if (zero) { + size_t data_len = elem->size - MALLOC_ELEM_OVERHEAD; + + rte_memset_sensitive(addr, 0, data_len); + } + + if (malloc_heap_free(elem) < 0) EAL_LOG(ERR, "Error: Invalid memory"); } void rte_free(void *addr) { - mem_free(addr, true); + mem_free(addr, true, false); +} + +void +rte_free_sensitive(void *addr) +{ + mem_free(addr, true, true); } void eal_free_no_trace(void *addr) { - mem_free(addr, false); + mem_free(addr, false, false); } static void * diff --git a/lib/eal/include/rte_malloc.h b/lib/eal/include/rte_malloc.h index c8836de67c..d472ebb7db 100644 --- a/lib/eal/include/rte_malloc.h +++ b/lib/eal/include/rte_malloc.h @@ -51,6 +51,24 @@ struct rte_malloc_socket_stats { void rte_free(void *ptr); + +/** + * Frees the memory space pointed to by the provided pointer + * and guarantees it will be zero'd before reuse. + * + * This pointer must have been returned by a previous call to + * rte_malloc(), rte_zmalloc(), rte_calloc() or rte_realloc(). The behaviour of + * rte_free() is undefined if the pointer does not match this requirement. + * + * If the pointer is NULL, the function does nothing. + * + * @param ptr + * The pointer to memory to be freed. + */ +__rte_experimental +void +rte_free_sensitive(void *ptr); + /** * This function allocates memory from the huge-page area of memory. The memory * is not cleared. In NUMA systems, the memory allocated resides on the same diff --git a/lib/eal/version.map b/lib/eal/version.map index a20c713eb1..fa67ff44d5 100644 --- a/lib/eal/version.map +++ b/lib/eal/version.map @@ -398,6 +398,9 @@ EXPERIMENTAL { # added in 24.11 rte_bitset_to_str; rte_lcore_var_alloc; + + # added in 25.03 + rte_free_sensitive; }; INTERNAL { -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 03/12] crypto/qat: force zero of keys 2024-11-14 18:43 ` [PATCH v4 00/12] memset security fixes Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 01/12] eal: introduce new secure memory fill Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 02/12] eal: add new secure free function Stephen Hemminger @ 2024-11-14 18:43 ` Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 04/12] crypto/qat: fix size calculation for memset Stephen Hemminger ` (8 subsequent siblings) 11 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 18:43 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Bruce Richardson, Kai Ji Just doing memset() on keys is not enough, compiler can optimize it away. Need something with a barrier. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> Acked-by: Bruce Richardson <bruce.richardson@intel.com> --- drivers/crypto/qat/qat_sym_session.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/crypto/qat/qat_sym_session.c b/drivers/crypto/qat/qat_sym_session.c index 50d687fd37..2fce8fcb16 100644 --- a/drivers/crypto/qat/qat_sym_session.c +++ b/drivers/crypto/qat/qat_sym_session.c @@ -26,6 +26,7 @@ #include <rte_crypto_sym.h> #include <rte_security_driver.h> #include <rte_ether.h> +#include <rte_string_fns.h> #include "qat_logs.h" #include "qat_sym_session.h" @@ -1633,7 +1634,7 @@ static int qat_sym_do_precomputes(enum icp_qat_hw_auth_algo hash_alg, aes_cmac_key_derive(k0, k1); aes_cmac_key_derive(k1, k2); - memset(k0, 0, ICP_QAT_HW_AES_128_KEY_SZ); + rte_memset_sensitive(k0, 0, ICP_QAT_HW_AES_128_KEY_SZ); *p_state_len = ICP_QAT_HW_AES_XCBC_MAC_STATE2_SZ; rte_free(in); goto out; @@ -1668,7 +1669,7 @@ static int qat_sym_do_precomputes(enum icp_qat_hw_auth_algo hash_alg, &enc_key) != 0) { rte_free(in - (x * ICP_QAT_HW_AES_XCBC_MAC_KEY_SZ)); - memset(out - + rte_memset_sensitive(out - (x * ICP_QAT_HW_AES_XCBC_MAC_KEY_SZ), 0, ICP_QAT_HW_AES_XCBC_MAC_STATE2_SZ); return -EFAULT; @@ -1698,7 +1699,7 @@ static int qat_sym_do_precomputes(enum icp_qat_hw_auth_algo hash_alg, return -ENOMEM; } - memset(in, 0, ICP_QAT_HW_GALOIS_H_SZ); + rte_memset_sensitive(in, 0, ICP_QAT_HW_GALOIS_H_SZ); if (AES_set_encrypt_key(auth_key, auth_keylen << 3, &enc_key) != 0) { return -EFAULT; @@ -1757,8 +1758,8 @@ static int qat_sym_do_precomputes(enum icp_qat_hw_auth_algo hash_alg, } /* don't leave data lying around */ - memset(ipad, 0, block_size); - memset(opad, 0, block_size); + rte_memset_sensitive(ipad, 0, block_size); + rte_memset_sensitive(opad, 0, block_size); out: return 0; } @@ -2006,8 +2007,8 @@ static int qat_sym_do_precomputes_ipsec_mb(enum icp_qat_hw_auth_algo hash_alg, out: /* don't leave data lying around */ - memset(ipad, 0, block_size); - memset(opad, 0, block_size); + rte_memset_sensitive(ipad, 0, block_size); + rte_memset_sensitive(opad, 0, block_size); free_mb_mgr(m); return ret; } @@ -3232,7 +3233,7 @@ qat_security_session_destroy(void *dev __rte_unused, if (s->mb_mgr) free_mb_mgr(s->mb_mgr); #endif - memset(s, 0, qat_sym_session_get_private_size(dev)); + rte_memset_sensitive(s, 0, qat_sym_session_get_private_size(dev)); } return 0; -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 04/12] crypto/qat: fix size calculation for memset 2024-11-14 18:43 ` [PATCH v4 00/12] memset security fixes Stephen Hemminger ` (2 preceding siblings ...) 2024-11-14 18:43 ` [PATCH v4 03/12] crypto/qat: force zero of keys Stephen Hemminger @ 2024-11-14 18:43 ` Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 05/12] crypto/qat: use secure memset Stephen Hemminger ` (7 subsequent siblings) 11 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 18:43 UTC (permalink / raw) To: dev Cc: Stephen Hemminger, stable, Bruce Richardson, Kai Ji, Fan Zhang, Ciara Power The memset was always doing 0 bytes since size computed later. Link: https://pvs-studio.com/en/blog/posts/cpp/1179/ Fixes: 3a80d7fb2ecd ("crypto/qat: support SHA3 plain hash") Cc: stable@dpdk.org Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> Acked-by: Bruce Richardson <bruce.richardson@intel.com> --- drivers/crypto/qat/qat_sym_session.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/crypto/qat/qat_sym_session.c b/drivers/crypto/qat/qat_sym_session.c index 2fce8fcb16..6f3eacaf43 100644 --- a/drivers/crypto/qat/qat_sym_session.c +++ b/drivers/crypto/qat/qat_sym_session.c @@ -2347,7 +2347,7 @@ int qat_sym_cd_cipher_set(struct qat_sym_session *cdesc, return 0; } -int qat_sym_cd_auth_set(struct qat_sym_session *cdesc, +static int qat_sym_cd_auth_set(struct qat_sym_session *cdesc, const uint8_t *authkey, uint32_t authkeylen, uint32_t aad_length, @@ -2620,27 +2620,27 @@ int qat_sym_cd_auth_set(struct qat_sym_session *cdesc, break; case ICP_QAT_HW_AUTH_ALGO_SHA3_224: /* Plain SHA3-224 */ - memset(cdesc->cd_cur_ptr, 0, state1_size); state1_size = qat_hash_get_state1_size( cdesc->qat_hash_alg); + memset(cdesc->cd_cur_ptr, 0, state1_size); break; case ICP_QAT_HW_AUTH_ALGO_SHA3_256: /* Plain SHA3-256 */ - memset(cdesc->cd_cur_ptr, 0, state1_size); state1_size = qat_hash_get_state1_size( cdesc->qat_hash_alg); + memset(cdesc->cd_cur_ptr, 0, state1_size); break; case ICP_QAT_HW_AUTH_ALGO_SHA3_384: /* Plain SHA3-384 */ - memset(cdesc->cd_cur_ptr, 0, state1_size); state1_size = qat_hash_get_state1_size( cdesc->qat_hash_alg); + memset(cdesc->cd_cur_ptr, 0, state1_size); break; case ICP_QAT_HW_AUTH_ALGO_SHA3_512: /* Plain SHA3-512 */ - memset(cdesc->cd_cur_ptr, 0, state1_size); state1_size = qat_hash_get_state1_size( cdesc->qat_hash_alg); + memset(cdesc->cd_cur_ptr, 0, state1_size); break; case ICP_QAT_HW_AUTH_ALGO_AES_XCBC_MAC: state1_size = ICP_QAT_HW_AES_XCBC_MAC_STATE1_SZ; -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 05/12] crypto/qat: use secure memset 2024-11-14 18:43 ` [PATCH v4 00/12] memset security fixes Stephen Hemminger ` (3 preceding siblings ...) 2024-11-14 18:43 ` [PATCH v4 04/12] crypto/qat: fix size calculation for memset Stephen Hemminger @ 2024-11-14 18:43 ` Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 06/12] bus/uacce: remove memset before free Stephen Hemminger ` (6 subsequent siblings) 11 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 18:43 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Bruce Richardson, Kai Ji Regular memset maybe removed by compiler if done before a free function. Use new rte_free_sensitive instead. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> Acked-by: Bruce Richardson <bruce.richardson@intel.com> --- drivers/crypto/qat/qat_asym.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/drivers/crypto/qat/qat_asym.c b/drivers/crypto/qat/qat_asym.c index f5b56b2f71..d8a1406819 100644 --- a/drivers/crypto/qat/qat_asym.c +++ b/drivers/crypto/qat/qat_asym.c @@ -102,10 +102,7 @@ static const struct rte_driver cryptodev_qat_asym_driver = { curve.p.data, curve.bytesize) #define PARAM_CLR(what) \ - do { \ - memset(what.data, 0, what.length); \ - rte_free(what.data); \ - } while (0) + rte_free_sensitive(what.data) static void request_init(struct icp_qat_fw_pke_request *qat_req) -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 06/12] bus/uacce: remove memset before free 2024-11-14 18:43 ` [PATCH v4 00/12] memset security fixes Stephen Hemminger ` (4 preceding siblings ...) 2024-11-14 18:43 ` [PATCH v4 05/12] crypto/qat: use secure memset Stephen Hemminger @ 2024-11-14 18:43 ` Stephen Hemminger 2024-11-15 6:04 ` fengchengwen 2024-11-14 18:43 ` [PATCH v4 07/12] compress/octeontx: remove unnecessary memset Stephen Hemminger ` (5 subsequent siblings) 11 siblings, 1 reply; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 18:43 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Chengwen Feng Doing memset before free maybe removed by compiler, and is flagged by security scanning tools as potential problem. In this case the memset is unnecessary. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/bus/uacce/uacce.c | 1 - 1 file changed, 1 deletion(-) diff --git a/drivers/bus/uacce/uacce.c b/drivers/bus/uacce/uacce.c index c1529c38c0..35c1027245 100644 --- a/drivers/bus/uacce/uacce.c +++ b/drivers/bus/uacce/uacce.c @@ -454,7 +454,6 @@ uacce_cleanup(void) dev->device.driver = NULL; free: - memset(dev, 0, sizeof(*dev)); free(dev); } -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH v4 06/12] bus/uacce: remove memset before free 2024-11-14 18:43 ` [PATCH v4 06/12] bus/uacce: remove memset before free Stephen Hemminger @ 2024-11-15 6:04 ` fengchengwen 0 siblings, 0 replies; 42+ messages in thread From: fengchengwen @ 2024-11-15 6:04 UTC (permalink / raw) To: Stephen Hemminger, dev There is no sensitive information in dev, so Acked-by: Chengwen Feng <fengchengwen@huawei.com> On 2024/11/15 2:43, Stephen Hemminger wrote: > Doing memset before free maybe removed by compiler, and > is flagged by security scanning tools as potential problem. > In this case the memset is unnecessary. > > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> > --- > drivers/bus/uacce/uacce.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/bus/uacce/uacce.c b/drivers/bus/uacce/uacce.c > index c1529c38c0..35c1027245 100644 > --- a/drivers/bus/uacce/uacce.c > +++ b/drivers/bus/uacce/uacce.c > @@ -454,7 +454,6 @@ uacce_cleanup(void) > dev->device.driver = NULL; > > free: > - memset(dev, 0, sizeof(*dev)); > free(dev); > } > ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 07/12] compress/octeontx: remove unnecessary memset 2024-11-14 18:43 ` [PATCH v4 00/12] memset security fixes Stephen Hemminger ` (5 preceding siblings ...) 2024-11-14 18:43 ` [PATCH v4 06/12] bus/uacce: remove memset before free Stephen Hemminger @ 2024-11-14 18:43 ` Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 08/12] test: remove unneeded memset Stephen Hemminger ` (4 subsequent siblings) 11 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 18:43 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Bruce Richardson, Ashish Gupta, Fan Zhang Calling memset before rte_free not necessary, and could be removed by the compiler. In this case, the data is not security sensitive so the memset can be removed. Some security scanning tools will flag this. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> Acked-by: Bruce Richardson <bruce.richardson@intel.com> --- drivers/compress/octeontx/otx_zip.c | 1 - drivers/compress/octeontx/otx_zip_pmd.c | 2 -- 2 files changed, 3 deletions(-) diff --git a/drivers/compress/octeontx/otx_zip.c b/drivers/compress/octeontx/otx_zip.c index 11471dcbb4..331d2d9475 100644 --- a/drivers/compress/octeontx/otx_zip.c +++ b/drivers/compress/octeontx/otx_zip.c @@ -81,7 +81,6 @@ zipvf_q_term(struct zipvf_qp *qp) struct zip_vf *vf = qp->vf; if (cmdq->va != NULL) { - memset(cmdq->va, 0, ZIP_MAX_CMDQ_SIZE); rte_free(cmdq->va); } diff --git a/drivers/compress/octeontx/otx_zip_pmd.c b/drivers/compress/octeontx/otx_zip_pmd.c index c8f456b319..74e3e942ad 100644 --- a/drivers/compress/octeontx/otx_zip_pmd.c +++ b/drivers/compress/octeontx/otx_zip_pmd.c @@ -479,8 +479,6 @@ zip_pmd_stream_free(struct rte_compressdev *dev, void *stream) (void *)&(z_stream->bufs[0]), (MAX_BUFS_PER_STREAM * ZIP_BURST_SIZE)); - /* Zero out the whole structure */ - memset(stream, 0, sizeof(struct zip_stream)); rte_free(stream); return 0; -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 08/12] test: remove unneeded memset 2024-11-14 18:43 ` [PATCH v4 00/12] memset security fixes Stephen Hemminger ` (6 preceding siblings ...) 2024-11-14 18:43 ` [PATCH v4 07/12] compress/octeontx: remove unnecessary memset Stephen Hemminger @ 2024-11-14 18:43 ` Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 09/12] net/ntnic: remove unnecessary void cast Stephen Hemminger ` (3 subsequent siblings) 11 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 18:43 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Bruce Richardson Since tmp is not used later in the function, this memset is unnecessary. Even though this is harmless, it causes tools that look for security issues around memset to flag this a bug. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> Acked-by: Bruce Richardson <bruce.richardson@intel.com> --- app/test/test_cmdline_cirbuf.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/app/test/test_cmdline_cirbuf.c b/app/test/test_cmdline_cirbuf.c index 8ac326cb02..1be357abf6 100644 --- a/app/test/test_cmdline_cirbuf.c +++ b/app/test/test_cmdline_cirbuf.c @@ -281,8 +281,6 @@ test_cirbuf_string_add_del_reverse(void) printf("Error: buffer should have been empty!\n"); return -1; } - /* clear tmp buffer */ - memset(tmp, 0, sizeof(tmp)); /* * reinitialize circular buffer -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 09/12] net/ntnic: remove unnecessary void cast 2024-11-14 18:43 ` [PATCH v4 00/12] memset security fixes Stephen Hemminger ` (7 preceding siblings ...) 2024-11-14 18:43 ` [PATCH v4 08/12] test: remove unneeded memset Stephen Hemminger @ 2024-11-14 18:43 ` Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 10/12] net/ntnic: check result of malloc Stephen Hemminger ` (2 subsequent siblings) 11 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 18:43 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Christian Koue Muf, Serhii Iliushyk There is no need to cast memset to void. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/net/ntnic/nim/i2c_nim.c | 2 +- drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_ifr.c | 2 +- drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c | 4 ++-- drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c | 4 ++-- drivers/net/ntnic/ntnic_ethdev.c | 2 +- 17 files changed, 31 insertions(+), 31 deletions(-) diff --git a/drivers/net/ntnic/nim/i2c_nim.c b/drivers/net/ntnic/nim/i2c_nim.c index e6f7755ded..f9fec5f767 100644 --- a/drivers/net/ntnic/nim/i2c_nim.c +++ b/drivers/net/ntnic/nim/i2c_nim.c @@ -292,7 +292,7 @@ static int qsfp_nim_state_build(nim_i2c_ctx_t *ctx, sfp_nim_state_t *state) assert(ctx && state); assert(ctx->nim_id != NT_NIM_UNKNOWN && "Nim is not initialized"); - (void)memset(state, 0, sizeof(*state)); + memset(state, 0, sizeof(*state)); switch (ctx->nim_id) { case 12U: diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c index d3213593e1..649a060682 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c @@ -18,7 +18,7 @@ struct cat_nthw *cat_nthw_new(void) struct cat_nthw *p = malloc(sizeof(struct cat_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -26,7 +26,7 @@ struct cat_nthw *cat_nthw_new(void) void cat_nthw_delete(struct cat_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c index c1b73179a8..dc3582c9f5 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c @@ -22,7 +22,7 @@ struct csu_nthw *csu_nthw_new(void) struct csu_nthw *p = malloc(sizeof(struct csu_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -30,7 +30,7 @@ struct csu_nthw *csu_nthw_new(void) void csu_nthw_delete(struct csu_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c index 8855978349..7647949bc7 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c @@ -19,7 +19,7 @@ struct flm_nthw *flm_nthw_new(void) struct flm_nthw *p = malloc(sizeof(struct flm_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -27,7 +27,7 @@ struct flm_nthw *flm_nthw_new(void) void flm_nthw_delete(struct flm_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c index 4a8b17101d..d0e65c071d 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c @@ -22,7 +22,7 @@ struct hfu_nthw *hfu_nthw_new(void) struct hfu_nthw *p = malloc(sizeof(struct hfu_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -30,7 +30,7 @@ struct hfu_nthw *hfu_nthw_new(void) void hfu_nthw_delete(struct hfu_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c index 80ead2729a..caf84790cd 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c @@ -23,7 +23,7 @@ struct hsh_nthw *hsh_nthw_new(void) struct hsh_nthw *p = malloc(sizeof(struct hsh_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -31,7 +31,7 @@ struct hsh_nthw *hsh_nthw_new(void) void hsh_nthw_delete(struct hsh_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_ifr.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_ifr.c index 11b6b5e5b8..df2bd1cc5c 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_ifr.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_ifr.c @@ -22,7 +22,7 @@ struct ifr_nthw *ifr_nthw_new(void) struct ifr_nthw *p = malloc(sizeof(struct ifr_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c index 8e0b24dd9a..007c71d1b0 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c @@ -23,7 +23,7 @@ struct info_nthw *info_nthw_new(void) struct info_nthw *p = malloc(sizeof(struct info_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -31,7 +31,7 @@ struct info_nthw *info_nthw_new(void) void info_nthw_delete(struct info_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c index edc8f58759..3909194214 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c @@ -32,7 +32,7 @@ struct km_nthw *km_nthw_new(void) struct km_nthw *p = malloc(sizeof(struct km_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -40,7 +40,7 @@ struct km_nthw *km_nthw_new(void) void km_nthw_delete(struct km_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c index 4a9713965b..700d868cef 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c @@ -23,7 +23,7 @@ struct pdb_nthw *pdb_nthw_new(void) struct pdb_nthw *p = malloc(sizeof(struct pdb_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -31,7 +31,7 @@ struct pdb_nthw *pdb_nthw_new(void) void pdb_nthw_delete(struct pdb_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c index c887fe25e2..47fea6553f 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c @@ -23,7 +23,7 @@ struct qsl_nthw *qsl_nthw_new(void) struct qsl_nthw *p = malloc(sizeof(struct qsl_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -31,7 +31,7 @@ struct qsl_nthw *qsl_nthw_new(void) void qsl_nthw_delete(struct qsl_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c index 28c7a05fe2..2f141acbde 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c @@ -22,7 +22,7 @@ struct rpp_lr_nthw *rpp_lr_nthw_new(void) struct rpp_lr_nthw *p = malloc(sizeof(struct rpp_lr_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -30,7 +30,7 @@ struct rpp_lr_nthw *rpp_lr_nthw_new(void) void rpp_lr_nthw_delete(struct rpp_lr_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c index e062a700eb..e278ad0a41 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c @@ -23,7 +23,7 @@ struct slc_lr_nthw *slc_lr_nthw_new(void) struct slc_lr_nthw *p = malloc(sizeof(struct slc_lr_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -31,7 +31,7 @@ struct slc_lr_nthw *slc_lr_nthw_new(void) void slc_lr_nthw_delete(struct slc_lr_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c index ee85a1c61b..58e5672462 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c @@ -22,7 +22,7 @@ struct tx_cpy_nthw *tx_cpy_nthw_new(void) struct tx_cpy_nthw *p = malloc(sizeof(struct tx_cpy_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -31,7 +31,7 @@ void tx_cpy_nthw_delete(struct tx_cpy_nthw *p) { if (p) { free(p->m_writers); - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c index 47af44945f..0ff9af22c7 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c @@ -22,7 +22,7 @@ struct tx_ins_nthw *tx_ins_nthw_new(void) struct tx_ins_nthw *p = malloc(sizeof(struct tx_ins_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -30,7 +30,7 @@ struct tx_ins_nthw *tx_ins_nthw_new(void) void tx_ins_nthw_delete(struct tx_ins_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c index c40857db0f..221ea94eed 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c @@ -22,7 +22,7 @@ struct tx_rpl_nthw *tx_rpl_nthw_new(void) struct tx_rpl_nthw *p = malloc(sizeof(struct tx_rpl_nthw)); if (p) - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); return p; } @@ -30,7 +30,7 @@ struct tx_rpl_nthw *tx_rpl_nthw_new(void) void tx_rpl_nthw_delete(struct tx_rpl_nthw *p) { if (p) { - (void)memset(p, 0, sizeof(*p)); + memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/ntnic_ethdev.c b/drivers/net/ntnic/ntnic_ethdev.c index 2a2643a106..9f3e9440dd 100644 --- a/drivers/net/ntnic/ntnic_ethdev.c +++ b/drivers/net/ntnic/ntnic_ethdev.c @@ -1251,7 +1251,7 @@ eth_set_mc_addr_list(struct rte_eth_dev *eth_dev, mc_addrs[i] = mc_addr_set[i]; else - (void)memset(&mc_addrs[i], 0, sizeof(mc_addrs[i])); + memset(&mc_addrs[i], 0, sizeof(mc_addrs[i])); return 0; } -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 10/12] net/ntnic: check result of malloc 2024-11-14 18:43 ` [PATCH v4 00/12] memset security fixes Stephen Hemminger ` (8 preceding siblings ...) 2024-11-14 18:43 ` [PATCH v4 09/12] net/ntnic: remove unnecessary void cast Stephen Hemminger @ 2024-11-14 18:43 ` Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 11/12] net/ntnic: remove unnecessary memset Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 12/12] devtools/cocci: add script to find problematic memset Stephen Hemminger 11 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 18:43 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Christian Koue Muf, Serhii Iliushyk Need to check the result of malloc() before calling memset. This is only place in this driver that forgot, other code does check. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/net/ntnic/nthw/nthw_rac.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/ntnic/nthw/nthw_rac.c b/drivers/net/ntnic/nthw/nthw_rac.c index ca6aba6db2..f275e64da3 100644 --- a/drivers/net/ntnic/nthw/nthw_rac.c +++ b/drivers/net/ntnic/nthw/nthw_rac.c @@ -31,7 +31,9 @@ nthw_rac_t *nthw_rac_new(void) { nthw_rac_t *p = malloc(sizeof(nthw_rac_t)); - memset(p, 0, sizeof(nthw_rac_t)); + + if (p) + memset(p, 0, sizeof(nthw_rac_t)); return p; } -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 11/12] net/ntnic: remove unnecessary memset 2024-11-14 18:43 ` [PATCH v4 00/12] memset security fixes Stephen Hemminger ` (9 preceding siblings ...) 2024-11-14 18:43 ` [PATCH v4 10/12] net/ntnic: check result of malloc Stephen Hemminger @ 2024-11-14 18:43 ` Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 12/12] devtools/cocci: add script to find problematic memset Stephen Hemminger 11 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 18:43 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger, Christian Koue Muf, Serhii Iliushyk Calling memset before free() has no effect and will be flagged by security parsing tools as a potential bug. None of these data structures have sensitive information. Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- drivers/net/ntnic/nthw/core/nthw_hif.c | 5 +---- drivers/net/ntnic/nthw/core/nthw_iic.c | 5 +---- drivers/net/ntnic/nthw/core/nthw_pcie3.c | 5 +---- drivers/net/ntnic/nthw/core/nthw_rpf.c | 5 +---- drivers/net/ntnic/nthw/core/nthw_sdc.c | 5 +---- drivers/net/ntnic/nthw/core/nthw_si5340.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c | 1 - drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c | 5 +---- drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c | 5 +---- drivers/net/ntnic/nthw/model/nthw_fpga_model.c | 1 - 21 files changed, 19 insertions(+), 78 deletions(-) diff --git a/drivers/net/ntnic/nthw/core/nthw_hif.c b/drivers/net/ntnic/nthw/core/nthw_hif.c index 9f699e4f94..d702257d76 100644 --- a/drivers/net/ntnic/nthw/core/nthw_hif.c +++ b/drivers/net/ntnic/nthw/core/nthw_hif.c @@ -23,10 +23,7 @@ nthw_hif_t *nthw_hif_new(void) void nthw_hif_delete(nthw_hif_t *p) { - if (p) { - memset(p, 0, sizeof(nthw_hif_t)); - free(p); - } + free(p); } int nthw_hif_init(nthw_hif_t *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/core/nthw_iic.c b/drivers/net/ntnic/nthw/core/nthw_iic.c index 269754c24a..a98ec659c4 100644 --- a/drivers/net/ntnic/nthw/core/nthw_iic.c +++ b/drivers/net/ntnic/nthw/core/nthw_iic.c @@ -253,10 +253,7 @@ int nthw_iic_init(nthw_iic_t *p, nthw_fpga_t *p_fpga, int n_iic_instance, void nthw_iic_delete(nthw_iic_t *p) { - if (p) { - memset(p, 0, sizeof(nthw_iic_t)); - free(p); - } + free(p); } int nthw_iic_set_retry_params(nthw_iic_t *p, const int n_poll_delay, const int n_bus_ready_retry, diff --git a/drivers/net/ntnic/nthw/core/nthw_pcie3.c b/drivers/net/ntnic/nthw/core/nthw_pcie3.c index 5997ebb419..a5833e166c 100644 --- a/drivers/net/ntnic/nthw/core/nthw_pcie3.c +++ b/drivers/net/ntnic/nthw/core/nthw_pcie3.c @@ -24,10 +24,7 @@ nthw_pcie3_t *nthw_pcie3_new(void) void nthw_pcie3_delete(nthw_pcie3_t *p) { - if (p) { - memset(p, 0, sizeof(nthw_pcie3_t)); - free(p); - } + free(p); } int nthw_pcie3_init(nthw_pcie3_t *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/core/nthw_rpf.c b/drivers/net/ntnic/nthw/core/nthw_rpf.c index 1ed4d7b4e0..d5c19e312b 100644 --- a/drivers/net/ntnic/nthw/core/nthw_rpf.c +++ b/drivers/net/ntnic/nthw/core/nthw_rpf.c @@ -22,10 +22,7 @@ nthw_rpf_t *nthw_rpf_new(void) void nthw_rpf_delete(nthw_rpf_t *p) { - if (p) { - memset(p, 0, sizeof(nthw_rpf_t)); - free(p); - } + free(p); } int nthw_rpf_init(nthw_rpf_t *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/core/nthw_sdc.c b/drivers/net/ntnic/nthw/core/nthw_sdc.c index fc73e6957c..e32d87b967 100644 --- a/drivers/net/ntnic/nthw/core/nthw_sdc.c +++ b/drivers/net/ntnic/nthw/core/nthw_sdc.c @@ -22,10 +22,7 @@ nthw_sdc_t *nthw_sdc_new(void) void nthw_sdc_delete(nthw_sdc_t *p) { - if (p) { - memset(p, 0, sizeof(nthw_sdc_t)); - free(p); - } + free(p); } int nthw_sdc_init(nthw_sdc_t *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/core/nthw_si5340.c b/drivers/net/ntnic/nthw/core/nthw_si5340.c index 05cadc0bf4..ceaa58e0f7 100644 --- a/drivers/net/ntnic/nthw/core/nthw_si5340.c +++ b/drivers/net/ntnic/nthw/core/nthw_si5340.c @@ -44,10 +44,7 @@ int nthw_si5340_init(nthw_si5340_t *p, nthw_iic_t *p_nthw_iic, uint8_t n_iic_add void nthw_si5340_delete(nthw_si5340_t *p) { - if (p) { - memset(p, 0, sizeof(nthw_si5340_t)); - free(p); - } + free(p); } /* diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c index 649a060682..776d4986f7 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_cat.c @@ -25,10 +25,7 @@ struct cat_nthw *cat_nthw_new(void) void cat_nthw_delete(struct cat_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } void cat_nthw_set_debug_mode(struct cat_nthw *p, unsigned int n_debug_mode) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c index dc3582c9f5..036a25d06c 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_csu.c @@ -29,10 +29,7 @@ struct csu_nthw *csu_nthw_new(void) void csu_nthw_delete(struct csu_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int csu_nthw_init(struct csu_nthw *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c index 7647949bc7..e016c80bc9 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_flm.c @@ -26,10 +26,7 @@ struct flm_nthw *flm_nthw_new(void) void flm_nthw_delete(struct flm_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } void flm_nthw_set_debug_mode(struct flm_nthw *p, unsigned int n_debug_mode) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c index d0e65c071d..02064f1bc6 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hfu.c @@ -29,10 +29,7 @@ struct hfu_nthw *hfu_nthw_new(void) void hfu_nthw_delete(struct hfu_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int hfu_nthw_init(struct hfu_nthw *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c index caf84790cd..10f982ea76 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_hsh.c @@ -30,10 +30,7 @@ struct hsh_nthw *hsh_nthw_new(void) void hsh_nthw_delete(struct hsh_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int hsh_nthw_init(struct hsh_nthw *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c index 007c71d1b0..acdc40b8b1 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_info.c @@ -30,10 +30,7 @@ struct info_nthw *info_nthw_new(void) void info_nthw_delete(struct info_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int info_nthw_init(struct info_nthw *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c index 3909194214..7acdd6d36f 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_km.c @@ -39,10 +39,7 @@ struct km_nthw *km_nthw_new(void) void km_nthw_delete(struct km_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int km_nthw_init(struct km_nthw *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c index 700d868cef..850bb1d22d 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_pdb.c @@ -30,10 +30,7 @@ struct pdb_nthw *pdb_nthw_new(void) void pdb_nthw_delete(struct pdb_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int pdb_nthw_init(struct pdb_nthw *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c index 47fea6553f..ccc5f46f02 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_qsl.c @@ -30,10 +30,7 @@ struct qsl_nthw *qsl_nthw_new(void) void qsl_nthw_delete(struct qsl_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int qsl_nthw_init(struct qsl_nthw *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c index 2f141acbde..11f691dcb5 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_rpp_lr.c @@ -29,10 +29,7 @@ struct rpp_lr_nthw *rpp_lr_nthw_new(void) void rpp_lr_nthw_delete(struct rpp_lr_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int rpp_lr_nthw_init(struct rpp_lr_nthw *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c index e278ad0a41..8b51fa18ad 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_slc_lr.c @@ -30,10 +30,7 @@ struct slc_lr_nthw *slc_lr_nthw_new(void) void slc_lr_nthw_delete(struct slc_lr_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int slc_lr_nthw_init(struct slc_lr_nthw *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c index 58e5672462..f7987b9c8c 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_cpy.c @@ -31,7 +31,6 @@ void tx_cpy_nthw_delete(struct tx_cpy_nthw *p) { if (p) { free(p->m_writers); - memset(p, 0, sizeof(*p)); free(p); } } diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c index 0ff9af22c7..c21fb5413b 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_ins.c @@ -29,10 +29,7 @@ struct tx_ins_nthw *tx_ins_nthw_new(void) void tx_ins_nthw_delete(struct tx_ins_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int tx_ins_nthw_init(struct tx_ins_nthw *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c index 221ea94eed..62bed95be1 100644 --- a/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c +++ b/drivers/net/ntnic/nthw/flow_filter/flow_nthw_tx_rpl.c @@ -29,10 +29,7 @@ struct tx_rpl_nthw *tx_rpl_nthw_new(void) void tx_rpl_nthw_delete(struct tx_rpl_nthw *p) { - if (p) { - memset(p, 0, sizeof(*p)); - free(p); - } + free(p); } int tx_rpl_nthw_init(struct tx_rpl_nthw *p, nthw_fpga_t *p_fpga, int n_instance) diff --git a/drivers/net/ntnic/nthw/model/nthw_fpga_model.c b/drivers/net/ntnic/nthw/model/nthw_fpga_model.c index 9eaaeb550d..0fb525d34f 100644 --- a/drivers/net/ntnic/nthw/model/nthw_fpga_model.c +++ b/drivers/net/ntnic/nthw/model/nthw_fpga_model.c @@ -183,7 +183,6 @@ nthw_fpga_mgr_t *nthw_fpga_mgr_new(void) void nthw_fpga_mgr_delete(nthw_fpga_mgr_t *p) { - memset(p, 0, sizeof(nthw_fpga_mgr_t)); free(p); } -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH v4 12/12] devtools/cocci: add script to find problematic memset 2024-11-14 18:43 ` [PATCH v4 00/12] memset security fixes Stephen Hemminger ` (10 preceding siblings ...) 2024-11-14 18:43 ` [PATCH v4 11/12] net/ntnic: remove unnecessary memset Stephen Hemminger @ 2024-11-14 18:43 ` Stephen Hemminger 11 siblings, 0 replies; 42+ messages in thread From: Stephen Hemminger @ 2024-11-14 18:43 UTC (permalink / raw) To: dev; +Cc: Stephen Hemminger Script that converts memset before free into rte_memset_sensitive and memset before rte_free into rte_free_sensitive Signed-off-by: Stephen Hemminger <stephen@networkplumber.org> --- devtools/cocci/memset_free.cocci | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 devtools/cocci/memset_free.cocci diff --git a/devtools/cocci/memset_free.cocci b/devtools/cocci/memset_free.cocci new file mode 100644 index 0000000000..834c5dbf2b --- /dev/null +++ b/devtools/cocci/memset_free.cocci @@ -0,0 +1,20 @@ +// Replace calls to memset before free +@@ +expression E, size; +@@ +( +- memset(E, 0, size); +- free(E); ++ rte_memset_sensitive(E, 0, size); ++ free(E); +) + +// replace to memset before rte_free +@@ +expression E, size; +@@ +( +- memset(E, 0, size); +- rte_free(E); ++ rte_free_sensitive(E); +) -- 2.45.2 ^ permalink raw reply [flat|nested] 42+ messages in thread
end of thread, other threads:[~2024-11-17 18:26 UTC | newest] Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2024-11-14 1:10 [PATCH 0/3] introduce rte_memset_sensative Stephen Hemminger 2024-11-14 1:10 ` [PATCH 1/3] eal: " Stephen Hemminger 2024-11-14 1:10 ` [PATCH 2/3] crypto/qat: use rte_memset_sensative Stephen Hemminger 2024-11-14 1:10 ` [PATCH 3/3] eal: add rte_free_sensative Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 0/8] memset security handling Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 1/8] eal: introduce new secure memory fill Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 2/8] eal: add new secure free function Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 3/8] crypto/qat: force zero of keys Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 4/8] crypto/qat: fix size calculation for memset Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 5/8] crypto/qat: use secure memset Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 6/8] bus/uacce: remove memset before free Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 7/8] compress/octeontx: remove unnecessary memset Stephen Hemminger 2024-11-14 1:52 ` [PATCH v2 8/8] test: remove unneeded memset Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 00/11] memset security handling Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 01/11] eal: introduce new secure memory fill Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 02/11] eal: add new secure free function Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 03/11] crypto/qat: force zero of keys Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 04/11] crypto/qat: fix size calculation for memset Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 05/11] crypto/qat: use secure memset Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 06/11] bus/uacce: remove memset before free Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 07/11] compress/octeontx: remove unnecessary memset Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 08/11] test: remove unneeded memset Stephen Hemminger 2024-11-14 2:35 ` [PATCH v3 09/11] net/ntnic: remove unnecessary void cast Stephen Hemminger 2024-11-17 18:16 ` Serhii Iliushyk 2024-11-14 2:35 ` [PATCH v3 10/11] net/ntnic: check result of malloc Stephen Hemminger 2024-11-17 18:24 ` Serhii Iliushyk 2024-11-14 2:36 ` [PATCH v3 11/11] net/ntnic: remove unnecessary memset Stephen Hemminger 2024-11-17 18:26 ` Serhii Iliushyk 2024-11-14 18:43 ` [PATCH v4 00/12] memset security fixes Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 01/12] eal: introduce new secure memory fill Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 02/12] eal: add new secure free function Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 03/12] crypto/qat: force zero of keys Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 04/12] crypto/qat: fix size calculation for memset Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 05/12] crypto/qat: use secure memset Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 06/12] bus/uacce: remove memset before free Stephen Hemminger 2024-11-15 6:04 ` fengchengwen 2024-11-14 18:43 ` [PATCH v4 07/12] compress/octeontx: remove unnecessary memset Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 08/12] test: remove unneeded memset Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 09/12] net/ntnic: remove unnecessary void cast Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 10/12] net/ntnic: check result of malloc Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 11/12] net/ntnic: remove unnecessary memset Stephen Hemminger 2024-11-14 18:43 ` [PATCH v4 12/12] devtools/cocci: add script to find problematic memset Stephen Hemminger
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).