Messages in this thread Patch in this message | | | Date | Fri, 22 Jul 2022 15:20:50 +0200 | From | Borislav Petkov <> | Subject | Re: [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
| |