commit 75ee73a60e0218bde351222082941a0317657ace
author Khanjan Desai <khanjan@codeaurora.org> Wed Dec 16 16:41:41 2020 +0530
committer Khanjan Desai <khanjan@codeaurora.org> Wed Dec 16 18:03:36 2020 +0530
tree de7c865e8f74034565c2d10e800e6956ffba7a23
parent adbaa11414b43289a2276562cb88447f1e748921

WifiHal: Invalid length check for fw-diag msg

Due to invalid length in firmware message, the
ring buffer overflows.
Added check for length and returned error in
case of invalid length.

Change-Id: I0daeed2335f7fb7661f44bb119776979d51b906d
CRs-Fixed: 2837711

qcwcn/wifi_hal/ring_buffer.cpp

diff --git a/qcwcn/wifi_hal/ring_buffer.cpp b/qcwcn/wifi_hal/ring_buffer.cpp
index cd9865a..a568a73 100644
--- a/qcwcn/wifi_hal/ring_buffer.cpp
+++ b/qcwcn/wifi_hal/ring_buffer.cpp
@@ -160,7 +160,7 @@
                                      // write in current buffer
     unsigned int total_push_in_rd_ptr = 0; // Total amount of push in read pointer in this write
 
-    if (record_length > rbc->each_buf_size) {
+    if (record_length > rbc->each_buf_size || length > rbc->each_buf_size) {
         return RB_FAILURE;
     }
 
@@ -279,6 +279,17 @@
             }
         }
         rb_unlock(&rbc->rb_rw_lock);
+        if(rbc->bufs[rbc->wr_buf_no].data == NULL || (rbc->bufs[rbc->wr_buf_no].data + rbc->cur_wr_buf_idx) == NULL ||
+                buf == NULL || buf + bytes_written == NULL) {
+            ALOGE("The read or Write buffer is null");
+            return RB_FAILURE;
+        }
+        if (((bytes_written + cur_copy_len) > length
+                || (rbc->cur_wr_buf_idx + cur_copy_len) > rbc->each_buf_size)) {
+            ALOGE("LOG_RB rb_write overflow - cur_copy_len=%d wr_buf[max=%zu no=%d idx=%d] buf[max=%zu accessed=%d]",
+              cur_copy_len, rbc->each_buf_size, rbc->wr_buf_no, rbc->cur_wr_buf_idx, length, bytes_written + cur_copy_len);
+            return RB_FAILURE;
+        }
 
         /* don't use lock while doing memcpy, so that we don't block the read
          * context for too long. There is no harm while writing the memory if
@@ -476,7 +487,12 @@
             cur_read_len = rbc->cur_wr_buf_idx - rbc->cur_rd_buf_idx;
         } else {
             /* write is rolled over and just behind the read */
-            cur_read_len = rbc->bufs[rbc->rd_buf_no].last_wr_index - rbc->cur_rd_buf_idx;
+            if (rbc->bufs[rbc->rd_buf_no].last_wr_index >= rbc->cur_rd_buf_idx) {
+                cur_read_len = rbc->bufs[rbc->rd_buf_no].last_wr_index - rbc->cur_rd_buf_idx;
+            } else {
+                ALOGE("Alert: cur_read_len=%u invalid, rd_buf[no=%d rd_idx=%d wr_index=%d]",cur_read_len, rbc->rd_buf_no, rbc->cur_rd_buf_idx, rbc->bufs[rbc->rd_buf_no].last_wr_index);
+                return NULL;
+            }
         }
     } else {
         if (rbc->cur_rd_buf_idx == 0) {

qcwcn/wifi_hal/wifilogger_diag.cpp

diff --git a/qcwcn/wifi_hal/wifilogger_diag.cpp b/qcwcn/wifi_hal/wifilogger_diag.cpp
index 827e235..e2bb153 100644
--- a/qcwcn/wifi_hal/wifilogger_diag.cpp
+++ b/qcwcn/wifi_hal/wifilogger_diag.cpp
@@ -1009,6 +1009,11 @@
                 payloadlen = diag_msg_hdr->u.msg_hdr.payload_len;
                 hdr_size = sizeof(fw_diag_msg_hdr_t);
                 payload = diag_msg_hdr->payload;
+                if ((count + hdr_size + payloadlen) > length) {
+                    ALOGE("WLAN_DIAG_TYPE_MSG - possible buffer over access, length=%d count=%d hdr_size=%d payload len=%d",
+                           length, count, hdr_size, payloadlen);
+                    return WIFI_ERROR_UNKNOWN;
+                }
                 process_firmware_prints(info, (u8 *)diag_msg_fixed_hdr,
                                        payloadlen + hdr_size);
                 break;
@@ -1019,6 +1024,11 @@
                 payloadlen = diag_msg_hdr_v2->u.msg_hdr.payload_len;
                 hdr_size = sizeof(fw_diag_msg_hdr_v2_t);
                 payload = diag_msg_hdr_v2->payload;
+                if ((count + hdr_size + payloadlen) > length) {
+                    ALOGE("WLAN_DIAG_TYPE_MSG_V2 - possible buffer over access, length=%d count=%d hdr_size=%d payload len=%d",
+                           length, count, hdr_size, payloadlen);
+                    return WIFI_ERROR_UNKNOWN;
+                }
                 process_firmware_prints(info, (u8 *)diag_msg_fixed_hdr,
                                        payloadlen + hdr_size);
                 break;
@@ -1030,6 +1040,11 @@
                 payload = diag_msg_hdr->payload;
                 payloadlen = diag_msg_hdr->u.payload_len;
                 hdr_size = sizeof(fw_diag_msg_hdr_t);
+                if ((count + hdr_size + payloadlen) > length) {
+                    ALOGE("WLAN_DIAG_TYPE_CONFIG - possible buffer over access, length=%d count=%d hdr_size=%d payload len=%d",
+                           length, count, hdr_size, payloadlen);
+                    return WIFI_ERROR_UNKNOWN;
+                }
                 process_firmware_prints(info, (u8 *)diag_msg_hdr,
                                         payloadlen + hdr_size);
             }