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  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 22 Jan 2019 16:09:12 +0000
From:   Ross Lagerwall <ross.lagerwall@...rix.com>
To:     <linux-kernel@...r.kernel.org>
CC:     <linux-acpi@...r.kernel.org>, <linux-efi@...r.kernel.org>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Len Brown <lenb@...nel.org>, Tony Luck <tony.luck@...el.com>,
        Borislav Petkov <bp@...en8.de>,
        Huang Ying <ying.huang@...el.com>,
        Ross Lagerwall <ross.lagerwall@...rix.com>
Subject: [PATCH 2/2] efi/cper: Avoid possible OOB when checking generic data block

When checking a generic status block, we iterate over all the generic
data blocks. The loop condition only checks that the start of the
generic data block is valid (within estatus->data_length) but not the
whole block. Because the size of data blocks (excluding error data) may
vary depending on the revision and the revision is contained within the
data block, ensure that enough of the current data block is valid before
dereferencing any members otherwise an OOB access may occur if
estatus->data_length is invalid. This relies on the fact that
struct acpi_hest_generic_data_v300 is a superset of the earlier version.
Also rework the other checks to avoid potential underflow.

Signed-off-by: Ross Lagerwall <ross.lagerwall@...rix.com>
---
 drivers/firmware/efi/cper.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index a7902fccdcfa..7cc18874b9d0 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -546,7 +546,7 @@ EXPORT_SYMBOL_GPL(cper_estatus_check_header);
 int cper_estatus_check(const struct acpi_hest_generic_status *estatus)
 {
 	struct acpi_hest_generic_data *gdata;
-	unsigned int data_len, gedata_len;
+	unsigned int data_len, record_len;
 	int rc;
 
 	rc = cper_estatus_check_header(estatus);
@@ -555,10 +555,12 @@ int cper_estatus_check(const struct acpi_hest_generic_status *estatus)
 	data_len = estatus->data_length;
 
 	apei_estatus_for_each_section(estatus, gdata) {
-		gedata_len = acpi_hest_get_error_length(gdata);
-		if (gedata_len > data_len - acpi_hest_get_size(gdata))
+		if (sizeof(struct acpi_hest_generic_data) > data_len)
 			return -EINVAL;
-		data_len -= acpi_hest_get_record_size(gdata);
+		record_len = acpi_hest_get_record_size(gdata);
+		if (record_len > data_len)
+			return -EINVAL;
+		data_len -= record_len;
 	}
 	if (data_len)
 		return -EINVAL;
-- 
2.17.2

Powered by blists - more mailing lists