linux-kernel - Re: [PATCH AUTOSEL for 4.9 078/293] firmware: dmi

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

Date:   Mon, 9 Apr 2018 11:24:16 +0200
From:   Jean Delvare <jdelvare@...e.de>
To:     Sasha Levin <Alexander.Levin@...rosoft.com>
Cc:     stable@...r.kernel.org, linux-kernel@...r.kernel.org,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Andy Shevchenko <andy.shevchenko@...il.com>,
        Linus Walleij <linus.walleij@...aro.org>
Subject: Re: [PATCH AUTOSEL for 4.9 078/293] firmware: dmi_scan: Check DMI
 structure length

On Mon, 9 Apr 2018 00:23:55 +0000, Sasha Levin wrote:
> From: Jean Delvare <jdelvare@...e.de>
> 
> [ Upstream commit a814c3597a6b6040e2ef9459748081a6d5b7312d ]
> 
> Before accessing DMI data to record it for later, we should ensure
> that the DMI structures are large enough to contain the data in
> question.
> 
> Signed-off-by: Jean Delvare <jdelvare@...e.de>
> Reviewed-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
> Cc: Dmitry Torokhov <dmitry.torokhov@...il.com>
> Cc: Andy Shevchenko <andy.shevchenko@...il.com>
> Cc: Linus Walleij <linus.walleij@...aro.org>
> Signed-off-by: Sasha Levin <alexander.levin@...rosoft.com>
> ---
>  drivers/firmware/dmi_scan.c | 23 ++++++++++++++++-------
>  1 file changed, 16 insertions(+), 7 deletions(-)
> (...)
> @@ -191,13 +191,14 @@ static void __init dmi_save_ident(const struct dmi_header *dm, int slot,
>  static void __init dmi_save_uuid(const struct dmi_header *dm, int slot,
>  		int index)
>  {
> -	const u8 *d = (u8 *) dm + index;
> +	const u8 *d;
>  	char *s;
>  	int is_ff = 1, is_00 = 1, i;
>  
> -	if (dmi_ident[slot])
> +	if (dmi_ident[slot] || dm->length <= index + 16)

I'm afraid this check is off by one and nobody noticed :-( I'll send a
fix-up patch.

Probably harmless in practice as I have never seen a system with a DMI
type 1 structure of exactly 24 bytes (would be 8 bytes for very old
implementations and at least 25 for anything even remotely recent), but
still not good. Sorry about that.

>  		return;
>  
> +	d = (u8 *) dm + index;
>  	for (i = 0; i < 16 && (is_ff || is_00); i++) {
>  		if (d[i] != 0x00)
>  			is_00 = 0;

-- 
Jean Delvare
SUSE L3 Support