[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#1003906: memtest86+: buffer overflow in DMI data parsing, crash when displaying DMI memory info on larger PCs



Package: memtest86+
Version: 5.31b+dfsg-1
Severity: important
Tags: patch

Dear Maintainer,

When using the brand-new memtest86+ 5.31b package from Debian experimental 
on a server with 32 RAM slots, the program first displayed garbage 
manufacturer name, board name and CPU number at the bottom of the screen.
These pieces of information are extracted from DMI data, so I attempted to 
display the DMI information from the appropriate menu. The server 
displayed a popup containing... something... then it simply rebooted.

I went looking into the source code, in file dmi.c, and found an unbounded 
increment of the mem_devs_count and md_maps_count variables, which can 
lead to significant OOB writes (memory corruption) on computers which have 
more than MAX_DMI_MEMDEVS memory devices. That value is low, only 16. 
Granted, computers with > 16 RAM slots aren't one's average laptop / 
desktop computer, but recent dual-socket workstations and servers tend to 
have 24 or 32 RAM slots, and quad-socket servers easily have 32.

Then, I searched for memtest86+ forks on the Web, and found
https://github.com/anphsw/memtest86
where both the buffer overflow and the low value for MAX_DMI_MEMDEVS have 
been fixed since February 2018... This version also contains a number of 
other improvements over upstream: more tests, more supported memory 
controllers and SMBus controllers, etc. However, upstream has a number of 
typo fixes, etc.

I applied the relevant part of the fix locally, sprinkled 3 "else" into 
the if() chain because these are exclusive conditions, rebuilt and 
installed the package, and tested it:
* the manufacturer name, board name and (last) CPU number have become 
correct;
* the program displays DMI information for all 32 RAM slots, in 4 pages, 
without rebooting the server - as it should.

Two days ago, I contacted both the upstream maintainer and the 
aforementioned downstream maintainer about this matter.

memtest86+ 5.01b has the same bug, but not memtest86 4.3.7, which doesn't 
have DMI parsing code.


Regards,
Lionel Debroux.


-- System Information:
Debian Release: bookworm/sid
  APT prefers unstable
  APT policy: (500, 'unstable'), (1, 'experimental')
Architecture: amd64 (x86_64)
Foreign Architectures: i386, armhf, armel, arm64

[system information removed because not only the bug report is sent from 
another computer, but memtest86+ is freestanding, non-Linux software]

Versions of packages memtest86+ depends on:
ii  debconf [debconf-2.0]  1.5.79

memtest86+ recommends no packages.

Versions of packages memtest86+ suggests:
pn  grub-pc | grub-legacy  <none>
pn  hwtools                <none>
pn  kernel-patch-badram    <none>
pn  memtest86              <none>
pn  memtester              <none>
ii  mtools                 4.0.33-1+really4.0.32-1

-- debconf information:
  shared/memtest86-run-lilo: false
Author: Michael Brown <mcb30@ipxe.org>
Author: Lionel Debroux <lionel_debroux@yahoo.fr>
Description: make it possible to retrieve DMI memory information for 128 
memory devices (up from 16), and prevent crash/reboot-inducing buffer 
overflow in DMI memory information retrieval.

Via: https://github.com/anphsw/memtest86/commit/d15092a56940810b6e124d59c20c819e5e3e7df1
Via: https://github.com/anphsw/memtest86/commit/1144e231b5dad0e315ff834e4056cebafc4cba0b
---
diff -Naurp a/debian/patches/dmi-more-ram-slots-and-buffer-overflow-fix.patch b/debian/patches/dmi-more-ram-slots-and-buffer-overflow-fix.patch
--- a/debian/patches/dmi-more-ram-slots-and-buffer-overflow-fix.patch	1970-01-01 01:00:00.000000000 +0100
+++ b/debian/patches/dmi-more-ram-slots-and-buffer-overflow-fix.patch	2022-01-16 20:07:44.144559811 +0100
@@ -0,0 +1,41 @@
+diff -Naur memtest86+-5.31b+dfsg_orig/dmi.c memtest86+-5.31b+dfsg/dmi.c
+--- a/dmi.c	2020-04-12 17:14:51.000000000 +0200
++++ b/dmi.c	2022-01-16 20:05:32.245340025 +0100
+@@ -209,21 +209,21 @@
+ 	while(dmi < table_start + eps->tablelength){
+ 		struct tstruct_header *header = (struct tstruct_header *)dmi;
+ 		
+-		if (header->type == 17)
++		if (header->type == 17 && mem_devs_count < sizeof(mem_devs) / sizeof(mem_devs[0]))
+ 			mem_devs[mem_devs_count++] = (struct mem_dev *)dmi;
+ 		
+ 		// Mem Dev Map
+-		if (header->type == 20)
++		else if (header->type == 20 && md_maps_count < sizeof(md_maps) / sizeof(md_maps[0]))
+ 			md_maps[md_maps_count++] = (struct md_map *)dmi;
+ 
+ 		// MB_SPEC
+-		if (header->type == 2)
++		else if (header->type == 2)
+ 		{
+ 			dmi_system_info = (struct system_map *)dmi;
+ 		}
+ 
+ 		// CPU_SPEC
+-		if (header->type == 4)
++		else if (header->type == 4)
+ 		{
+ 			dmi_cpu_info = (struct cpu_map *)dmi;
+ 		}
+diff -Naur memtest86+-5.31b+dfsg_orig/test.h memtest86+-5.31b+dfsg/test.h
+--- a/test.h	2022-01-16 20:02:23.000000000 +0100
++++ b/test.h	2022-01-16 20:03:54.139841007 +0100
+@@ -53,7 +53,7 @@
+ 
+ #define DMI_SEARCH_START  0x0000F000
+ #define DMI_SEARCH_LENGTH 0x000F0FFF
+-#define MAX_DMI_MEMDEVS 16
++#define MAX_DMI_MEMDEVS 128
+ 
+ #define TITLE_WIDTH	 28
+ #define LINE_TITLE 		0
diff -Naurp a/debian/patches/series b/debian/patches/series
--- a/debian/patches/series	2022-01-16 20:06:36.789041768 +0100
+++ b/debian/patches/series	2022-01-16 20:06:42.692649698 +0100
@@ -8,3 +8,4 @@ make-iso-reproducible
 test-random-cflags.patch
 fix-gcc8-freeze-crash.patch
 discard-note_gnu_property.patch
+dmi-more-ram-slots-and-buffer-overflow-fix.patch

Reply to: