Browse Source

qcacmn: Race condition while using pkt log buffer

There can be a race condition if the pktlog_buf inside pktlog APIs,
is accessed simultanously from two threads.
To prevent this use mutex in the caller functions of pktlog APIs.

Change-Id: Iea6e3cd28a7a347c1753fe71d0646fb43ee184fa
CRs-Fixed: 2047150
Ashish Kumar Dhanotiya 8 years ago
parent
commit
a55792d148
3 changed files with 108 additions and 9 deletions
  1. 1 0
      utils/pktlog/include/pktlog_ac_api.h
  2. 39 7
      utils/pktlog/linux_ac.c
  3. 68 2
      utils/pktlog/pktlog_ac.c

+ 1 - 0
utils/pktlog/include/pktlog_ac_api.h

@@ -98,6 +98,7 @@ struct ath_pktlog_info {
 	/* Size of buffer in bytes */
 	int32_t buf_size;
 	spinlock_t log_lock;
+	struct mutex pktlog_mutex;
 
 	/* Threshold of TCP SACK packets for triggered stop */
 	int sack_thr;

+ 39 - 7
utils/pktlog/linux_ac.c

@@ -128,6 +128,7 @@ int pktlog_alloc_buf(struct hif_opaque_softc *scn)
 	unsigned long vaddr;
 	struct page *vpg;
 	struct ath_pktlog_info *pl_info;
+	struct ath_pktlog_buf *buffer;
 	ol_txrx_pdev_handle pdev_txrx_handle;
 	pdev_txrx_handle = cds_get_context(QDF_MODULE_ID_TXRX);
 
@@ -143,25 +144,39 @@ int pktlog_alloc_buf(struct hif_opaque_softc *scn)
 
 	page_cnt = (sizeof(*(pl_info->buf)) + pl_info->buf_size) / PAGE_SIZE;
 
-	pl_info->buf = vmalloc((page_cnt + 2) * PAGE_SIZE);
-	if (pl_info->buf == NULL) {
+	spin_lock_bh(&pl_info->log_lock);
+	if (pl_info->buf != NULL) {
+		printk(PKTLOG_TAG "Buffer is already in use\n");
+		spin_unlock_bh(&pl_info->log_lock);
+		return -EINVAL;
+	}
+	spin_unlock_bh(&pl_info->log_lock);
+
+	buffer = vmalloc((page_cnt + 2) * PAGE_SIZE);
+	if (buffer == NULL) {
 		printk(PKTLOG_TAG
 		       "%s: Unable to allocate buffer "
 		       "(%d pages)\n", __func__, page_cnt);
 		return -ENOMEM;
 	}
 
-	pl_info->buf = (struct ath_pktlog_buf *)
-		       (((unsigned long)(pl_info->buf) + PAGE_SIZE - 1)
+	buffer = (struct ath_pktlog_buf *)
+		       (((unsigned long)(buffer) + PAGE_SIZE - 1)
 			& PAGE_MASK);
 
-	for (vaddr = (unsigned long)(pl_info->buf);
-	     vaddr < ((unsigned long)(pl_info->buf) + (page_cnt * PAGE_SIZE));
+	for (vaddr = (unsigned long)(buffer);
+	     vaddr < ((unsigned long)(buffer) + (page_cnt * PAGE_SIZE));
 	     vaddr += PAGE_SIZE) {
 		vpg = vmalloc_to_page((const void *)vaddr);
 		SetPageReserved(vpg);
 	}
 
+	spin_lock_bh(&pl_info->log_lock);
+	if (pl_info->buf != NULL)
+		pktlog_release_buf(pdev_txrx_handle);
+
+	pl_info->buf =  buffer;
+	spin_unlock_bh(&pl_info->log_lock);
 	return 0;
 }
 
@@ -200,6 +215,7 @@ static void pktlog_cleanup(struct ath_pktlog_info *pl_info)
 {
 	pl_info->log_state = 0;
 	PKTLOG_LOCK_DESTROY(pl_info);
+	mutex_destroy(&pl_info->pktlog_mutex);
 }
 
 /* sysctl procfs handler to enable pktlog */
@@ -802,7 +818,7 @@ rd_done:
 }
 
 static ssize_t
-pktlog_read(struct file *file, char *buf, size_t nbytes, loff_t *ppos)
+__pktlog_read(struct file *file, char *buf, size_t nbytes, loff_t *ppos)
 {
 	size_t bufhdr_size;
 	size_t count = 0, ret_val = 0;
@@ -944,6 +960,22 @@ rd_done:
 	return ret_val;
 }
 
+static ssize_t
+pktlog_read(struct file *file, char *buf, size_t nbytes, loff_t *ppos)
+{
+	size_t ret;
+	struct ath_pktlog_info *pl_info;
+
+	pl_info = (struct ath_pktlog_info *)
+					PDE_DATA(file->f_path.dentry->d_inode);
+	if (!pl_info)
+		return 0;
+	mutex_lock(&pl_info->pktlog_mutex);
+	ret = __pktlog_read(file, buf, nbytes, ppos);
+	mutex_unlock(&pl_info->pktlog_mutex);
+	return ret;
+}
+
 #ifndef VMALLOC_VMADDR
 #define VMALLOC_VMADDR(x) ((unsigned long)(x))
 #endif

+ 68 - 2
utils/pktlog/pktlog_ac.c

@@ -362,6 +362,7 @@ void pktlog_init(struct hif_opaque_softc *scn)
 
 	OS_MEMZERO(pl_info, sizeof(*pl_info));
 	PKTLOG_LOCK_INIT(pl_info);
+	mutex_init(&pl_info->pktlog_mutex);
 
 	pl_info->buf_size = PKTLOG_DEFAULT_BUFSIZE;
 	pl_info->buf = NULL;
@@ -387,7 +388,7 @@ void pktlog_init(struct hif_opaque_softc *scn)
 	PKTLOG_SW_EVENT_SUBSCRIBER.callback = pktlog_callback;
 }
 
-int pktlog_enable(struct hif_opaque_softc *scn, int32_t log_state,
+static int __pktlog_enable(struct hif_opaque_softc *scn, int32_t log_state,
 		 bool ini_triggered, uint8_t user_triggered,
 		 uint32_t is_iwpriv_command)
 {
@@ -505,7 +506,49 @@ int pktlog_enable(struct hif_opaque_softc *scn, int32_t log_state,
 	return 0;
 }
 
-int pktlog_setsize(struct hif_opaque_softc *scn, int32_t size)
+int pktlog_enable(struct hif_opaque_softc *scn, int32_t log_state,
+		 bool ini_triggered, uint8_t user_triggered,
+		 uint32_t is_iwpriv_command)
+{
+	struct ol_pktlog_dev_t *pl_dev;
+	struct ath_pktlog_info *pl_info;
+	struct ol_txrx_pdev_t *txrx_pdev;
+	int error;
+
+	if (!scn) {
+		printk("%s: Invalid scn context\n", __func__);
+		ASSERT(0);
+		return -EINVAL;
+	}
+
+	txrx_pdev = cds_get_context(QDF_MODULE_ID_TXRX);
+	if (!txrx_pdev) {
+		printk("%s: Invalid txrx_pdev context\n", __func__);
+		ASSERT(0);
+		return -EINVAL;
+	}
+
+	pl_dev = txrx_pdev->pl_dev;
+	if (!pl_dev) {
+		printk("%s: Invalid pktlog context\n", __func__);
+		ASSERT(0);
+		return -EINVAL;
+	}
+
+	pl_info = pl_dev->pl_info;
+
+	if (!pl_info)
+		return 0;
+
+
+	mutex_lock(&pl_info->pktlog_mutex);
+	error = __pktlog_enable(scn, log_state, ini_triggered,
+				user_triggered, is_iwpriv_command);
+	mutex_unlock(&pl_info->pktlog_mutex);
+	return error;
+}
+
+static int __pktlog_setsize(struct hif_opaque_softc *scn, int32_t size)
 {
 	ol_txrx_pdev_handle pdev_txrx_handle =
 		cds_get_context(QDF_MODULE_ID_TXRX);
@@ -568,6 +611,29 @@ int pktlog_setsize(struct hif_opaque_softc *scn, int32_t size)
 	return 0;
 }
 
+int pktlog_setsize(struct hif_opaque_softc *scn, int32_t size)
+{
+	int status;
+	ol_txrx_pdev_handle pdev_txrx_handle =
+		cds_get_context(QDF_MODULE_ID_TXRX);
+	struct ol_pktlog_dev_t *pl_dev;
+	struct ath_pktlog_info *pl_info;
+
+	if (pdev_txrx_handle == NULL ||
+			pdev_txrx_handle->pl_dev == NULL ||
+			pdev_txrx_handle->pl_dev->pl_info == NULL)
+		return -EFAULT;
+
+	pl_dev = pdev_txrx_handle->pl_dev;
+	pl_info = pl_dev->pl_info;
+
+	mutex_lock(&pl_info->pktlog_mutex);
+	status = __pktlog_setsize(scn, size);
+	mutex_unlock(&pl_info->pktlog_mutex);
+
+	return status;
+}
+
 int pktlog_clearbuff(struct hif_opaque_softc *scn, bool clear_buff)
 {
 	ol_txrx_pdev_handle pdev_txrx_handle =