linux-kernel - [PATCH] scsi: esas2r: reduce the risk of a possible buffer-overflow vulnerability caused by DMA failures/attacks in esas2r_process_vda

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Date:   Tue,  5 May 2020 01:24:12 +0800
From:   Jia-Ju Bai <baijiaju1990@...il.com>
To:     linuxdrivers@...otech.com, jejb@...ux.ibm.com,
        martin.petersen@...cle.com
Cc:     linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jia-Ju Bai <baijiaju1990@...il.com>
Subject: [PATCH] scsi: esas2r: reduce the risk of a possible buffer-overflow vulnerability caused by DMA failures/attacks in esas2r_process_vda_ioctl()

The function esas2r_read_vda() uses a DMA value "vi":

  struct atto_ioctl_vda *vi =
			(struct atto_ioctl_vda *)a->vda_buffer;

Then esas2r_read_vda() calls esas2r_process_vda_ioctl() with vi:
  esas2r_process_vda_ioctl(a, vi, rq, &sgc);

In esas2r_process_vda_ioctl(), the DMA value "vi->function" is 
used at many places, such as:
  if (vi->function >= vercnt)
  ...
  if (vi->version > esas2r_vdaioctl_versions[vi->function])
  ...

However, when DMA failures or attacks occur, the value of vi->function 
can be changed at any time. In this case, vi->function can be
first smaller than vercnt, and then it can be larger than vercnt when it
is used as the array index of esas2r_vdaioctl_versions, causing a
buffer-overflow vulnerability.

To avoid the risk of this vulnerability, vi->function is assigned to a 
non-DMA local variable at the beginning of esas2r_process_vda_ioctl(), 
and this variable replaces each use of vi->function in the function.

Signed-off-by: Jia-Ju Bai <baijiaju1990@...il.com>
---
 drivers/scsi/esas2r/esas2r_vda.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/esas2r/esas2r_vda.c b/drivers/scsi/esas2r/esas2r_vda.c
index 30028e56df63..c3a6811145cf 100644
--- a/drivers/scsi/esas2r/esas2r_vda.c
+++ b/drivers/scsi/esas2r/esas2r_vda.c
@@ -70,16 +70,17 @@ bool esas2r_process_vda_ioctl(struct esas2r_adapter *a,
 	u32 datalen = 0;
 	struct atto_vda_sge *firstsg = NULL;
 	u8 vercnt = (u8)ARRAY_SIZE(esas2r_vdaioctl_versions);
+	u8 function = vi->function;
 
 	vi->status = ATTO_STS_SUCCESS;
 	vi->vda_status = RS_PENDING;
 
-	if (vi->function >= vercnt) {
+	if (function >= vercnt) {
 		vi->status = ATTO_STS_INV_FUNC;
 		return false;
 	}
 
-	if (vi->version > esas2r_vdaioctl_versions[vi->function]) {
+	if (vi->version > esas2r_vdaioctl_versions[function]) {
 		vi->status = ATTO_STS_INV_VERSION;
 		return false;
 	}
@@ -89,14 +90,14 @@ bool esas2r_process_vda_ioctl(struct esas2r_adapter *a,
 		return false;
 	}
 
-	if (vi->function != VDA_FUNC_SCSI)
+	if (function != VDA_FUNC_SCSI)
 		clear_vda_request(rq);
 
-	rq->vrq->scsi.function = vi->function;
+	rq->vrq->scsi.function = function;
 	rq->interrupt_cb = esas2r_complete_vda_ioctl;
 	rq->interrupt_cx = vi;
 
-	switch (vi->function) {
+	switch (function) {
 	case VDA_FUNC_FLASH:
 
 		if (vi->cmd.flash.sub_func != VDA_FLASH_FREAD
-- 
2.17.1