qcacmn: Race condition while using pkt log buffer

There can be a race condition if two different threads use the
pkt log buffer at the same time. This issue can lead to Use-After-Free
of the packet log buffer.

To address this issue, protect the pktlog buffer access using spinlock.

Change-Id: I75d9375c9d85ac26dab1c06658d3f0fdbeb62935
CRs-Fixed: 2034486
This commit is contained in:
Ashish Kumar Dhanotiya
2017-04-17 17:17:37 +05:30
committed by Sandeep Puligilla
parent 8d88af5f32
commit 485d594ea1
2 changed files with 30 additions and 6 deletions

View File

@@ -519,12 +519,15 @@ static void pktlog_detach(struct ol_txrx_pdev_t *handle)
pl_info = pl_dev->pl_info; pl_info = pl_dev->pl_info;
remove_proc_entry(WLANDEV_BASENAME, g_pktlog_pde); remove_proc_entry(WLANDEV_BASENAME, g_pktlog_pde);
pktlog_sysctl_unregister(pl_dev); pktlog_sysctl_unregister(pl_dev);
pktlog_cleanup(pl_info);
spin_lock_bh(&pl_info->log_lock);
if (pl_info->buf) { if (pl_info->buf) {
pktlog_release_buf(txrx_pdev); pktlog_release_buf(txrx_pdev);
pl_dev->tgt_pktlog_alloced = false; pl_dev->tgt_pktlog_alloced = false;
} }
spin_unlock_bh(&pl_info->log_lock);
pktlog_cleanup(pl_info);
if (pl_dev) { if (pl_dev) {
kfree(pl_info); kfree(pl_info);
@@ -668,11 +671,16 @@ pktlog_read_proc_entry(char *buf, size_t nbytes, loff_t *ppos,
int rem_len; int rem_len;
int start_offset, end_offset; int start_offset, end_offset;
int fold_offset, ppos_data, cur_rd_offset, cur_wr_offset; int fold_offset, ppos_data, cur_rd_offset, cur_wr_offset;
struct ath_pktlog_buf *log_buf = pl_info->buf; struct ath_pktlog_buf *log_buf;
spin_lock_bh(&pl_info->log_lock);
log_buf = pl_info->buf;
*read_complete = false; *read_complete = false;
if (log_buf == NULL) { if (log_buf == NULL) {
*read_complete = true; *read_complete = true;
spin_unlock_bh(&pl_info->log_lock);
return 0; return 0;
} }
@@ -775,7 +783,6 @@ rd_done:
*ppos += ret_val; *ppos += ret_val;
if (ret_val == 0) { if (ret_val == 0) {
PKTLOG_LOCK(pl_info);
/* Write pointer might have been updated during the read. /* Write pointer might have been updated during the read.
* So, if some data is written into, lets not reset the pointers * So, if some data is written into, lets not reset the pointers
* We can continue to read from the offset position * We can continue to read from the offset position
@@ -789,9 +796,8 @@ rd_done:
pl_info->buf->offset = PKTLOG_READ_OFFSET; pl_info->buf->offset = PKTLOG_READ_OFFSET;
*read_complete = true; *read_complete = true;
} }
PKTLOG_UNLOCK(pl_info);
} }
spin_unlock_bh(&pl_info->log_lock);
return ret_val; return ret_val;
} }
@@ -811,16 +817,20 @@ pktlog_read(struct file *file, char *buf, size_t nbytes, loff_t *ppos)
if (!pl_info) if (!pl_info)
return 0; return 0;
spin_lock_bh(&pl_info->log_lock);
log_buf = pl_info->buf; log_buf = pl_info->buf;
if (log_buf == NULL) if (log_buf == NULL) {
spin_unlock_bh(&pl_info->log_lock);
return 0; return 0;
}
if (pl_info->log_state) { if (pl_info->log_state) {
/* Read is not allowed when write is going on /* Read is not allowed when write is going on
* When issuing cat command, ensure to send * When issuing cat command, ensure to send
* pktlog disable command first. * pktlog disable command first.
*/ */
spin_unlock_bh(&pl_info->log_lock);
return -EINVAL; return -EINVAL;
} }
@@ -837,11 +847,13 @@ pktlog_read(struct file *file, char *buf, size_t nbytes, loff_t *ppos)
if (*ppos < bufhdr_size) { if (*ppos < bufhdr_size) {
count = QDF_MIN((bufhdr_size - *ppos), rem_len); count = QDF_MIN((bufhdr_size - *ppos), rem_len);
spin_unlock_bh(&pl_info->log_lock);
if (copy_to_user(buf, ((char *)&log_buf->bufhdr) + *ppos, if (copy_to_user(buf, ((char *)&log_buf->bufhdr) + *ppos,
count)) count))
return -EFAULT; return -EFAULT;
rem_len -= count; rem_len -= count;
ret_val += count; ret_val += count;
spin_lock_bh(&pl_info->log_lock);
} }
start_offset = log_buf->rd_offset; start_offset = log_buf->rd_offset;
@@ -883,19 +895,23 @@ pktlog_read(struct file *file, char *buf, size_t nbytes, loff_t *ppos)
goto rd_done; goto rd_done;
count = QDF_MIN(rem_len, (end_offset - ppos_data + 1)); count = QDF_MIN(rem_len, (end_offset - ppos_data + 1));
spin_unlock_bh(&pl_info->log_lock);
if (copy_to_user(buf + ret_val, if (copy_to_user(buf + ret_val,
log_buf->log_data + ppos_data, count)) log_buf->log_data + ppos_data, count))
return -EFAULT; return -EFAULT;
ret_val += count; ret_val += count;
rem_len -= count; rem_len -= count;
spin_lock_bh(&pl_info->log_lock);
} else { } else {
if (ppos_data <= fold_offset) { if (ppos_data <= fold_offset) {
count = QDF_MIN(rem_len, (fold_offset - ppos_data + 1)); count = QDF_MIN(rem_len, (fold_offset - ppos_data + 1));
spin_unlock_bh(&pl_info->log_lock);
if (copy_to_user(buf + ret_val, if (copy_to_user(buf + ret_val,
log_buf->log_data + ppos_data, count)) log_buf->log_data + ppos_data, count))
return -EFAULT; return -EFAULT;
ret_val += count; ret_val += count;
rem_len -= count; rem_len -= count;
spin_lock_bh(&pl_info->log_lock);
} }
if (rem_len == 0) if (rem_len == 0)
@@ -907,11 +923,13 @@ pktlog_read(struct file *file, char *buf, size_t nbytes, loff_t *ppos)
if (ppos_data <= end_offset) { if (ppos_data <= end_offset) {
count = QDF_MIN(rem_len, (end_offset - ppos_data + 1)); count = QDF_MIN(rem_len, (end_offset - ppos_data + 1));
spin_unlock_bh(&pl_info->log_lock);
if (copy_to_user(buf + ret_val, if (copy_to_user(buf + ret_val,
log_buf->log_data + ppos_data, count)) log_buf->log_data + ppos_data, count))
return -EFAULT; return -EFAULT;
ret_val += count; ret_val += count;
rem_len -= count; rem_len -= count;
spin_lock_bh(&pl_info->log_lock);
} }
} }
@@ -922,6 +940,7 @@ rd_done:
} }
*ppos += ret_val; *ppos += ret_val;
spin_unlock_bh(&pl_info->log_lock);
return ret_val; return ret_val;
} }

View File

@@ -457,6 +457,7 @@ int pktlog_enable(struct hif_opaque_softc *scn, int32_t log_state,
} }
spin_lock_bh(&pl_info->log_lock);
pl_info->buf->bufhdr.version = CUR_PKTLOG_VER; pl_info->buf->bufhdr.version = CUR_PKTLOG_VER;
pl_info->buf->bufhdr.magic_num = PKTLOG_MAGIC_NUM; pl_info->buf->bufhdr.magic_num = PKTLOG_MAGIC_NUM;
pl_info->buf->wr_offset = 0; pl_info->buf->wr_offset = 0;
@@ -465,6 +466,7 @@ int pktlog_enable(struct hif_opaque_softc *scn, int32_t log_state,
pl_info->buf->bytes_written = 0; pl_info->buf->bytes_written = 0;
pl_info->buf->msg_index = 1; pl_info->buf->msg_index = 1;
pl_info->buf->offset = PKTLOG_READ_OFFSET; pl_info->buf->offset = PKTLOG_READ_OFFSET;
spin_unlock_bh(&pl_info->log_lock);
pl_info->start_time_thruput = os_get_timestamp(); pl_info->start_time_thruput = os_get_timestamp();
pl_info->start_time_per = pl_info->start_time_thruput; pl_info->start_time_per = pl_info->start_time_thruput;
@@ -542,12 +544,14 @@ int pktlog_setsize(struct hif_opaque_softc *scn, int32_t size)
return -EINVAL; return -EINVAL;
} }
spin_lock_bh(&pl_info->log_lock);
if (pl_info->buf != NULL) { if (pl_info->buf != NULL) {
if (pl_dev->is_pktlog_cb_subscribed && if (pl_dev->is_pktlog_cb_subscribed &&
wdi_pktlog_unsubscribe(pdev_txrx_handle, wdi_pktlog_unsubscribe(pdev_txrx_handle,
pl_info->log_state)) { pl_info->log_state)) {
pl_info->curr_pkt_state = PKTLOG_OPR_NOT_IN_PROGRESS; pl_info->curr_pkt_state = PKTLOG_OPR_NOT_IN_PROGRESS;
printk("Cannot unsubscribe pktlog from the WDI\n"); printk("Cannot unsubscribe pktlog from the WDI\n");
spin_unlock_bh(&pl_info->log_lock);
return -EFAULT; return -EFAULT;
} }
pktlog_release_buf(pdev_txrx_handle); pktlog_release_buf(pdev_txrx_handle);
@@ -560,6 +564,7 @@ int pktlog_setsize(struct hif_opaque_softc *scn, int32_t size)
pl_info->buf_size = size; pl_info->buf_size = size;
} }
pl_info->curr_pkt_state = PKTLOG_OPR_NOT_IN_PROGRESS; pl_info->curr_pkt_state = PKTLOG_OPR_NOT_IN_PROGRESS;
spin_unlock_bh(&pl_info->log_lock);
return 0; return 0;
} }