lkml.org 
[lkml]   [2022]   [Jul]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH v2] EDAC/ghes: Fix buffer overflow in ghes_edac_register()
+ fix Robert's address.

On Thu, Jul 21, 2022 at 12:05:03PM -0600, Toshi Kani wrote:
> The following buffer overflow BUG was observed on an HPE system.
> ghes_edac_register() called strlen() on an uninitialized label,
> which had non-zero values from krealloc_array().

I ended up massaging it into this:

---
From: Toshi Kani <toshi.kani@hpe.com>
Date: Thu, 21 Jul 2022 12:05:03 -0600
Subject: [PATCH] EDAC/ghes: Set the DIMM label unconditionally

The commit in Fixes enforced that both the bank and device strings
passed to dimm_setup_label() are not NULL.

However, there are BIOSes, for example on a

HPE ProLiant DL360 Gen10/ProLiant DL360 Gen10, BIOS U32 03/15/2019

which don't populate both strings:

Handle 0x0020, DMI type 17, 84 bytes
Memory Device
Array Handle: 0x0013
Error Information Handle: Not Provided
Total Width: 72 bits
Data Width: 64 bits
Size: 32 GB
Form Factor: DIMM
Set: None
Locator: PROC 1 DIMM 1 <===== device
Bank Locator: Not Specified <===== bank

This results in a buffer overflow because ghes_edac_register() calls
strlen() on an uninitialized label, which had non-zero values left over
from krealloc_array():

detected buffer overflow in __fortify_strlen
------------[ cut here ]------------
kernel BUG at lib/string_helpers.c:983!
invalid opcode: 0000 [#1] PREEMPT SMP NOPTI
CPU: 1 PID: 1 Comm: swapper/0 Tainted: G I 5.18.6-200.fc36.x86_64 #1
Hardware name: HPE ProLiant DL360 Gen10/ProLiant DL360 Gen10, BIOS U32 03/15/2019
RIP: 0010:fortify_panic
...
Call Trace:
<TASK>
ghes_edac_register.cold
ghes_probe
platform_probe
really_probe
__driver_probe_device
driver_probe_device
__driver_attach
? __device_attach_driver
bus_for_each_dev
bus_add_driver
driver_register
acpi_ghes_init
acpi_init
? acpi_sleep_proc_init
do_one_initcall

Change dimm_setup_label() to always initialize the label and use
"N/A" in case bank or device is null.

[ bp: Rewrite commit message. ]

Fixes: cb51a371d08e ("EDAC/ghes: Setup DIMM label from DMI and use it in error reports")
Signed-off-by: Toshi Kani <toshi.kani@hpe.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Tested-by: Robert Elliott <elliott@hpe.com>
Link: https://lore.kernel.org/r/20220719220124.760359-1-toshi.kani@hpe.com
---
drivers/edac/ghes_edac.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c
index 59b0bedc9c24..fcab10e26b43 100644
--- a/drivers/edac/ghes_edac.c
+++ b/drivers/edac/ghes_edac.c
@@ -103,9 +103,9 @@ static void dimm_setup_label(struct dimm_info *dimm, u16 handle)

dmi_memdev_name(handle, &bank, &device);

- /* both strings must be non-zero */
- if (bank && *bank && device && *device)
- snprintf(dimm->label, sizeof(dimm->label), "%s %s", bank, device);
+ snprintf(dimm->label, sizeof(dimm->label), "%s %s",
+ (bank && *bank) ? bank : "N/A",
+ (device && *device) ? device : "N/A");
}

static void assign_dmi_dimm_info(struct dimm_info *dimm, struct memdev_dmi_entry *entry)
--
2.35.1
--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette

\
 
 \ /
  Last update: 2022-07-22 15:22    [W:0.105 / U:0.328 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site