From c9681754d0aa93a1788dc37a2ddc23dfa999c03e Mon Sep 17 00:00:00 2001 From: Dustin Brown Date: Fri, 1 Mar 2019 15:08:21 -0800 Subject: [PATCH] qcacmn: Use panic in qdf_mem_copy/move/set/zero/cmp() It has been observed at that several issues have crept into mainline due to qdf_mem_copy() gracefully returning when the source or destination pointers are NULL. In order to prevent such regressions in the future, change calls to qdf_mem_copy/move/set/zero/cmp() with NULL pointer parameters so they panic in debug builds. Change-Id: I6df4c42245ed1d0d8a85477f59552f8243488924 CRs-Fixed: 2408266 --- qdf/inc/qdf_mem.h | 90 +++++++++++++++++++++++-------- qdf/linux/src/i_qdf_mem.h | 23 +------- qdf/linux/src/qdf_mem.c | 109 +++++++++----------------------------- 3 files changed, 93 insertions(+), 129 deletions(-) diff --git a/qdf/inc/qdf_mem.h b/qdf/inc/qdf_mem.h index b1d815f8d0..c82c7bd1b1 100644 --- a/qdf/inc/qdf_mem.h +++ b/qdf/inc/qdf_mem.h @@ -318,41 +318,87 @@ void qdf_mem_set_io(void *ptr, uint32_t num_bytes, uint32_t value); void qdf_mem_copy_toio(void *dst_addr, const void *src_addr, uint32_t num_bytes); +/** + * qdf_mem_set() - set (fill) memory with a specified byte value. + * @ptr: Pointer to memory that will be set + * @num_bytes: Number of bytes to be set + * @value: Byte set in memory + * + * WARNING: parameter @num_bytes and @value are swapped comparing with + * standard C function "memset", please ensure correct usage of this function! + * + * Return: None + */ void qdf_mem_set(void *ptr, uint32_t num_bytes, uint32_t value); -void qdf_mem_zero(void *ptr, uint32_t num_bytes); +/** + * qdf_mem_zero() - zero out memory + * @ptr: pointer to memory that will be set to zero + * @num_bytes: number of bytes zero + * + * This function sets the memory location to all zeros, essentially clearing + * the memory. + * + * Return: None + */ +static inline void qdf_mem_zero(void *ptr, uint32_t num_bytes) +{ + qdf_mem_set(ptr, num_bytes, 0); +} +/** + * qdf_mem_copy() - copy memory + * @dst_addr: Pointer to destination memory location (to copy to) + * @src_addr: Pointer to source memory location (to copy from) + * @num_bytes: Number of bytes to copy. + * + * Copy host memory from one location to another, similar to memcpy in + * standard C. Note this function does not specifically handle overlapping + * source and destination memory locations. Calling this function with + * overlapping source and destination memory locations will result in + * unpredictable results. Use qdf_mem_move() if the memory locations + * for the source and destination are overlapping (or could be overlapping!) + * + * Return: none + */ void qdf_mem_copy(void *dst_addr, const void *src_addr, uint32_t num_bytes); +/** + * qdf_mem_move() - move memory + * @dst_addr: pointer to destination memory location (to move to) + * @src_addr: pointer to source memory location (to move from) + * @num_bytes: number of bytes to move. + * + * Move host memory from one location to another, similar to memmove in + * standard C. Note this function *does* handle overlapping + * source and destination memory locations. + + * Return: None + */ void qdf_mem_move(void *dst_addr, const void *src_addr, uint32_t num_bytes); +/** + * qdf_mem_cmp() - memory compare + * @left: pointer to one location in memory to compare + * @right: pointer to second location in memory to compare + * @size: the number of bytes to compare + * + * Function to compare two pieces of memory, similar to memcmp function + * in standard C. + * + * Return: + * 0 -- equal + * < 0 -- *memory1 is less than *memory2 + * > 0 -- *memory1 is bigger than *memory2 + */ +int qdf_mem_cmp(const void *left, const void *right, size_t size); + void qdf_mem_free_outline(void *buf); void qdf_mem_zero_outline(void *buf, qdf_size_t size); void qdf_ether_addr_copy(void *dst_addr, const void *src_addr); -/** - * qdf_mem_cmp() - memory compare - * @memory1: pointer to one location in memory to compare. - * @memory2: pointer to second location in memory to compare. - * @num_bytes: the number of bytes to compare. - * - * Function to compare two pieces of memory, similar to memcmp function - * in standard C. - * Return: - * int32_t - returns an int value that tells if the memory - * locations are equal or not equal. - * 0 -- equal - * < 0 -- *memory1 is less than *memory2 - * > 0 -- *memory1 is bigger than *memory2 - */ -static inline int32_t qdf_mem_cmp(const void *memory1, const void *memory2, - uint32_t num_bytes) -{ - return __qdf_mem_cmp(memory1, memory2, num_bytes); -} - /** * qdf_mem_map_nbytes_single - Map memory for DMA * @osdev: pomter OS device context diff --git a/qdf/linux/src/i_qdf_mem.h b/qdf/linux/src/i_qdf_mem.h index 21a92979b0..fa195ad4d3 100644 --- a/qdf/linux/src/i_qdf_mem.h +++ b/qdf/linux/src/i_qdf_mem.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2014-2018 The Linux Foundation. All rights reserved. + * Copyright (c) 2014-2019 The Linux Foundation. All rights reserved. * * Permission to use, copy, modify, and/or distribute this software for * any purpose with or without fee is hereby granted, provided that the @@ -180,27 +180,6 @@ void __qdf_mempool_free(qdf_device_t osdev, __qdf_mempool_t pool, void *buf); #define __qdf_mempool_elem_size(_pool) ((_pool)->elem_size) #endif -/** - * __qdf_mem_cmp() - memory compare - * @memory1: pointer to one location in memory to compare. - * @memory2: pointer to second location in memory to compare. - * @num_bytes: the number of bytes to compare. - * - * Function to compare two pieces of memory, similar to memcmp function - * in standard C. - * Return: - * int32_t - returns an int value that tells if the memory - * locations are equal or not equal. - * 0 -- equal - * < 0 -- *memory1 is less than *memory2 - * > 0 -- *memory1 is bigger than *memory2 - */ -static inline int32_t __qdf_mem_cmp(const void *memory1, const void *memory2, - uint32_t num_bytes) -{ - return (int32_t) memcmp(memory1, memory2, num_bytes); -} - /** * __qdf_mem_smmu_s1_enabled() - Return SMMU stage 1 translation enable status * @osdev parent device instance diff --git a/qdf/linux/src/qdf_mem.c b/qdf/linux/src/qdf_mem.c index 1292119984..4ceedee782 100644 --- a/qdf/linux/src/qdf_mem.c +++ b/qdf/linux/src/qdf_mem.c @@ -1423,35 +1423,17 @@ int qdf_mem_multi_page_link(qdf_device_t osdev, } qdf_export_symbol(qdf_mem_multi_page_link); -/** - * qdf_mem_copy() - copy memory - * @dst_addr: Pointer to destination memory location (to copy to) - * @src_addr: Pointer to source memory location (to copy from) - * @num_bytes: Number of bytes to copy. - * - * Copy host memory from one location to another, similar to memcpy in - * standard C. Note this function does not specifically handle overlapping - * source and destination memory locations. Calling this function with - * overlapping source and destination memory locations will result in - * unpredictable results. Use qdf_mem_move() if the memory locations - * for the source and destination are overlapping (or could be overlapping!) - * - * Return: none - */ void qdf_mem_copy(void *dst_addr, const void *src_addr, uint32_t num_bytes) { - if (0 == num_bytes) { - /* special case where dst_addr or src_addr can be NULL */ + /* special case where dst_addr or src_addr can be NULL */ + if (!num_bytes) return; - } - if ((dst_addr == NULL) || (src_addr == NULL)) { - QDF_TRACE(QDF_MODULE_ID_QDF, QDF_TRACE_LEVEL_ERROR, - "%s called with NULL parameter, source:%pK destination:%pK", - __func__, src_addr, dst_addr); - QDF_ASSERT(0); + QDF_BUG(dst_addr); + QDF_BUG(src_addr); + if (!dst_addr || !src_addr) return; - } + memcpy(dst_addr, src_addr, num_bytes); } qdf_export_symbol(qdf_mem_copy); @@ -1504,32 +1486,6 @@ qdf_shared_mem_t *qdf_mem_shared_mem_alloc(qdf_device_t osdev, uint32_t size) qdf_export_symbol(qdf_mem_shared_mem_alloc); -/** - * qdf_mem_zero() - zero out memory - * @ptr: pointer to memory that will be set to zero - * @num_bytes: number of bytes zero - * - * This function sets the memory location to all zeros, essentially clearing - * the memory. - * - * Return: None - */ -void qdf_mem_zero(void *ptr, uint32_t num_bytes) -{ - if (0 == num_bytes) { - /* special case where ptr can be NULL */ - return; - } - - if (ptr == NULL) { - QDF_TRACE(QDF_MODULE_ID_QDF, QDF_TRACE_LEVEL_ERROR, - "%s called with NULL parameter ptr", __func__); - return; - } - memset(ptr, 0, num_bytes); -} -qdf_export_symbol(qdf_mem_zero); - /** * qdf_mem_copy_toio() - copy memory * @dst_addr: Pointer to destination memory location (to copy to) @@ -1576,57 +1532,40 @@ void qdf_mem_set_io(void *ptr, uint32_t num_bytes, uint32_t value) qdf_export_symbol(qdf_mem_set_io); -/** - * qdf_mem_set() - set (fill) memory with a specified byte value. - * @ptr: Pointer to memory that will be set - * @num_bytes: Number of bytes to be set - * @value: Byte set in memory - * - * WARNING: parameter @num_bytes and @value are swapped comparing with - * standard C function "memset", please ensure correct usage of this function! - * - * Return: None - */ void qdf_mem_set(void *ptr, uint32_t num_bytes, uint32_t value) { - if (ptr == NULL) { - qdf_print("%s called with NULL parameter ptr", __func__); + QDF_BUG(ptr); + if (!ptr) return; - } + memset(ptr, value, num_bytes); } qdf_export_symbol(qdf_mem_set); -/** - * qdf_mem_move() - move memory - * @dst_addr: pointer to destination memory location (to move to) - * @src_addr: pointer to source memory location (to move from) - * @num_bytes: number of bytes to move. - * - * Move host memory from one location to another, similar to memmove in - * standard C. Note this function *does* handle overlapping - * source and destination memory locations. - - * Return: None - */ void qdf_mem_move(void *dst_addr, const void *src_addr, uint32_t num_bytes) { - if (0 == num_bytes) { - /* special case where dst_addr or src_addr can be NULL */ + /* special case where dst_addr or src_addr can be NULL */ + if (!num_bytes) return; - } - if ((dst_addr == NULL) || (src_addr == NULL)) { - QDF_TRACE(QDF_MODULE_ID_QDF, QDF_TRACE_LEVEL_ERROR, - "%s called with NULL parameter, source:%pK destination:%pK", - __func__, src_addr, dst_addr); - QDF_ASSERT(0); + QDF_BUG(dst_addr); + QDF_BUG(src_addr); + if (!dst_addr || !src_addr) return; - } + memmove(dst_addr, src_addr, num_bytes); } qdf_export_symbol(qdf_mem_move); +int qdf_mem_cmp(const void *left, const void *right, size_t size) +{ + QDF_BUG(left); + QDF_BUG(right); + + return memcmp(left, right, size); +} +qdf_export_symbol(qdf_mem_cmp); + #if defined(A_SIMOS_DEVHOST) || defined(HIF_SDIO) || defined(HIF_USB) /** * qdf_mem_dma_alloc() - allocates memory for dma