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

Tom Rini
2026-01-06T17:15:24Z

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

On Tue, Jan 06, 2026 at 10:37:48AM +0100, Mattijs Korpershoek wrote:
> Hi Tom,
>
> On Mon, Jan 05, 2026 at 17:58, Tom Rini <trini@konsulko.com> wrote:
>
> > Hey all,
> >
> > Here's the latest report, now that next has been merged to master. A few
> > of these are oddly showing up now, despite being in older code that
> > hasn't been touched and was being built before.
>
> For fastboot, some code has been moved from mmc only support to
> fb_block.c, which might explain the new errors.
>
> See: https://lore.kernel.org/all/20251121-topic-fastboot-blk-v7-0-9589d902fc91@linaro.org/
>
> >
> > ---------- Forwarded message ---------
> > From: <scan-admin@coverity.com>
> > Date: Mon, Jan 5, 2026 at 3:24 PM
> > Subject: New Defects reported by Coverity Scan for Das U-Boot
> > To: <tom.rini@gmail.com>
> >
> >
> > Hi,
> >
> > Please find the latest report on new defect(s) introduced to *Das U-Boot*
> > found with Coverity Scan.
> >
> > - *New Defects Found:* 15
> > - 23 defect(s), reported by Coverity Scan earlier, were marked fixed in
> > the recent build analyzed by Coverity Scan.
> > - *Defects Shown:* Showing 15 of 15 defect(s)
> >
> > Defect Details
> >
> > ** CID 640423: Control flow issues (DEADCODE)
> > /drivers/fastboot/fb_common.c: 112 in fastboot_set_reboot_flag()
> >
> >
> > _____________________________________________________________________________________________
> > *** CID 640423: Control flow issues (DEADCODE)
> > /drivers/fastboot/fb_common.c: 112 in fastboot_set_reboot_flag()
> > 106 }
> > 107 const char *bcb_iface = config_opt_enabled(CONFIG_FASTBOOT_FLASH_BLOCK,
> > 108 CONFIG_FASTBOOT_FLASH_BLOCK_INTERFACE_NAME,
> > 109 "mmc");
> > 110
> > 111 if (device == -1)
> >>>> CID 640423: Control flow issues (DEADCODE)
> >>>> Execution cannot reach this statement: "return -22;".
>
> I believe coverity is wrong here.
> we call config_opt_enabled() which by default returns -1 so it's
> possible to have device == -1
>
> This can happen when both CONFIG_FASTBOOT_FLASH_BLOCK and
> CONFIG_FASTBOOT_FLASH_MMC are unset.
> (for example when we use CONFIG_FASTBOOT_FLASH_SPI)
>
> > 112 return -EINVAL;
> > 113
> > 114 if (reason >= FASTBOOT_REBOOT_REASONS_COUNT)
> > 115 return -EINVAL;
> > 116
> > 117 ret = bcb_find_partition_and_load(bcb_iface, device, "misc");
> >
>
> [...]
>
> >
> > ** CID 640421: Possible Control flow issues (DEADCODE)
> > /drivers/fastboot/fb_block.c: 138 in fastboot_block_get_part_info()
> >
> >
> > _____________________________________________________________________________________________
> > *** CID 640421: Possible Control flow issues (DEADCODE)
> > /drivers/fastboot/fb_block.c: 138 in fastboot_block_get_part_info()
> > 132 CONFIG_FASTBOOT_FLASH_BLOCK_DEVICE_ID, -1);
> > 133
> > 134 if (!part_name || !strcmp(part_name, "")) {
> > 135 fastboot_fail("partition not given", response);
> > 136 return -ENOENT;
> > 137 }
> >>>> CID 640421: Possible Control flow issues (DEADCODE)
> >>>> Execution cannot reach the expression "strcmp(interface, "")" inside this statement: "if (!interface || !strcmp(i...".
> > 138 if (!interface || !strcmp(interface, "")) {
> > 139 fastboot_fail("block interface isn't provided", response);
> > 140 return -EINVAL;
>
> I believe coverity is wrong here as well.
> we call config_opt_enabled() which by default returns NULL for interface.
>
> And when we enable CONFIG_FASTBOOT_FLASH_BLOCK,
> CONFIG_FASTBOOT_FLASH_BLOCK_INTERFACE_NAME will be set to "" by default:
>
> $ rg 'FASTBOOT_FLASH_BLOCK_INTERFACE_NAME' .config
> 1097:CONFIG_FASTBOOT_FLASH_BLOCK_INTERFACE_NAME=""
>
>
> > 141 }
> > 142
> > 143 *dev_desc = blk_get_dev(interface, device);
> >
>
> [...]
>
> >
> > ** CID 640419: Null pointer dereferences (REVERSE_INULL)
> > /drivers/fastboot/fb_block.c: 144 in fastboot_block_get_part_info()
> >
> >
> > _____________________________________________________________________________________________
> > *** CID 640419: Null pointer dereferences (REVERSE_INULL)
> > /drivers/fastboot/fb_block.c: 144 in fastboot_block_get_part_info()
> > 138 if (!interface || !strcmp(interface, "")) {
> > 139 fastboot_fail("block interface isn't provided", response);
> > 140 return -EINVAL;
> > 141 }
> > 142
> > 143 *dev_desc = blk_get_dev(interface, device);
> >>>> CID 640419: Null pointer dereferences (REVERSE_INULL)
> >>>> Null-checking "dev_desc" suggests that it may be null, but it has already been dereferenced on all paths leading to the check.
> > 144 if (!dev_desc) {
> > 145 fastboot_fail("no such device", response);
> > 146 return -ENODEV;
> > 147 }
>
> Fair enough for this one. We can check that dev_desc is not NULL to make
> sure that the caller cannot call fastboot_block_get_part_info() with
> NULL as second argument.
>
> I'll submit a patch for this once I've cleared out my review queue.
>
> > 148
> > 149 ret = part_get_info_by_name(*dev_desc, part_name, part_info);
> >
> >
>
> [...]
>
> For the first 2, do you want me to update the coverity database online
> with these explanations?
> It has been a while but I think I can do that myself.

Thanks for looking in to all of these. I've gone ahead and updated
Coverity, but in the future if you'd like to go in and do that while
composing the emails, please feel free.

--
Tom

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