From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752843AbbCZOrW (ORCPT ); Thu, 26 Mar 2015 10:47:22 -0400 Received: from mail-wi0-f179.google.com ([209.85.212.179]:37893 "EHLO mail-wi0-f179.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752391AbbCZOrU (ORCPT ); Thu, 26 Mar 2015 10:47:20 -0400 Date: Thu, 26 Mar 2015 14:47:17 +0000 From: Matt Fleming To: Jean Delvare Cc: LKML , Matt Fleming , Ard Biesheuvel , Ivan Khoronzhuk Subject: Re: [PATCH] firmware: dmi_scan: Prevent dmi_num integer overflow Message-ID: <20150326144717.GD6525@codeblueprint.co.uk> References: <20150320095947.644f9c67@endymion.delvare> <20150326130651.GC6525@codeblueprint.co.uk> <1427375705.4261.2.camel@chaos.site> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1427375705.4261.2.camel@chaos.site> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 26 Mar, at 02:15:05PM, Jean Delvare wrote: > > I don't actually have a tree, so feel free to pick it. OK will do, but this is a regression fix for a potential bug introduced in commit 6d9ff4733172 ("firmware: dmi_scan: Fix dmi_len type"), right? If so, it would be useful for this patch to make reference to that commit. How about this? --- >>From 8aabaf128c85bab02fdbe594481826e137c25da4 Mon Sep 17 00:00:00 2001 From: Jean Delvare Date: Fri, 20 Mar 2015 09:59:47 +0100 Subject: [PATCH] firmware: dmi_scan: Prevent dmi_num integer overflow 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. This potential overflow was introduced in commit 6d9ff4733172 ("firmware: dmi_scan: Fix dmi_len type"). Signed-off-by: Jean Delvare Cc: Matt Fleming Cc: Ivan Khoronzhuk Cc: Acked-by: Ard Biesheuvel Signed-off-by: Matt Fleming --- drivers/firmware/dmi_scan.c | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c index 69fac068669f..2eebd28b4c40 100644 --- a/drivers/firmware/dmi_scan.c +++ b/drivers/firmware/dmi_scan.c @@ -86,10 +86,13 @@ static void dmi_table(u8 *buf, u32 len, int num, 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(const u8 *buf) 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); -- 2.1.0 -- Matt Fleming, Intel Open Source Technology Center