From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andre Przywara <andre.przywara@arm.com>
Date: Wed, 20 Jan 2021 21:03:40 +0000
Subject: [scan-admin@coverity.com: New Defects reported by Coverity Scan
 for Das U-Boot]
In-Reply-To: <20210120190418.GA9782@bill-the-cat>
References: <20210120190418.GA9782@bill-the-cat>
Message-ID: <20210120210235.279a769f@slackpad.fritz.box>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

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 at coverity.com -----
> 
> Date: Mon, 18 Jan 2021 17:53:19 +0000 (UTC)
> From: scan-admin at coverity.com
> To: tom.rini at 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?

Cheers,
Andre

> 96     	strcpy((char *)header->string_pool, params->imagename);