linux-kernel - RE: [PATCH] firmware: dmi: Stop decoding on broken entry

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]

Message-ID:
 <SN6PR02MB415755044A025D66B4BC8955D40F2@SN6PR02MB4157.namprd02.prod.outlook.com>
Date: Wed, 17 Apr 2024 15:43:49 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Jean Delvare <jdelvare@...e.de>, Michael Schierl <schierlm@....de>
CC: "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] firmware: dmi: Stop decoding on broken entry

From: Jean Delvare <jdelvare@...e.de> Sent: Wednesday, April 17, 2024 8:34 AM
> 
> If a DMI table entry is shorter than 4 bytes, it is invalid. Due to
> how DMI table parsing works, it is impossible to safely recover from
> such an error, so we have to stop decoding the table.
> 
> Signed-off-by: Jean Delvare <jdelvare@...e.de>
> Link: https://lore.kernel.org/linux-kernel/Zh2K3-HLXOesT_vZ@liuwe-devbox-debian-v2/T/
> ---
> Michael, can you please test this patch and confirm that it prevents
> the early oops?
> 
> The root cause of the DMI table corruption still needs to be
> investigated.
> 
>  drivers/firmware/dmi_scan.c |   11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> --- linux-6.8.orig/drivers/firmware/dmi_scan.c
> +++ linux-6.8/drivers/firmware/dmi_scan.c
> @@ -102,6 +102,17 @@ static void dmi_decode_table(u8 *buf,
>  		const struct dmi_header *dm = (const struct dmi_header *)data;
> 
>  		/*
> +		 * If a short entry is found (less than 4 bytes), not only it
> +		 * is invalid, but we cannot reliably locate the next entry.
> +		 */
> +		if (dm->length < sizeof(struct dmi_header)) {
> +			pr_warn(FW_BUG
> +				"Corrupted DMI table (only %d entries processed)\n",
> +				i);

It would be useful to also output the three header fields: type, handle, and length,
and perhaps also the offset of the header in the DMI blob (i.e., "data - buf").
When looking at the error reported by user space dmidecode, the first thing
I did was add those fields to the error message.

Michael 

> +			break;
> +		}
> +
> +		/*
>  		 *  We want to know the total length (formatted area and
>  		 *  strings) before decoding to make sure we won't run off the
>  		 *  table in dmi_decode or dmi_string
> 
> --
> Jean Delvare
> SUSE L3 Support