wlan: Reallocate skb before reusing in wlan_logging pkt_stats filled list
In wlan_queue_pkt_stats_for_app, if the pkt_stat_free_list is empty,
the current node is taken from the head of pkt_stat_filled_list,
detached from the list and skb is set for re-use. This current node
is reattached back to the tail of the pkt_stat_filled_list for the next
log. This could lead to skb tail overrun as skb is not reset before
re-use.
Free the existing skb allocated to the pkt_stat_filled_list head
and allocate again to have a clean skb.
Change-Id: I6bb5de71c8bed062022622cb5394b20d61d35545
CRs-Fixed: 2195551
diff --git a/CORE/SVC/src/logging/wlan_logging_sock_svc.c b/CORE/SVC/src/logging/wlan_logging_sock_svc.c
index fd48982..ed5f632 100644
--- a/CORE/SVC/src/logging/wlan_logging_sock_svc.c
+++ b/CORE/SVC/src/logging/wlan_logging_sock_svc.c
@@ -193,6 +193,7 @@
struct fw_mem_dump_logging fw_mem_dump_ctx;
int pkt_stat_num_buf;
unsigned int pkt_stat_drop_cnt;
+ unsigned int malloc_fail_count;
struct list_head pkt_stat_free_list;
struct list_head pkt_stat_filled_list;
struct pkt_stats_msg *pkt_stats_pcur_node;
@@ -407,34 +408,51 @@
/* Need to call this with spin_lock acquired */
-static int wlan_queue_pkt_stats_for_app(void)
+static int wlan_queue_pkt_stats_for_app(struct sk_buff *skb_to_free)
{
int ret = 0;
-
- list_add_tail(&gwlan_logging.pkt_stats_pcur_node->node,
- &gwlan_logging.pkt_stat_filled_list);
+ struct sk_buff *skb_new = NULL;
+ struct pkt_stats_msg *tmp_node = NULL;
if (!list_empty(&gwlan_logging.pkt_stat_free_list)) {
+ /* Add current node to end of filled list */
+ list_add_tail(&gwlan_logging.pkt_stats_pcur_node->node,
+ &gwlan_logging.pkt_stat_filled_list);
+
/* Get buffer from free list */
gwlan_logging.pkt_stats_pcur_node =
- (struct pkt_stats_msg *)(gwlan_logging.pkt_stat_free_list.next);
+ (struct pkt_stats_msg *)(gwlan_logging.
+ pkt_stat_free_list.next);
list_del_init(gwlan_logging.pkt_stat_free_list.next);
} else if (!list_empty(&gwlan_logging.pkt_stat_filled_list)) {
- /* Get buffer from filled list */
- /* This condition will drop the packet from being
+ /*
+ * Get buffer from filled list, free the skb and allocate a
+ * new skb so that the current node has a clean skb
+ */
+ tmp_node = (struct pkt_stats_msg *)(gwlan_logging.
+ pkt_stat_filled_list.next);
+
+ skb_new = dev_alloc_skb(MAX_PKTSTATS_LOG_LENGTH);
+ if (skb_new == NULL) {
+ /* Print error for every 512 malloc fails */
+ gwlan_logging.malloc_fail_count++;
+ return -ENOMEM;
+ }
+ skb_to_free = tmp_node->skb;
+ tmp_node->skb = skb_new;
+
+ /*
+ * This condition will drop the packet from being
* indicated to app
*/
- gwlan_logging.pkt_stats_pcur_node =
- (struct pkt_stats_msg *)(gwlan_logging.pkt_stat_filled_list.next);
++gwlan_logging.pkt_stat_drop_cnt;
- /* print every 64th drop count */
- if (vos_is_multicast_logging() &&
- (!(gwlan_logging.pkt_stat_drop_cnt % 0x40))) {
- pr_err("%s: drop_count = %u filled_length = %d\n",
- __func__, gwlan_logging.pkt_stat_drop_cnt,
- gwlan_logging.pkt_stats_pcur_node->skb->len);
- }
- list_del_init(gwlan_logging.pkt_stat_filled_list.next);
+
+ /* Add current node to end of filled list */
+ list_add_tail(&gwlan_logging.pkt_stats_pcur_node->node,
+ &gwlan_logging.pkt_stat_filled_list);
+
+ gwlan_logging.pkt_stats_pcur_node = tmp_node;
+ list_del_init((struct list_head *) tmp_node);
ret = 1;
}
@@ -450,7 +468,9 @@
unsigned long flags;
tx_rx_pkt_stats rx_tx_stats;
int total_log_len = 0;
+ int ret = 0;
struct sk_buff *ptr;
+ struct sk_buff *skb_to_free = NULL;
tpSirMacMgmtHdr hdr;
uint32 rateIdx;
@@ -526,19 +546,42 @@
return -EIO;
}
- ;
-
- /* Check if we can accomodate more log into current node/buffer */
+ /* Check if we can accomodate more log into current node/buffer */
if (total_log_len + sizeof(vos_log_pktlog_info) + sizeof(tAniNlHdr) >=
- skb_tailroom(gwlan_logging.pkt_stats_pcur_node->skb)) {
+ skb_tailroom(gwlan_logging.pkt_stats_pcur_node->skb)) {
+ ret = wlan_queue_pkt_stats_for_app(skb_to_free);
+ if (ret == -ENOMEM) {
+ spin_unlock_irqrestore(&gwlan_logging.pkt_stats_lock,
+ flags);
+ /* Print every 64 malloc fails */
+ if (!(gwlan_logging.malloc_fail_count % 64))
+ pr_err("%s: dev_alloc_skb() failed for msg size[%d]",
+ __func__, MAX_PKTSTATS_LOG_LENGTH);
+ return ret;
+ }
wake_up_thread = true;
- wlan_queue_pkt_stats_for_app();
}
ptr = gwlan_logging.pkt_stats_pcur_node->skb;
-
vos_mem_copy(skb_put(ptr, total_log_len), &rx_tx_stats, total_log_len);
spin_unlock_irqrestore(&gwlan_logging.pkt_stats_lock, flags);
+
+ /*
+ * If ret is non-zero and not ENOMEM, we have re-allocated from
+ * filled list, free the older skb and print every 64th drop count
+ */
+ if (ret) {
+ if (skb_to_free)
+ dev_kfree_skb(skb_to_free);
+
+ /* print every 64th drop count */
+ if (vos_is_multicast_logging() &&
+ (!(gwlan_logging.pkt_stat_drop_cnt % 0x40))) {
+ pr_err("%s: drop_count = %u\n",
+ __func__, gwlan_logging.pkt_stat_drop_cnt);
+ }
+ }
+
/* Wakeup logger thread */
if ((true == wake_up_thread)) {
/* If there is logger app registered wakeup the logging
@@ -554,11 +597,37 @@
void wlan_disable_and_flush_pkt_stats()
{
unsigned long flags;
+ int ret = 0;
+ struct sk_buff *skb_to_free = NULL;
+
+
spin_lock_irqsave(&gwlan_logging.pkt_stats_lock, flags);
if(gwlan_logging.pkt_stats_pcur_node->skb->len){
- wlan_queue_pkt_stats_for_app();
+ ret = wlan_queue_pkt_stats_for_app(skb_to_free);
}
spin_unlock_irqrestore(&gwlan_logging.pkt_stats_lock, flags);
+
+ /*
+ * If ret is non-zero and not ENOMEM, we have re-allocated from
+ * filled list, free the older skb and print every 64th drop count
+ */
+ if (ret == -ENOMEM) {
+ /* Print every 64 malloc fails */
+ if (!(gwlan_logging.malloc_fail_count % 64))
+ pr_err("%s: dev_alloc_skb() failed for msg size[%d]",
+ __func__, MAX_PKTSTATS_LOG_LENGTH);
+ } else if (ret) {
+ if (skb_to_free)
+ dev_kfree_skb(skb_to_free);
+
+ /* print every 64th drop count */
+ if (vos_is_multicast_logging() &&
+ (!(gwlan_logging.pkt_stat_drop_cnt % 0x40))) {
+ pr_err("%s: drop_count = %u\n",
+ __func__, gwlan_logging.pkt_stat_drop_cnt);
+ }
+ }
+
set_bit(HOST_PKT_STATS_POST, &gwlan_logging.event_flag);
wake_up_interruptible(&gwlan_logging.wait_queue);
}