Re: Fwd: New Defects reported by Coverity Scan for Das U-Boot

Tom Rini
2025-04-30T17:01:47Z

[-- Attachment #1: Type: text/plain, Size: 3395 bytes --]

On Wed, Apr 30, 2025 at 06:50:50PM +0200, Marek Vasut wrote:
> On 4/28/25 11:59 PM, Tom Rini wrote:
> > ________________________________________________________________________________________________________
> > *** CID 550306: Control flow issues (DEADCODE)
> > /fs/exfat/io.c: 547 in exfat_generic_pwrite()
> > 541 int rc;
> > 542 cluster_t cluster;
> > 543 const char* bufp = buffer;
> > 544 off_t lsize, loffset, remainder;
> > 545
> > 546 if (offset < 0)
> > > > > CID 550306: Control flow issues (DEADCODE)
> > > > > Execution cannot reach this statement: "return -22L;".
> > 547 return -EINVAL;
> > 548 if (uoffset > node->size)
> > 549 {
> > 550 rc = exfat_truncate(ef, node, uoffset, true);
> > 551 if (rc != 0)
> > 552 return rc;
>
> This one is I think false positive, off_t can be signed integer, so the
> check should be in place.

I've updated Coverity with this comment.

> > ** CID 550305: Security best practices violations (STRING_OVERFLOW)
> > /fs/exfat/io.c: 739 in exfat_fs_opendir()
> >
> >
> > ________________________________________________________________________________________________________
> > *** CID 550305: Security best practices violations (STRING_OVERFLOW)
> > /fs/exfat/io.c: 739 in exfat_fs_opendir()
> > 733 return err;
> > 734
> > 735 dirs = calloc(1, sizeof(*dirs));
> > 736 if (!dirs)
> > 737 return -ENOMEM;
> > 738
> > > > > CID 550305: Security best practices violations (STRING_OVERFLOW)
> > > > > You might overrun the 1024-character fixed-size string
> > "dirs->dirname" by copying "filename" without checking the length.
> > 739 strcpy(dirs->dirname, filename);
> > 740 dirs->offset = -1;
> > 741
> > 742 *dirsp = &dirs->fs_dirs;
> > 743
> > 744 return 0;
> >
> > ** CID 550304: Error handling issues (NEGATIVE_RETURNS)
> > /tools/fit_check_sign.c: 98 in main()
>
> Fixed:
>
> https://patchwork.ozlabs.org/project/uboot/patch/20250430164559.27095-1-marex@denx.de/
>
> > ________________________________________________________________________________________________________
> > *** CID 550300: Integer handling issues (INTEGER_OVERFLOW)
> > /fs/exfat/utils.c: 146 in exfat_humanize_bytes()
> > 140 /* 16 EB (minus 1 byte) is the largest size that can be
> > represented by
> > 141 uint64_t */
> > 142 const char* units[] = {"bytes", "KB", "MB", "GB", "TB",
> > "PB", "EB"};
> > 143 uint64_t divisor = 1;
> > 144 uint64_t temp = 0;
> > 145
> > > > > CID 550300: Integer handling issues (INTEGER_OVERFLOW)
> > > > > Expression "divisor", overflows the type of "divisor", which is
> > type "uint64_t".
> > 146 for (i = 0; ; i++, divisor *= 1024)
> > 147 {
> > 148 temp = (value + divisor / 2) / divisor;
> > 149
> > 150 if (temp == 0)
> > 151 break;
> Fixed:
>
> https://patchwork.ozlabs.org/project/uboot/patch/20250430164559.27095-2-marex@denx.de/

Thanks!

--
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]