From: Jean Delvare <jdelvare@xxxxxxx> Sent: Wednesday, April 17, 2024 10:34 AM > > Hi Michael, > > On Wed, 2024-04-17 at 15:43 +0000, Michael Kelley wrote: > > From: Jean Delvare <jdelvare@xxxxxxx> 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@xxxxxxx> > > > 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, > > 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. > > 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. > > > and perhaps also the offset of the header in the DMI blob (i.e., "data - buf"). > > 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. > > > When looking at the error reported by user space dmidecode, the first thing > > I did was add those fields to the error message. > > 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
