Skip to content

Commit 942859d

Browse files
Villemoesardbiesheuvel
authored andcommitted
efi: cper: fix snprintf() use in cper_dimm_err_location()
snprintf() should be given the full buffer size, not one less. And it guarantees nul-termination, so doing it manually afterwards is pointless. It's even potentially harmful (though probably not in practice because CPER_REC_LEN is 256), due to the "return how much would have been written had the buffer been big enough" semantics. I.e., if the bank and/or device strings are long enough that the "DIMM location ..." output gets truncated, writing to msg[n] is a buffer overflow. Signed-off-by: Rasmus Villemoes <[email protected]> Fixes: 3760cd2 ("CPER: Adjust code flow of some functions") Signed-off-by: Ard Biesheuvel <[email protected]>
1 parent c4039b2 commit 942859d

File tree

1 file changed

+1
-3
lines changed

1 file changed

+1
-3
lines changed

drivers/firmware/efi/cper.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -276,8 +276,7 @@ static int cper_dimm_err_location(struct cper_mem_err_compact *mem, char *msg)
276276
if (!msg || !(mem->validation_bits & CPER_MEM_VALID_MODULE_HANDLE))
277277
return 0;
278278

279-
n = 0;
280-
len = CPER_REC_LEN - 1;
279+
len = CPER_REC_LEN;
281280
dmi_memdev_name(mem->mem_dev_handle, &bank, &device);
282281
if (bank && device)
283282
n = snprintf(msg, len, "DIMM location: %s %s ", bank, device);
@@ -286,7 +285,6 @@ static int cper_dimm_err_location(struct cper_mem_err_compact *mem, char *msg)
286285
"DIMM location: not present. DMI handle: 0x%.4x ",
287286
mem->mem_dev_handle);
288287

289-
msg[n] = '\0';
290288
return n;
291289
}
292290

0 commit comments

Comments
 (0)