linux-kernel - [PATCH] mcb: fix a missing-check bug

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>] [thread-next>] [day] [month] [year] [list]

Message-Id: <1539961894-11928-1-git-send-email-wang6495@umn.edu>
Date:   Fri, 19 Oct 2018 10:11:34 -0500
From:   Wenwen Wang <wang6495@....edu>
To:     Wenwen Wang <wang6495@....edu>
Cc:     Kangjie Lu <kjlu@....edu>,
        Johannes Thumshirn <morbidrsa@...il.com>,
        linux-kernel@...r.kernel.org (open list)
Subject: [PATCH] mcb: fix a missing-check bug

In chameleon_parse_cells(), to parse each cell, the descriptor type 'dtype'
is acquired from the IO memory region pointed by 'p' through readl() in
get_next_dtype(). Then 'dtype' is checked to see whether it is
CHAMELEON_DTYPE_GENERAL. If yes, chameleon_parse_gdd() is invoked to parse
Chameleon general device descriptor. In chameleon_parse_gdd(), the data in
the IO memory region is read again through readl() field by field.
Specifically, the 'reg1' field contains the type information. That means
the type is read twice. More importantly, no check is re-enforced after the
second read. Given that the IO memory region can also be accessed by the
device, it is possible that a malicious device controlled by an attacker
can modify the type information between the two reads. This can cause
undefined behavior of the kernel and introduce potential security risk.

This patch adds a necessary check after the second read to make sure the
descriptor type is CHAMELEON_DTYPE_GENERAL. Otherwise, an error code EINVAL
will be returned.

Signed-off-by: Wenwen Wang <wang6495@....edu>
---
 drivers/mcb/mcb-parse.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/mcb/mcb-parse.c b/drivers/mcb/mcb-parse.c
index 7369bda..f01a6c7 100644
--- a/drivers/mcb/mcb-parse.c
+++ b/drivers/mcb/mcb-parse.c
@@ -51,6 +51,10 @@ static int chameleon_parse_gdd(struct mcb_bus *bus,
 		return -ENOMEM;

 	reg1 = readl(&gdd->reg1);
+	if ((reg1 >> 28) != CHAMELEON_DTYPE_GENERAL) {
+		ret = -EINVAL;
+		goto err;
+	}
 	reg2 = readl(&gdd->reg2);
 	offset = readl(&gdd->offset);
 	size = readl(&gdd->size);
-- 
2.7.4