From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751580AbbCTI75 (ORCPT ); Fri, 20 Mar 2015 04:59:57 -0400 Received: from cantor2.suse.de ([195.135.220.15]:39726 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751128AbbCTI7v (ORCPT ); Fri, 20 Mar 2015 04:59:51 -0400 Date: Fri, 20 Mar 2015 09:59:47 +0100 From: Jean Delvare To: LKML Cc: Matt Fleming , Ard Biesheuvel , Ivan Khoronzhuk Subject: [PATCH] firmware: dmi_scan: Prevent dmi_num integer overflow Message-ID: <20150320095947.644f9c67@endymion.delvare> Organization: SUSE Linux X-Mailer: Claws Mail 3.10.1 (GTK+ 2.24.23; x86_64-suse-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org dmi_num is a u16, dmi_len is a u32, so this construct: dmi_num = dmi_len / 4; would result in an integer overflow for a DMI table larger than 256 kB. I've never see such a large table so far, but SMBIOS 3.0 makes it possible so maybe we'll see such tables in the future. So instead of faking a structure count when the entry point does not provide it, adjust the loop condition in dmi_table() to properly deal with the case where dmi_num is not set. Signed-off-by: Jean Delvare Cc: Matt Fleming Cc: Ard Biesheuvel Cc: Ivan Khoronzhuk --- drivers/firmware/dmi_scan.c | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) --- linux-4.0-rc4.orig/drivers/firmware/dmi_scan.c 2015-03-19 14:19:56.384426825 +0100 +++ linux-4.0-rc4/drivers/firmware/dmi_scan.c 2015-03-20 09:23:37.689542485 +0100 @@ -86,10 +86,13 @@ static void dmi_table(u8 *buf, u32 len, int i = 0; /* - * Stop when we see all the items the table claimed to have - * OR we run off the end of the table (also happens) + * Stop when we have seen all the items the table claimed to have + * (SMBIOS < 3.0 only) OR we reach an end-of-table marker OR we run + * off the end of the table (should never happen but sometimes does + * on bogus implementations.) */ - while ((i < num) && (data - buf + sizeof(struct dmi_header)) <= len) { + while ((!num || i < num) && + (data - buf + sizeof(struct dmi_header)) <= len) { const struct dmi_header *dm = (const struct dmi_header *)data; /* @@ -529,21 +532,10 @@ static int __init dmi_smbios3_present(co if (memcmp(buf, "_SM3_", 5) == 0 && buf[6] < 32 && dmi_checksum(buf, buf[6])) { dmi_ver = get_unaligned_be16(buf + 7); + dmi_num = 0; /* No longer specified */ dmi_len = get_unaligned_le32(buf + 12); dmi_base = get_unaligned_le64(buf + 16); - /* - * The 64-bit SMBIOS 3.0 entry point no longer has a field - * containing the number of structures present in the table. - * Instead, it defines the table size as a maximum size, and - * relies on the end-of-table structure type (#127) to be used - * to signal the end of the table. - * So let's define dmi_num as an upper bound as well: each - * structure has a 4 byte header, so dmi_len / 4 is an upper - * bound for the number of structures in the table. - */ - dmi_num = dmi_len / 4; - if (dmi_walk_early(dmi_decode) == 0) { pr_info("SMBIOS %d.%d present.\n", dmi_ver >> 8, dmi_ver & 0xFF); -- Jean Delvare SUSE L3 Support