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

Mattijs Korpershoek
2026-01-06T09:37:57Z

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.