On Wed, Jan 20, 2021 at 09:03:40PM +0000, Andre Przywara wrote:
On Wed, 20 Jan 2021 14:04:18 -0500 Tom Rini trini@konsulko.com wrote:
Hi Tom,
I decided to run Coverity part-way through the merge window this time and here's what's been found so far.
Thanks for that!
----- Forwarded message from scan-admin@coverity.com -----
Date: Mon, 18 Jan 2021 17:53:19 +0000 (UTC) From: scan-admin@coverity.com To: tom.rini@gmail.com Subject: New Defects reported by Coverity Scan for Das U-Boot
Hi,
Please find the latest report on new defect(s) introduced to Das U-Boot found with Coverity Scan.
23 new defect(s) introduced to Das U-Boot found with Coverity Scan. 2 defect(s), reported by Coverity Scan earlier, were marked fixed in the recent build analyzed by Coverity Scan.
New defect(s) Reported-by: Coverity Scan Showing 20 of 23 defect(s)
** CID 316365: Memory - corruptions (STRING_OVERFLOW) /tools/sunxi_egon.c: 96 in egon_set_header()
*** CID 316365: Memory - corruptions (STRING_OVERFLOW) /tools/sunxi_egon.c: 96 in egon_set_header() 90 91 /* If an image name has been provided, use it as the DT name.*/ 92 if (params->imagename && params->imagename[0]) { 93 if (strlen(params->imagename) > sizeof(header->string_pool) - 1) 94 printf("WARNING: DT name too long for SPL header!\n"); 95 else {
CID 316365: Memory - corruptions (STRING_OVERFLOW) You might overrun the 13-character destination string"header->string_pool" by writing 51 characters from "params->imagename".
So this is a false report, as string_pool is 13 *words*: uint32_t string_pool[13]; And I explicitly used sizeof() to avoid any ambiguities here.
One could argue that this is at least misleading for a human reader, and a string pool should indeed be made of "char"s (which looks like indeed worth a patch), but the buffer is definitely 52 bytes long (and sizeof reports that). Not sure if that's worth reporting to Coverity, or we do just ignore it?
I'll mark it as false positive with your explanation above, thanks!