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

[Tom Rini <trini@konsulko.com>]

[-- 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 --]