diag: Validate copying length against source buffer length
There a possibility of out-of-bound read because of not
validating source buffer length against length that about
to be copied. The patch adds proper check for validating
length before copying data
CRs-Fixed: 2163793
Change-Id: I7c93839d0c4d83024ce23a0ce494d09dd08567a9
Signed-off-by: Hardik Arya <harya@codeaurora.org>
diff --git a/drivers/char/diag/diag_dci.c b/drivers/char/diag/diag_dci.c
index 23cf293..a469eb9 100644
--- a/drivers/char/diag/diag_dci.c
+++ b/drivers/char/diag/diag_dci.c
@@ -1163,18 +1163,31 @@
struct list_head *start, *temp;
struct diag_dci_client_tbl *entry = NULL;
- length = *(uint16_t *)(buf + 1); /* total length of event series */
- if (length == 0) {
- pr_err("diag: Incoming dci event length is invalid\n");
+ if (!buf) {
+ pr_err("diag: In %s buffer is NULL\n", __func__);
return;
}
/*
- * Move directly to the start of the event series. 1 byte for
- * event code and 2 bytes for the length field.
+ * 1 byte for event code and 2 bytes for the length field.
* The length field indicates the total length removing the cmd_code
* and the length field. The event parsing in that case should happen
* till the end.
*/
+ if (len < 3) {
+ pr_err("diag: In %s invalid len: %d\n", __func__, len);
+ return;
+ }
+ length = *(uint16_t *)(buf + 1); /* total length of event series */
+ if ((length == 0) || (len != (length + 3))) {
+ pr_err("diag: Incoming dci event length: %d is invalid\n",
+ length);
+ return;
+ }
+ /*
+ * Move directly to the start of the event series.
+ * The event parsing should happen from start of event
+ * series till the end.
+ */
temp_len = 3;
while (temp_len < length) {
event_id_packet = *(uint16_t *)(buf + temp_len);
@@ -1191,30 +1204,60 @@
* necessary.
*/
timestamp_len = 8;
- memcpy(timestamp, buf + temp_len + 2, timestamp_len);
+ if ((temp_len + timestamp_len + 2) <= len)
+ memcpy(timestamp, buf + temp_len + 2,
+ timestamp_len);
+ else {
+ pr_err("diag: Invalid length in %s, len: %d, temp_len: %d",
+ __func__, len, temp_len);
+ return;
+ }
}
/* 13th and 14th bit represent the payload length */
if (((event_id_packet & 0x6000) >> 13) == 3) {
payload_len_field = 1;
- payload_len = *(uint8_t *)
+ if ((temp_len + timestamp_len + 3) <= len) {
+ payload_len = *(uint8_t *)
(buf + temp_len + 2 + timestamp_len);
- if (payload_len < (MAX_EVENT_SIZE - 13)) {
- /* copy the payload length and the payload */
+ } else {
+ pr_err("diag: Invalid length in %s, len: %d, temp_len: %d",
+ __func__, len, temp_len);
+ return;
+ }
+ if ((payload_len < (MAX_EVENT_SIZE - 13)) &&
+ ((temp_len + timestamp_len + payload_len + 3) <= len)) {
+ /*
+ * Copy the payload length and the payload
+ * after skipping temp_len bytes for already
+ * parsed packet, timestamp_len for timestamp
+ * buffer, 2 bytes for event_id_packet.
+ */
memcpy(event_data + 12, buf + temp_len + 2 +
timestamp_len, 1);
memcpy(event_data + 13, buf + temp_len + 2 +
timestamp_len + 1, payload_len);
} else {
- pr_err("diag: event > %d, payload_len = %d\n",
- (MAX_EVENT_SIZE - 13), payload_len);
+ pr_err("diag: event > %d, payload_len = %d, temp_len = %d\n",
+ (MAX_EVENT_SIZE - 13), payload_len, temp_len);
return;
}
} else {
payload_len_field = 0;
payload_len = (event_id_packet & 0x6000) >> 13;
- /* copy the payload */
- memcpy(event_data + 12, buf + temp_len + 2 +
+ /*
+ * Copy the payload after skipping temp_len bytes
+ * for already parsed packet, timestamp_len for
+ * timestamp buffer, 2 bytes for event_id_packet.
+ */
+ if ((payload_len < (MAX_EVENT_SIZE - 12)) &&
+ ((temp_len + timestamp_len + payload_len + 2) <= len))
+ memcpy(event_data + 12, buf + temp_len + 2 +
timestamp_len, payload_len);
+ else {
+ pr_err("diag: event > %d, payload_len = %d, temp_len = %d\n",
+ (MAX_EVENT_SIZE - 12), payload_len, temp_len);
+ return;
+ }
}
/* Before copying the data to userspace, check if we are still
@@ -1340,19 +1383,19 @@
pr_err("diag: In %s buffer is NULL\n", __func__);
return;
}
-
- /* The first six bytes for the incoming log packet contains
- * Command code (2), the length of the packet (2) and the length
- * of the log (2)
+ /*
+ * The first eight bytes for the incoming log packet contains
+ * Command code (2), the length of the packet (2), the length
+ * of the log (2) and log code (2)
*/
- log_code = *(uint16_t *)(buf + 6);
- read_bytes += sizeof(uint16_t) + 6;
- if (read_bytes > len) {
- pr_err("diag: Invalid length in %s, len: %d, read: %d",
- __func__, len, read_bytes);
+ if (len < 8) {
+ pr_err("diag: In %s invalid len: %d\n", __func__, len);
return;
}
+ log_code = *(uint16_t *)(buf + 6);
+ read_bytes += sizeof(uint16_t) + 6;
+
/* parse through log mask table of each client and check mask */
mutex_lock(&driver->dci_mutex);
list_for_each_safe(start, temp, &driver->dci_client_list) {
@@ -1379,6 +1422,10 @@
pr_err("diag: In %s buffer is NULL\n", __func__);
return;
}
+ if (len < (EXT_HDR_LEN + sizeof(uint8_t))) {
+ pr_err("diag: In %s invalid len: %d\n", __func__, len);
+ return;
+ }
version = *(uint8_t *)buf + 1;
if (version < EXT_HDR_VERSION) {
@@ -1390,10 +1437,6 @@
pkt = buf + EXT_HDR_LEN;
pkt_cmd_code = *(uint8_t *)pkt;
len -= EXT_HDR_LEN;
- if (len < 0) {
- pr_err("diag: %s, Invalid length len: %d\n", __func__, len);
- return;
- }
switch (pkt_cmd_code) {
case LOG_CMD_CODE: