| Message ID | 20220726164628.1756924-1-toshi.kani@hpe.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] EDAC/ghes: Set the DIMM label unconditionally | expand |
On Tue, Jul 26, 2022 at 10:46:28AM -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(). ... > Fixes: b9cae27728d1f ("EDAC/ghes: Scan the system once on driver init") > Signed-off-by: Toshi Kani <toshi.kani@hpe.com> > Co-developed-by: Robert Richter <rric@kernel.org> > Tested-by: Robert Elliott <elliott@hpe.com> > --- > drivers/edac/ghes_edac.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) Amended and pushed out. Thx.
diff --git a/drivers/edac/ghes_edac.c b/drivers/edac/ghes_edac.c index 59b0bedc9c24..c229ed0ce678 100644 --- a/drivers/edac/ghes_edac.c +++ b/drivers/edac/ghes_edac.c @@ -103,9 +103,15 @@ 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); + /* + * Set a null string when both bank and device are zero. + * This keeps ghes_edac_register() preserving the default + * label from edac_mc_alloc_dimms(). + */ + snprintf(dimm->label, sizeof(dimm->label), "%s%s%s", + (bank && *bank) ? bank : "", + (bank && *bank && device && *device) ? " " : "", + (device && *device) ? device : ""); } static void assign_dmi_dimm_info(struct dimm_info *dimm, struct memdev_dmi_entry *entry)
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(). 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 dimm_setup_label() only initializes the label when both bank and device are set. This HPE BIOS only sets device since bank locator is unnecessary to locate a DIMM. 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 Change dimm_setup_label() to always initialize the label to fix the issue. It sets to a null string in case BIOS does not provide both bank and device so that ghes_edac_register() keeps the default label from edac_mc_alloc_dimms(). Fixes: b9cae27728d1f ("EDAC/ghes: Scan the system once on driver init") Signed-off-by: Toshi Kani <toshi.kani@hpe.com> Co-developed-by: Robert Richter <rric@kernel.org> Tested-by: Robert Elliott <elliott@hpe.com> --- drivers/edac/ghes_edac.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)