Received-SPF: pass (google.com: domain of linux-kernel+bounces-149038-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
From: Michael Kelley <mhklinux@outlook.com>
To: Jean Delvare <jdelvare@suse.de>, Michael Schierl <schierlm@gmx.de>
CC: "linux-hyperv@vger.kernel.org" <linux-hyperv@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH] firmware: dmi: Stop decoding on broken entry
Thread-Topic: [PATCH] firmware: dmi: Stop decoding on broken entry
Thread-Index: AQHakNyeQHU2XdbvxkyqvOE9tFpqPLFsmKbAgAAgboCAAAm3QA==
Date: Wed, 17 Apr 2024 18:12:34 +0000
Message-ID:
 <SN6PR02MB415762984B4CA784CA3B82D3D40F2@SN6PR02MB4157.namprd02.prod.outlook.com>
References: <b702b36b90b63b615d41e778570707043ea81551.camel@suse.de>
	 <SN6PR02MB415755044A025D66B4BC8955D40F2@SN6PR02MB4157.namprd02.prod.outlook.com>
 <a5cc2ca32f52f3dfce90e0a7e33656e3ee3a1a2e.camel@suse.de>
In-Reply-To: <a5cc2ca32f52f3dfce90e0a7e33656e3ee3a1a2e.camel@suse.de>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0:
 =?iso-8859-1?Q?wE+0XhXD8hIc1z/h3sBBjzi2ixqK1L1GflyfOtUH817SrUsq9I2SE0QrWx?=
 =?iso-8859-1?Q?NsRQ0Tie90reVPTogELPLNQYVYQ8lJnwDGfkfA+hEB1q8RGIC1whUOVz7k?=
 =?iso-8859-1?Q?3mdB5nWJhUCRbGhphD06UtzDn/vEVboiRqIAXQUSIBPFBSr7p4wW92uLoV?=
 =?iso-8859-1?Q?ivjVB7D1dwkKcEr1SUc6jmqY83WKqVVmDqaKgVWmnqJBQ4pnAkPugyKNn7?=
 =?iso-8859-1?Q?9XdnKpwFRNslZhTsywkicgLQ6KNyaWj7vmmfE8/f2ZeBIvUP5IiNVmOtP+?=
 =?iso-8859-1?Q?qT1f98CIeNLxFqLRsgZiGYtNM+OL+gZa5YXu6aauu2JnVpCgE64rh/ftKM?=
 =?iso-8859-1?Q?SRLQ9ZO8kmPooZ6ydCy4LFE3swES/3I0efHVgTmuXQw6mRysIQkWW8ttle?=
 =?iso-8859-1?Q?ytFQzwT5rjEbEkvSUZ93CPErdUTerylPJ/4WBIvK7O6z0aiRZagvhVnJIB?=
 =?iso-8859-1?Q?4EF2wHlBamd2UOt9nSe4ljeUoe7WI2lgpvfr+PBK7Nyc8rQiI649kJHi57?=
 =?iso-8859-1?Q?/Hx/e6pfqMhwHNRBuXKZ0faI79pzBBZ0Ksj/xvu+FhyKUDhQDOy881THa9?=
 =?iso-8859-1?Q?LxPNje9EsftKIDuaVe+W/FFfhhnASwu02L9KeO+cLfjZettuHZ/ngKyBL2?=
 =?iso-8859-1?Q?0s9xalkJxJuqvWxnndesZkKguxnm3bBgURREe3gYcOfIjkyEyC3uQAPtze?=
 =?iso-8859-1?Q?2OqdpYRkfhMrkY8zUvO4MuWUJCOeBDZsmYlaYywisf4DYcQuGnEPkQmqcx?=
 =?iso-8859-1?Q?z8CWqEyQGl2oap5YSuzUR4Q56Oai7Qg8Aeqkrb8kd074/p0tteqmuJiai4?=
 =?iso-8859-1?Q?Jmj5ZXnIqFCpsagZ5AxsF9JsWaIQ/rvhO4mDqhhbemsNJ0jMD+g460mtAT?=
 =?iso-8859-1?Q?td51angddBEqBwwMLkFK/wjwqGj9A9W2kDrS+h1kwRksh1htB+fkm6o9HO?=
 =?iso-8859-1?Q?We38MuguilSoHn1MlhQCE6oNVps3fcRWgrzqGIdQU+atTfIBM8w9rXkPJ4?=
 =?iso-8859-1?Q?JGV8IgqoHlewu+HAaI6N0Fo2c89ZYvg/4/3QmKbgax2t4nrbqJJ1XCR+Tw?=
 =?iso-8859-1?Q?WfBc9hugeC2YHjZm8LdSRA4tDnHNRssjAjmCxry0OQUYZMErczu7tEY2O/?=
 =?iso-8859-1?Q?yYVjlXavBPYDN8hXaZC1jWhMdnA9fVs8wU+5dbh0nYuLgS6qhhJkKDVGqh?=
 =?iso-8859-1?Q?+6V6EBGSa4k39/Tfr9WHGYS2wjgf7Ffn0XvztQv51Ay42OcKPGl4AT4Apx?=
 =?iso-8859-1?Q?yj9DL0g84uuJrx9+Tkaw=3D=3D?=
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-OriginatorOrg: outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SN6PR02MB4157.namprd02.prod.outlook.com
X-MS-Exchange-CrossTenant-RMS-PersistedConsumerOrg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-Network-Message-Id: b2282fb0-6f73-4b13-6d15-08dc5f09f50b
X-MS-Exchange-CrossTenant-rms-persistedconsumerorg: 00000000-0000-0000-0000-000000000000
X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Apr 2024 18:12:34.9655
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 84df9e7f-e9f6-40af-b435-aaaaaaaaaaaa
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR02MB8981

From: Jean Delvare <jdelvare@suse.de> Sent: Wednesday, April 17, 2024 10:34=
 AM
>=20
> Hi Michael,
>=20
> On Wed, 2024-04-17 at 15:43 +0000, Michael Kelley wrote:
> > From: Jean Delvare <jdelvare@suse.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@suse.de>
> > > Link: https://lore.kernel.org/linux-kernel/Zh2K3-HLXOesT_vZ@liuwe-dev=
box-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.
> > >
> > > =A0drivers/firmware/dmi_scan.c |=A0=A0 11 +++++++++++
> > > =A01 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,
> > > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0const struct dmi_head=
er *dm =3D (const struct dmi_header *)data;
> > >
> > > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0/*
> > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 * If a short entry is =
found (less than 4 bytes), not only it
> > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 * is invalid, but we c=
annot reliably locate the next entry.
> > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 */
> > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0if (dm->length < sizeof=
(struct dmi_header)) {
> > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0pr_warn(FW_BUG
> > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0=A0=A0=A0=A0=A0=A0=A0"Corrupted DMI table (only %d entries processed)=
\n",
> > > +=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=
=A0=A0=A0=A0=A0=A0=A0=A0=A0i);
> >
> > It would be useful to also output the three header fields: type, handle=
, and length,
>=20
> I object. The most likely cause for the length being wrong is memory
> corruption. We have no idea what caused it, nor what kind of data was
> written over the DMI table, so leaking the information to user-space
> doesn't sound like a good idea, even if it's only 4 bytes.
>=20
> Furthermore, the data in question is essentially useless anyway. It is
> likely to lead the person investigating the bug into the wrong
> direction by interpreting essentially random data as type, handle and
> length.
>=20
> > and perhaps also the offset of the header in the DMI blob (i.e., "data =
- buf").
>=20
> I could do that, as it isn't leaking any information, and this could be
> used to compute the memory address at which the corruption was
> detected, which is probably more useful than the number of the
> corrupted entry. Thanks for the suggestion.
>=20
> > When looking at the error reported by user space dmidecode, the first t=
hing
> > I did was add those fields to the error message.
>=20
> And this did not give you any further insight, did it?

Agreed.  The offset was probably more useful than the fields from the
header. With the offset, "hexdump /sys/firmware/dmi/tables/DMI"
shows what the bad data looks like.  So if you want to do only the offset,
I'm OK with that.

Michael