From 12cc7c6558f50310e1053da7fb0a6b29ca611d9b Mon Sep 17 00:00:00 2001 From: Nisha Menon Date: Tue, 21 Apr 2020 20:29:40 -0700 Subject: [PATCH] qcacmn: SKB buf memory Leak@ Func dp_pdev_nbuf_alloc_and_map Fix the skb leak in dp_rx_process where rx descriptor cookie validity fails. This skb should be cleaned up as part of the rx desc and nbuf free function called during the driver unload. However this will ensure that the skb released and added rx desc added to the free list during dp_rx_process itself. Add skb map, unmap functions, line numbers and if the nbuf is mapped or unmapped to the nbuf tracking table. This debug info will be logged once the skb is leaked. Change-Id: I52dbf38922be20fc0aaea380e0e572af16de773e CRs-Fixed: 2662992 --- dp/wifi3.0/dp_rx.c | 16 +++++++- qdf/inc/qdf_nbuf.h | 39 ++++++++++++++++++++ qdf/linux/src/qdf_nbuf.c | 79 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 1 deletion(-) diff --git a/dp/wifi3.0/dp_rx.c b/dp/wifi3.0/dp_rx.c index ad309fb00f..6fafbb217c 100644 --- a/dp/wifi3.0/dp_rx.c +++ b/dp/wifi3.0/dp_rx.c @@ -1943,6 +1943,20 @@ more_data: status = dp_rx_desc_sanity(soc, hal_soc, hal_ring_hdl, ring_desc, rx_desc); if (QDF_IS_STATUS_ERROR(status)) { + if (qdf_unlikely(rx_desc && rx_desc->nbuf)) { + qdf_assert_always(rx_desc->unmapped); + qdf_nbuf_unmap_nbytes_single( + soc->osdev, + rx_desc->nbuf, + QDF_DMA_FROM_DEVICE, + RX_DATA_BUFFER_SIZE); + rx_desc->unmapped = 1; + qdf_nbuf_free(rx_desc->nbuf); + dp_rx_add_to_free_desc_list( + &head[rx_desc->pool_id], + &tail[rx_desc->pool_id], + rx_desc); + } hal_srng_dst_get_next(hal_soc, hal_ring_hdl); continue; } @@ -2464,7 +2478,7 @@ QDF_STATUS dp_rx_vdev_detach(struct dp_vdev *vdev) if (vdev->osif_rx_flush) { ret = vdev->osif_rx_flush(vdev->osif_vdev, vdev->vdev_id); - if (!ret) { + if (!QDF_IS_STATUS_SUCCESS(ret)) { dp_err("Failed to flush rx pkts for vdev %d\n", vdev->vdev_id); return ret; diff --git a/qdf/inc/qdf_nbuf.h b/qdf/inc/qdf_nbuf.h index b825fe0c50..5d5d0e3cd6 100644 --- a/qdf/inc/qdf_nbuf.h +++ b/qdf/inc/qdf_nbuf.h @@ -1468,6 +1468,32 @@ void qdf_net_buf_debug_update_node(qdf_nbuf_t net_buf, const char *func_name, uint32_t line_num); void qdf_net_buf_debug_delete_node(qdf_nbuf_t net_buf); +/** + * qdf_net_buf_debug_update_map_node() - update nbuf in debug + * hash table with the mapping function info + * @nbuf: network buffer + * @func: function name that requests for mapping the nbuf + * @line_num: function line number + * + * Return: none + */ +void qdf_net_buf_debug_update_map_node(qdf_nbuf_t net_buf, + const char *func_name, + uint32_t line_num); + +/** + * qdf_net_buf_debug_update_unmap_node() - update nbuf in debug + * hash table with the unmap function info + * @nbuf: network buffer + * @func: function name that requests for unmapping the nbuf + * @line_num: function line number + * + * Return: none + */ +void qdf_net_buf_debug_update_unmap_node(qdf_nbuf_t net_buf, + const char *func_name, + uint32_t line_num); + /** * qdf_net_buf_debug_acquire_skb() - acquire skb to avoid memory leak * @net_buf: Network buf holding head segment (single) @@ -1572,6 +1598,19 @@ qdf_net_buf_debug_update_node(qdf_nbuf_t net_buf, const char *func_name, { } +static inline void +qdf_net_buf_debug_update_map_node(qdf_nbuf_t net_buf, + const char *func_name, + uint32_t line_num) +{ +} + +static inline void +qdf_net_buf_debug_update_unmap_node(qdf_nbuf_t net_buf, + const char *func_name, + uint32_t line_num) +{ +} /* Nbuf allocation rouines */ #define qdf_nbuf_alloc(osdev, size, reserve, align, prio) \ diff --git a/qdf/linux/src/qdf_nbuf.c b/qdf/linux/src/qdf_nbuf.c index abf396638b..860e1f5e22 100644 --- a/qdf/linux/src/qdf_nbuf.c +++ b/qdf/linux/src/qdf_nbuf.c @@ -671,6 +671,8 @@ QDF_STATUS qdf_nbuf_map_debug(qdf_device_t osdev, status = __qdf_nbuf_map(osdev, buf, dir); if (QDF_IS_STATUS_ERROR(status)) qdf_nbuf_untrack_map(buf, func, line); + else + qdf_net_buf_debug_update_map_node(buf, func, line); return status; } @@ -685,6 +687,7 @@ void qdf_nbuf_unmap_debug(qdf_device_t osdev, { qdf_nbuf_untrack_map(buf, func, line); __qdf_nbuf_unmap_single(osdev, buf, dir); + qdf_net_buf_debug_update_unmap_node(buf, func, line); } qdf_export_symbol(qdf_nbuf_unmap_debug); @@ -704,6 +707,8 @@ QDF_STATUS qdf_nbuf_map_single_debug(qdf_device_t osdev, status = __qdf_nbuf_map_single(osdev, buf, dir); if (QDF_IS_STATUS_ERROR(status)) qdf_nbuf_untrack_map(buf, func, line); + else + qdf_net_buf_debug_update_map_node(buf, func, line); return status; } @@ -718,6 +723,7 @@ void qdf_nbuf_unmap_single_debug(qdf_device_t osdev, { qdf_nbuf_untrack_map(buf, func, line); __qdf_nbuf_unmap_single(osdev, buf, dir); + qdf_net_buf_debug_update_unmap_node(buf, func, line); } qdf_export_symbol(qdf_nbuf_unmap_single_debug); @@ -738,6 +744,8 @@ QDF_STATUS qdf_nbuf_map_nbytes_debug(qdf_device_t osdev, status = __qdf_nbuf_map_nbytes(osdev, buf, dir, nbytes); if (QDF_IS_STATUS_ERROR(status)) qdf_nbuf_untrack_map(buf, func, line); + else + qdf_net_buf_debug_update_map_node(buf, func, line); return status; } @@ -753,6 +761,7 @@ void qdf_nbuf_unmap_nbytes_debug(qdf_device_t osdev, { qdf_nbuf_untrack_map(buf, func, line); __qdf_nbuf_unmap_nbytes(osdev, buf, dir, nbytes); + qdf_net_buf_debug_update_unmap_node(buf, func, line); } qdf_export_symbol(qdf_nbuf_unmap_nbytes_debug); @@ -773,6 +782,8 @@ QDF_STATUS qdf_nbuf_map_nbytes_single_debug(qdf_device_t osdev, status = __qdf_nbuf_map_nbytes_single(osdev, buf, dir, nbytes); if (QDF_IS_STATUS_ERROR(status)) qdf_nbuf_untrack_map(buf, func, line); + else + qdf_net_buf_debug_update_map_node(buf, func, line); return status; } @@ -788,6 +799,7 @@ void qdf_nbuf_unmap_nbytes_single_debug(qdf_device_t osdev, { qdf_nbuf_untrack_map(buf, func, line); __qdf_nbuf_unmap_nbytes_single(osdev, buf, dir, nbytes); + qdf_net_buf_debug_update_unmap_node(buf, func, line); } qdf_export_symbol(qdf_nbuf_unmap_nbytes_single_debug); @@ -2074,6 +2086,11 @@ qdf_export_symbol(__qdf_nbuf_is_bcast_pkt); * @func_name: Function name * @line_num: Line number * @size: Size + * @map_func_name: nbuf mapping function name + * @map_line_num: mapping function line number + * @unmap_func_name: nbuf unmapping function name + * @unmap_line_num: mapping function line number + * @is_nbuf_mapped: indicate mapped/unmapped nbuf */ struct qdf_nbuf_track_t { struct qdf_nbuf_track_t *p_next; @@ -2081,6 +2098,11 @@ struct qdf_nbuf_track_t { char func_name[QDF_MEM_FUNC_NAME_SIZE]; uint32_t line_num; size_t size; + char map_func_name[QDF_MEM_FUNC_NAME_SIZE]; + uint32_t map_line_num; + char unmap_func_name[QDF_MEM_FUNC_NAME_SIZE]; + uint32_t unmap_line_num; + bool is_nbuf_mapped; }; static spinlock_t g_qdf_net_buf_track_lock[QDF_NET_BUF_TRACK_MAX_SIZE]; @@ -2393,6 +2415,13 @@ void qdf_net_buf_debug_exit(void) qdf_info("SKB buf memory Leak@ Func %s, @Line %d, size %zu, nbuf %pK", p_prev->func_name, p_prev->line_num, p_prev->size, p_prev->net_buf); + qdf_info( + "SKB leak map %s, line %d, unmap %s line %d mapped=%d", + p_prev->map_func_name, + p_prev->map_line_num, + p_prev->unmap_func_name, + p_prev->unmap_line_num, + p_prev->is_nbuf_mapped); qdf_nbuf_track_free(p_prev); } spin_unlock_irqrestore(&g_qdf_net_buf_track_lock[i], irq_flag); @@ -2524,6 +2553,56 @@ void qdf_net_buf_debug_update_node(qdf_nbuf_t net_buf, const char *func_name, qdf_export_symbol(qdf_net_buf_debug_update_node); +void qdf_net_buf_debug_update_map_node(qdf_nbuf_t net_buf, + const char *func_name, + uint32_t line_num) +{ + uint32_t i; + unsigned long irq_flag; + QDF_NBUF_TRACK *p_node; + + if (is_initial_mem_debug_disabled) + return; + + i = qdf_net_buf_debug_hash(net_buf); + spin_lock_irqsave(&g_qdf_net_buf_track_lock[i], irq_flag); + + p_node = qdf_net_buf_debug_look_up(net_buf); + + if (p_node) { + qdf_str_lcopy(p_node->map_func_name, func_name, + QDF_MEM_FUNC_NAME_SIZE); + p_node->map_line_num = line_num; + p_node->is_nbuf_mapped = true; + } + spin_unlock_irqrestore(&g_qdf_net_buf_track_lock[i], irq_flag); +} + +void qdf_net_buf_debug_update_unmap_node(qdf_nbuf_t net_buf, + const char *func_name, + uint32_t line_num) +{ + uint32_t i; + unsigned long irq_flag; + QDF_NBUF_TRACK *p_node; + + if (is_initial_mem_debug_disabled) + return; + + i = qdf_net_buf_debug_hash(net_buf); + spin_lock_irqsave(&g_qdf_net_buf_track_lock[i], irq_flag); + + p_node = qdf_net_buf_debug_look_up(net_buf); + + if (p_node) { + qdf_str_lcopy(p_node->unmap_func_name, func_name, + QDF_MEM_FUNC_NAME_SIZE); + p_node->unmap_line_num = line_num; + p_node->is_nbuf_mapped = false; + } + spin_unlock_irqrestore(&g_qdf_net_buf_track_lock[i], irq_flag); +} + /** * qdf_net_buf_debug_delete_node() - remove skb from debug hash table *