public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* New Defects reported by Coverity Scan for Das U-Boot
       [not found] ` <c246b8e9-daea-389d-4437-9ce785a007e7@gmx.de>
@ 2021-03-24 22:00   ` Simon Glass
  0 siblings, 0 replies; 20+ messages in thread
From: Simon Glass @ 2021-03-24 22:00 UTC (permalink / raw)
  To: u-boot

+U-Boot Mailing List

On Wed, 3 Mar 2021 at 03:44, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 02.03.21 14:30, scan-admin at coverity.com wrote:
> > Hi,
> >
> > Please find the latest report on new defect(s) introduced to Das U-Boot found with Coverity Scan.
> >
> > 2 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> > 3 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 2 of 2 defect(s)
> >
> >
> > ** CID 325866:  Error handling issues  (CHECKED_RETURN)
> > /drivers/core/ofnode.c: 77 in ofnode_read_s32_default()
> >
> >
> > ________________________________________________________________________________________________________
> > *** CID 325866:  Error handling issues  (CHECKED_RETURN)
> > /drivers/core/ofnode.c: 77 in ofnode_read_s32_default()
> > 71            return def;
> > 72     }
> > 73
> > 74     int ofnode_read_s32_default(ofnode node, const char *propname, s32 def)
> > 75     {
> > 76            assert(ofnode_valid(node));
> >>>>     CID 325866:  Error handling issues  (CHECKED_RETURN)
> >>>>     Calling "ofnode_read_u32" without checking return value (as is done elsewhere 14 out of 17 times).
>
> This is a false positive. If the node is not found, def remains
> unchanged which matches the function description.
>
> > 77            ofnode_read_u32(node, propname, (u32 *)&def);
> > 78
> > 79            return def;
> > 80     }
> > 81
> > 82     int ofnode_read_u64(ofnode node, const char *propname, u64 *outp)
> >
> > ** CID 325865:  Memory - illegal accesses  (BUFFER_SIZE_WARNING)
> > /drivers/fastboot/fb_mmc.c: 64 in raw_part_get_info_by_name()
> >
> >
> > ________________________________________________________________________________________________________
> > *** CID 325865:  Memory - illegal accesses  (BUFFER_SIZE_WARNING)
> > /drivers/fastboot/fb_mmc.c: 64 in raw_part_get_info_by_name()
> > 58                    }
> > 59            }
> > 60
> > 61            info->start = simple_strtoul(argv[0], NULL, 0);
> > 62            info->size = simple_strtoul(argv[1], NULL, 0);
> > 63            info->blksz = dev_desc->blksz;
> >>>>     CID 325865:  Memory - illegal accesses  (BUFFER_SIZE_WARNING)
> >>>>     Calling "strncpy" with a maximum size argument of 32 bytes on destination array "info->name" of size 32 bytes might leave the destination string unterminated.
>
> Here we have a real issue.
>
> info->name is assumed to be null-terminated in different library calls,
> e.g. when calling part_get_info_by_name().
>
> Inside cmd/cpt.c we find the following conflicting lines:
>
> cmd/gpt.c:209:
> newpart->gpt_part_info.name[PART_NAME_LEN - 1] = '\0';
>
> cmd/gpt.c:842:
> if ((strlen(name1) > PART_NAME_LEN) || (strlen(name2) > PART_NAME_LEN)) {
>     printf("Names longer than %d characters are truncated.\n",
>
> So in line 209 we assume that PART_NAME_LEN includes the terminating NUL
> while in line 842 we assume that it does not.
>
> What needs to be done is:
>
> * document the structure field disk_partition.name so that it is
>   clear if this field is null-terminated or not
> * document the meaning of PART_NAME_LEN
> * check all usages of the field and PART_NAME_LEN
>
> Best regards
>
> Heinrich
>
>
> > 64            strncpy((char *)info->name, name, PART_NAME_LEN);
> > 65
> > 66            if (raw_part_desc) {
> > 67                    if (strcmp(strsep(&raw_part_desc, " "), "mmcpart") == 0) {
> > 68                            ulong mmcpart = simple_strtoul(raw_part_desc, NULL, 0);
> > 69                            int ret = blk_dselect_hwpart(dev_desc, mmcpart);
> >
> >
> > ________________________________________________________________________________________________________
> > To view the defects in Coverity Scan visit, https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yoA22WlOQ-2By3ieUvdbKmOyw68TMVT4Kip-2BBzfOGWXJ5yIiYplmPF9KAnKIja4Zd7tU-3DbjkX_N64QlSHam5hYYsLU0uvEm3xiMtcSlv2JwRoKVmjv-2F2W9scyTee7fb3OFpRv9kImrRqsKxlWoCO8zAeiIiAhqzSbrl4dXq9dXyAqi-2Fc0Jnwl-2FtH-2F6CeLIdaJ1B6nYgbrPrfQp-2FcoNWNRxS33O5yOY4dM-2FGF7XqrRr5G9AcX6O5K68VD-2FUnecqWgoMQ1p8zvxz5uSqy-2BRTvJPOAZAR5wWVKAbb5ohWYJh4aaJ24cyyKlc-3D
> >
> >   To manage Coverity Scan email notifications for "xypron.glpk at gmx.de", click https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yped04pjJnmXOsUBtKYNIXx4Y-2F1WK-2FIlbEOzfoxXLI-2FdwA0wwGn90rGGrBgiHW-2ByLDLbUOEV7XOvtc9zJmj9LPyrT06WSaMnNrm6wfrUN-2BXuWoaHdqOoEyL7CQlGSiE-2BfE-3DDif__N64QlSHam5hYYsLU0uvEm3xiMtcSlv2JwRoKVmjv-2F2W9scyTee7fb3OFpRv9kImr5eGADaWc9Gt2gE-2B5omGfwd2OEqtcwui0xgv2IURP-2BE-2BQ7i8p1lcSxhNpWKzf-2FstT1hBUmr8A-2Bondt-2Fcoj1lkWT-2BUp3IZnpnAzSzRNjIb0r85mSanNDe7kFVu6nNNGP4Gqpy1eEdxEVV3XbHRIfaxr0nF6YselCjEp-2BLdL5Rzfmg-3D
> >
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New Defects reported by Coverity Scan for Das U-Boot
       [not found] <618040194ba5b_1e5e522b0f269199b041648@prd-scan-dashboard-0.mail>
@ 2021-11-01 20:21 ` Heinrich Schuchardt
  0 siblings, 0 replies; 20+ messages in thread
From: Heinrich Schuchardt @ 2021-11-01 20:21 UTC (permalink / raw)
  To: Tom Rini; +Cc: U-Boot Mailing List

Hello Tom,

CID 340849:  Uninitialized variables  (UNINIT)
is invalid: If efi_allocate_pages fails, addr is not used.

CID 166730:  Integer handling issues  (SIGN_EXTENSION)
is invalid. u16 is first promoted to u32 (not int) and then shifted and
then promoted to u64.

Best regards

Heinrich

On 11/1/21 20:29, scan-admin@coverity.com wrote:
> Hi,
>
> Please find the latest report on new defect(s) introduced to Das U-Boot found with Coverity Scan.
>
> 10 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> 10 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 10 of 10 defect(s)
>
>
> ** CID 340850:  Control flow issues  (UNREACHABLE)
> /test/lib/abuf.c: 81 in lib_test_abuf_realloc()
>
>
> ________________________________________________________________________________________________________
> *** CID 340850:  Control flow issues  (UNREACHABLE)
> /test/lib/abuf.c: 81 in lib_test_abuf_realloc()
> 75     	/*
> 76     	 * TODO: crashes on sandbox sometimes due to an apparent bug in
> 77     	 * realloc().
> 78     	 */
> 79     	return 0;
> 80
>>>>      CID 340850:  Control flow issues  (UNREACHABLE)
>>>>      This code cannot be reached: "start = ut_check_free();".
> 81     	start = ut_check_free();
> 82
> 83     	abuf_init(&buf);
> 84
> 85     	/* Allocate an empty buffer */
> 86     	ut_asserteq(true, abuf_realloc(&buf, 0));
>
> ** CID 340849:  Uninitialized variables  (UNINIT)
> /lib/efi_loader/efi_boottime.c: 1991 in efi_load_image_from_path()
>
>
> ________________________________________________________________________________________________________
> *** CID 340849:  Uninitialized variables  (UNINIT)
> /lib/efi_loader/efi_boottime.c: 1991 in efi_load_image_from_path()
> 1985     					&buffer_size, (void *)(uintptr_t)addr));
> 1986     	if (ret != EFI_SUCCESS)
> 1987     		efi_free_pages(addr, pages);
> 1988     out:
> 1989     	EFI_CALL(efi_close_protocol(device, guid, efi_root, NULL));
> 1990     	if (ret == EFI_SUCCESS) {
>>>>      CID 340849:  Uninitialized variables  (UNINIT)
>>>>      Using uninitialized value "addr".
> 1991     		*buffer = (void *)(uintptr_t)addr;
> 1992     		*size = buffer_size;
> 1993     	}
> 1994
> 1995     	return ret;
> 1996     }
>
> ** CID 340848:  Control flow issues  (DEADCODE)
> /lib/rsa/rsa-sign.c: 255 in rsa_engine_get_priv_key()
>
>
> ________________________________________________________________________________________________________
> *** CID 340848:  Control flow issues  (DEADCODE)
> /lib/rsa/rsa-sign.c: 255 in rsa_engine_get_priv_key()
> 249     	} else if (engine_id) {
> 250     		if (keydir && name)
> 251     			snprintf(key_id, sizeof(key_id),
> 252     				 "%s%s",
> 253     				 keydir, name);
> 254     		else if (name)
>>>>      CID 340848:  Control flow issues  (DEADCODE)
>>>>      Execution cannot reach the expression """" inside this statement: "snprintf(key_id, 1024UL, "%...".
> 255     			snprintf(key_id, sizeof(key_id),
> 256     				 "%s",
> 257     				 name ? name : "");
> 258     		else if (keyfile)
> 259     			snprintf(key_id, sizeof(key_id), "%s", keyfile);
> 260     		else
>
> ** CID 340847:    (TAINTED_SCALAR)
>
>
> ________________________________________________________________________________________________________
> *** CID 340847:    (TAINTED_SCALAR)
> /lib/zstd/zstd.c: 49 in zstd_decompress()
> 43     	out_buf.pos = 0;
> 44     	out_buf.size = abuf_size(out);
> 45
> 46     	while (1) {
> 47     		size_t res;
> 48
>>>>      CID 340847:    (TAINTED_SCALAR)
>>>>      Passing tainted variable "dstream->inBuff" to a tainted sink.
> 49     		res = ZSTD_decompressStream(dstream, &out_buf, &in_buf);
> 50     		if (ZSTD_isError(res)) {
> 51     			ret = ZSTD_getErrorCode(res);
> 52     			log_err("ZSTD_decompressStream error %d\n", ret);
> 53     			goto do_free;
> 54     		}
> /lib/zstd/zstd.c: 49 in zstd_decompress()
> 43     	out_buf.pos = 0;
> 44     	out_buf.size = abuf_size(out);
> 45
> 46     	while (1) {
> 47     		size_t res;
> 48
>>>>      CID 340847:    (TAINTED_SCALAR)
>>>>      Passing tainted variable "in_buf.src" to a tainted sink.
> 49     		res = ZSTD_decompressStream(dstream, &out_buf, &in_buf);
> 50     		if (ZSTD_isError(res)) {
> 51     			ret = ZSTD_getErrorCode(res);
> 52     			log_err("ZSTD_decompressStream error %d\n", ret);
> 53     			goto do_free;
> 54     		}
>
> ** CID 340846:  Control flow issues  (UNREACHABLE)
> /test/lib/abuf.c: 144 in lib_test_abuf_large()
>
>
> ________________________________________________________________________________________________________
> *** CID 340846:  Control flow issues  (UNREACHABLE)
> /test/lib/abuf.c: 144 in lib_test_abuf_large()
> 138     	/*
> 139     	 * This crashes at present due to trying to allocate more memory than
> 140     	 * available, which breaks something on sandbox.
> 141     	 */
> 142     	return 0;
> 143
>>>>      CID 340846:  Control flow issues  (UNREACHABLE)
>>>>      This code cannot be reached: "start = ut_check_free();".
> 144     	start = ut_check_free();
> 145
> 146     	/* Try an impossible size */
> 147     	abuf_init(&buf);
> 148     	ut_asserteq(false, abuf_realloc(&buf, CONFIG_SYS_MALLOC_LEN));
> 149     	ut_assertnull(buf.data);
>
> ** CID 340845:  Control flow issues  (UNREACHABLE)
> /test/lib/abuf.c: 211 in lib_test_abuf_uninit_move()
>
>
> ________________________________________________________________________________________________________
> *** CID 340845:  Control flow issues  (UNREACHABLE)
> /test/lib/abuf.c: 211 in lib_test_abuf_uninit_move()
> 205     	 * TODO: crashes on sandbox sometimes due to an apparent bug in
> 206     	 * realloc().
> 207     	 */
> 208     	return 0;
> 209
> 210     	/* Move an empty buffer */
>>>>      CID 340845:  Control flow issues  (UNREACHABLE)
>>>>      This code cannot be reached: "abuf_init(&buf);".
> 211     	abuf_init(&buf);
> 212     	ut_assertnull(abuf_uninit_move(&buf, &size));
> 213     	ut_asserteq(0, size);
> 214     	ut_assertnull(abuf_uninit_move(&buf, NULL));
> 215
> 216     	/* Move an unallocated buffer */
>
> ** CID 340844:    (DEADCODE)
> /drivers/usb/gadget/ether.c: 2078 in eth_bind()
> /drivers/usb/gadget/ether.c: 2178 in eth_bind()
> /drivers/usb/gadget/ether.c: 2174 in eth_bind()
> /drivers/usb/gadget/ether.c: 2310 in eth_bind()
> /drivers/usb/gadget/ether.c: 2246 in eth_bind()
>
>
> ________________________________________________________________________________________________________
> *** CID 340844:    (DEADCODE)
> /drivers/usb/gadget/ether.c: 2078 in eth_bind()
> 2072     	 * needed to install MSFT drivers.  Current Linux kernels will use
> 2073     	 * the second configuration if it's CDC Ethernet, and need some help
> 2074     	 * to choose the right configuration otherwise.
> 2075     	 */
> 2076     	if (rndis) {
> 2077     #if defined(CONFIG_USB_GADGET_VENDOR_NUM) && defined(CONFIG_USB_GADGET_PRODUCT_NUM)
>>>>      CID 340844:    (DEADCODE)
>>>>      Execution cannot reach this statement: "device_desc.idVendor = 0;".
> 2078     		device_desc.idVendor =
> 2079     			__constant_cpu_to_le16(CONFIG_USB_GADGET_VENDOR_NUM);
> 2080     		device_desc.idProduct =
> 2081     			__constant_cpu_to_le16(CONFIG_USB_GADGET_PRODUCT_NUM);
> 2082     #else
> 2083     		device_desc.idVendor =
> /drivers/usb/gadget/ether.c: 2178 in eth_bind()
> 2172     	/* For now RNDIS is always a second config */
> 2173     	if (rndis)
> 2174     		device_desc.bNumConfigurations = 2;
> 2175
> 2176     	if (gadget_is_dualspeed(gadget)) {
> 2177     		if (rndis)
>>>>      CID 340844:    (DEADCODE)
>>>>      Execution cannot reach this statement: "dev_qualifier.bNumConfigura...".
> 2178     			dev_qualifier.bNumConfigurations = 2;
> 2179     		else if (!cdc)
> 2180     			dev_qualifier.bDeviceClass = USB_CLASS_VENDOR_SPEC;
> 2181
> 2182     		/* assumes ep0 uses the same value for both speeds ... */
> 2183     		dev_qualifier.bMaxPacketSize0 = device_desc.bMaxPacketSize0;
> /drivers/usb/gadget/ether.c: 2174 in eth_bind()
> 2168     	}
> 2169
> 2170     	usb_gadget_set_selfpowered(gadget);
> 2171
> 2172     	/* For now RNDIS is always a second config */
> 2173     	if (rndis)
>>>>      CID 340844:    (DEADCODE)
>>>>      Execution cannot reach this statement: "device_desc.bNumConfigurati...".
> 2174     		device_desc.bNumConfigurations = 2;
> 2175
> 2176     	if (gadget_is_dualspeed(gadget)) {
> 2177     		if (rndis)
> 2178     			dev_qualifier.bNumConfigurations = 2;
> 2179     		else if (!cdc)
> /drivers/usb/gadget/ether.c: 2310 in eth_bind()
> 2304     		printf("HOST MAC %02x:%02x:%02x:%02x:%02x:%02x\n",
> 2305     			dev->host_mac[0], dev->host_mac[1],
> 2306     			dev->host_mac[2], dev->host_mac[3],
> 2307     			dev->host_mac[4], dev->host_mac[5]);
> 2308
> 2309     	if (rndis) {
>>>>      CID 340844:    (DEADCODE)
>>>>      Execution cannot reach this statement: "vendorID = 0U;".
> 2310     		u32	vendorID = 0;
> 2311
> 2312     		/* FIXME RNDIS vendor id == "vendor NIC code" == ? */
> 2313
> 2314     		dev->rndis_config = rndis_register(rndis_control_ack);
> 2315     		if (dev->rndis_config < 0) {
> /drivers/usb/gadget/ether.c: 2246 in eth_bind()
> 2240     	sprintf(ethaddr, "%02X%02X%02X%02X%02X%02X",
> 2241     		dev->host_mac[0], dev->host_mac[1],
> 2242     			dev->host_mac[2], dev->host_mac[3],
> 2243     			dev->host_mac[4], dev->host_mac[5]);
> 2244
> 2245     	if (rndis) {
>>>>      CID 340844:    (DEADCODE)
>>>>      Execution cannot reach this statement: "status = rndis_init();".
> 2246     		status = rndis_init();
> 2247     		if (status < 0) {
> 2248     			pr_err("can't init RNDIS, %d", status);
> 2249     			goto fail;
> 2250     		}
> 2251     	}
>
> ** CID 340843:  Control flow issues  (UNREACHABLE)
> /test/lib/abuf.c: 315 in lib_test_abuf_init_move()
>
>
> ________________________________________________________________________________________________________
> *** CID 340843:  Control flow issues  (UNREACHABLE)
> /test/lib/abuf.c: 315 in lib_test_abuf_init_move()
> 309     	/*
> 310     	 * TODO: crashes on sandbox sometimes due to an apparent bug in
> 311     	 * realloc().
> 312     	 */
> 313     	return 0;
> 314
>>>>      CID 340843:  Control flow issues  (UNREACHABLE)
>>>>      This code cannot be reached: "ptr = sandbox_strdup(test_d...".
> 315     	ptr = strdup(test_data);
> 316     	ut_assertnonnull(ptr);
> 317
> 318     	free(ptr);
> 319
> 320     	abuf_init_move(&buf, ptr, TEST_DATA_LEN);
>
> ** CID 312933:  Error handling issues  (CHECKED_RETURN)
> /net/mdio-uclass.c: 33 in dm_mdio_probe_devices()
>
>
> ________________________________________________________________________________________________________
> *** CID 312933:  Error handling issues  (CHECKED_RETURN)
> /net/mdio-uclass.c: 33 in dm_mdio_probe_devices()
> 27
> 28     void dm_mdio_probe_devices(void)
> 29     {
> 30     	struct udevice *it;
> 31     	struct uclass *uc;
> 32
>>>>      CID 312933:  Error handling issues  (CHECKED_RETURN)
>>>>      Calling "uclass_get" without checking return value (as is done elsewhere 58 out of 72 times).
> 33     	uclass_get(UCLASS_MDIO, &uc);
> 34     	uclass_foreach_dev(it, uc) {
> 35     		device_probe(it);
> 36     	}
> 37     }
> 38
>
> ** CID 166730:  Integer handling issues  (SIGN_EXTENSION)
> /drivers/nvme/nvme.c: 786 in nvme_blk_rw()
>
>
> ________________________________________________________________________________________________________
> *** CID 166730:  Integer handling issues  (SIGN_EXTENSION)
> /drivers/nvme/nvme.c: 786 in nvme_blk_rw()
> 780     		c.rw.prp2 = cpu_to_le64(prp2);
> 781     		status = nvme_submit_sync_cmd(dev->queues[NVME_IO_Q],
> 782     				&c, NULL, IO_TIMEOUT);
> 783     		if (status)
> 784     			break;
> 785     		temp_len -= (u32)lbas << ns->lba_shift;
>>>>      CID 166730:  Integer handling issues  (SIGN_EXTENSION)
>>>>      Suspicious implicit sign extension: "lbas" with type "u16" (16 bits, unsigned) is promoted in "lbas << ns->lba_shift" to type "int" (32 bits, signed), then sign-extended to type "unsigned long" (64 bits, unsigned).  If "lbas << ns->lba_shift" is greater than 0x7FFFFFFF, the upper bits of the result will all be 1.
> 786     		temp_buffer += lbas << ns->lba_shift;
> 787     	}
> 788
> 789     	if (read)
> 790     		invalidate_dcache_range((unsigned long)buffer,
> 791     					(unsigned long)buffer + total_len);
>
>
> ________________________________________________________________________________________________________
> To view the defects in Coverity Scan visit, https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yoA22WlOQ-2By3ieUvdbKmOyw68TMVT4Kip-2BBzfOGWXJ5yIiYplmPF9KAnKIja4Zd7tU-3DZsDS_N64QlSHam5hYYsLU0uvEm3xiMtcSlv2JwRoKVmjv-2F2UsMGDkb6QQ9zv03O-2B521th4jk9hdxmyjqr4mvO8TNNoh0FnQ-2B5N3U5DGzMq2yk1UZZ-2FQb1oOcWdWOfY78ZlgiVwleQahFPDPcwRvW6D61sR497IHf99iJnYLg00Ftzy7iWuIa28dd2x3FHtb4iktmmQnx-2FyuscxPEBjMTurr2nmw-3D-3D
>
>    To manage Coverity Scan email notifications for "xypron.glpk@gmx.de", click https://u15810271.ct.sendgrid.net/ls/click?upn=HRESupC-2F2Czv4BOaCWWCy7my0P0qcxCbhZ31OYv50yped04pjJnmXOsUBtKYNIXx4Y-2F1WK-2FIlbEOzfoxXLI-2FdwA0wwGn90rGGrBgiHW-2ByLDLbUOEV7XOvtc9zJmj9LPyrT06WSaMnNrm6wfrUN-2BXuWoaHdqOoEyL7CQlGSiE-2BfE-3D8EDp_N64QlSHam5hYYsLU0uvEm3xiMtcSlv2JwRoKVmjv-2F2UsMGDkb6QQ9zv03O-2B521thJABpoyXzmILBz-2BmBPIJrfwYv1VTyAhy-2B9qTGTR8xpLaJ-2FMpjceXc35Vn8wZ1WXx-2BK2Clwq4JsG5Hq1xEX0r8P-2FIujbH5BmoWs4V889wI4hYkm9RxslrZI3cXv39AA01GmDd-2F7x5qGQhqwowNrPodNg-3D-3D
>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New Defects reported by Coverity Scan for Das U-Boot
  2024-01-19  8:47 ` Fwd: " Heinrich Schuchardt
@ 2024-01-22  6:44   ` Masahisa Kojima
  0 siblings, 0 replies; 20+ messages in thread
From: Masahisa Kojima @ 2024-01-22  6:44 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: U-Boot Mailing List, Ilias Apalodimas

Hi Heinrich,

On Fri, 19 Jan 2024 at 17:47, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
>
>
> ________________________________________________________________________________________________________
> *** CID 478333:  Error handling issues  (CHECKED_RETURN)
> /lib/efi_loader/efi_firmware.c: 413 in efi_firmware_set_fmp_state_var()
> 407             /*
> 408              * GetVariable may fail, EFI_NOT_FOUND is returned if FmpState
> 409              * variable has not been set yet.
> 410              * Ignore the error here since the correct FmpState variable
> 411              * is set later.
> 412              */
> >>>     CID 478333:  Error handling issues  (CHECKED_RETURN)
> >>>     Calling "efi_get_variable_int" without checking return value (as is done elsewhere 29 out of 33 times).
> 413             efi_get_variable_int(varname, image_type_id, NULL, &size,
> var_state,
> 414                                  NULL);
> 415     416             /*
> 417              * Only the fw_version is set here.
> 418              * lowest_supported_version in FmpState variable is ignored since
>
> There are a lot of different return values that may occur when calling
> efi_get_variable_int, e.g.
>
> * EFI_BUFFER_TOO_SMALL
> * EFI_DEVICE_ERROR
>
> Should we overwrite the variable in these cases with NUL values except
> for var_state[update_bank].fw_version?

The var_state buffer is allocated by calloc(), and efi_get_variable_int()
will not update the buffer in case of error.
But it is better to set NUL values to var_state, I will send a fix.

Thanks,
Masahisa Kojima

>
> Best regards
>
> Heinrich

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New Defects reported by Coverity Scan for Das U-Boot
  2024-04-22 21:48 Fwd: " Tom Rini
@ 2024-04-23  6:19 ` Ilias Apalodimas
  0 siblings, 0 replies; 20+ messages in thread
From: Ilias Apalodimas @ 2024-04-23  6:19 UTC (permalink / raw)
  To: Tom Rini; +Cc: u-boot, Charles Hardin

Hi Tom,

Thanks! Already sent a fix for UEFI

On Tue, 23 Apr 2024 at 00:48, Tom Rini <trini@konsulko.com> wrote:
>
> Here's the latest report.
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Apr 22, 2024 at 3:23 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.
>
> 2 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> 7 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 2 of 2 defect(s)
>
>
> ** CID 492766:  Control flow issues  (DEADCODE)
> /lib/efi_loader/efi_var_mem.c: 236 in efi_var_mem_init()
>
>
> ________________________________________________________________________________________________________
> *** CID 492766:  Control flow issues  (DEADCODE)
> /lib/efi_loader/efi_var_mem.c: 236 in efi_var_mem_init()
> 230             memset(efi_var_buf, 0, EFI_VAR_BUF_SIZE);
> 231             efi_var_buf->magic = EFI_VAR_FILE_MAGIC;
> 232             efi_var_buf->length = (uintptr_t)efi_var_buf->var -
> 233                                   (uintptr_t)efi_var_buf;
> 234
> 235             if (ret != EFI_SUCCESS)
> >>>     CID 492766:  Control flow issues  (DEADCODE)
> >>>     Execution cannot reach this statement: "return ret;".
> 236                     return ret;
> 237             ret =
> efi_create_event(EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE, TPL_CALLBACK,
> 238
> efi_var_mem_notify_virtual_address_map, NULL,
> 239                                    NULL, &event);
> 240             if (ret != EFI_SUCCESS)
> 241                     return ret;
>
> ** CID 492765:  Uninitialized variables  (UNINIT)
>
>
> ________________________________________________________________________________________________________
> *** CID 492765:  Uninitialized variables  (UNINIT)
> /net/bootp.c: 888 in dhcp_process_options()
> 882                             net_root_path[size] = 0;
> 883                             break;
> 884                     case 28:        /* Ignore Broadcast Address Option */
> 885                             break;
> 886                     case 40:        /* NIS Domain name */
> 887                             if (net_nis_domain[0] == 0) {
> >>>     CID 492765:  Uninitialized variables  (UNINIT)
> >>>     Using uninitialized value "size" when calling "truncate_sz".
> 888                                     size = truncate_sz("NIS Domain Name",
> 889                                             sizeof(net_nis_domain), size);
> 890                                     memcpy(&net_nis_domain, popt + 2, size);
> 891                                     net_nis_domain[size] = 0;
> 892                             }
> 893                             break;
>
>
> --
> Tom

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Fwd: New Defects reported by Coverity Scan for Das U-Boot
@ 2024-10-07 17:15 Tom Rini
  2024-10-07 18:17 ` Richard Weinberger
  0 siblings, 1 reply; 20+ messages in thread
From: Tom Rini @ 2024-10-07 17:15 UTC (permalink / raw)
  To: u-boot
  Cc: Simon Glass, Joao Marcos Costa, Thomas Petazzoni,
	Jerome Forissier, Sughosh Ganu, Caleb Connolly,
	Richard Weinberger

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

Now that I've merged next to master, there's a number of issues to
address.

---------- Forwarded message ---------
From: <scan-admin@coverity.com>
Date: Mon, Oct 7, 2024 at 10:59 AM
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.

24 new defect(s) introduced to Das U-Boot found with Coverity Scan.
9 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 24 defect(s)


** CID 510469:    (RESOURCE_LEAK)
/tools/mkeficapsule.c: 877 in load_dtb()
/tools/mkeficapsule.c: 862 in load_dtb()
/tools/mkeficapsule.c: 855 in load_dtb()
/tools/mkeficapsule.c: 870 in load_dtb()


________________________________________________________________________________________________________
*** CID 510469:    (RESOURCE_LEAK)
/tools/mkeficapsule.c: 877 in load_dtb()
871             }
872
873             if (fread(dtb, dtb_size, 1, f) != 1) {
874                     fprintf(stderr, "Can't read %ld bytes from %s\n",
875                             dtb_size, path);
876                     free(dtb);
>>>     CID 510469:    (RESOURCE_LEAK)
>>>     Variable "f" going out of scope leaks the storage it points to.
877                     return NULL;
878             }
879
880             fclose(f);
881
882             return dtb;
/tools/mkeficapsule.c: 862 in load_dtb()
856             }
857
858             dtb_size = ftell(f);
859             if (dtb_size < 0) {
860                     fprintf(stderr, "Cannot ftell %s: %s\n",
861                             path, strerror(errno));
>>>     CID 510469:    (RESOURCE_LEAK)
>>>     Variable "f" going out of scope leaks the storage it points to.
862                     return NULL;
863             }
864
865             fseek(f, 0, SEEK_SET);
866
867             dtb = malloc(dtb_size);
/tools/mkeficapsule.c: 855 in load_dtb()
849                     return NULL;
850             }
851
852             if (fseek(f, 0, SEEK_END)) {
853                     fprintf(stderr, "Cannot seek to the end of %s: %s\n",
854                             path, strerror(errno));
>>>     CID 510469:    (RESOURCE_LEAK)
>>>     Variable "f" going out of scope leaks the storage it points to.
855                     return NULL;
856             }
857
858             dtb_size = ftell(f);
859             if (dtb_size < 0) {
860                     fprintf(stderr, "Cannot ftell %s: %s\n",
/tools/mkeficapsule.c: 870 in load_dtb()
864
865             fseek(f, 0, SEEK_SET);
866
867             dtb = malloc(dtb_size);
868             if (!dtb) {
869                     fprintf(stderr, "Can't allocated %ld\n", dtb_size);
>>>     CID 510469:    (RESOURCE_LEAK)
>>>     Variable "f" going out of scope leaks the storage it points to.
870                     return NULL;
871             }
872
873             if (fread(dtb, dtb_size, 1, f) != 1) {
874                     fprintf(stderr, "Can't read %ld bytes from %s\n",
875                             dtb_size, path);

** CID 510468:  Integer handling issues  (SIGN_EXTENSION)
/lib/alist.c: 65 in alist_expand_to()


________________________________________________________________________________________________________
*** CID 510468:  Integer handling issues  (SIGN_EXTENSION)
/lib/alist.c: 65 in alist_expand_to()
59      new_data = malloc(lst->obj_size * new_alloc);
60      if (!new_data) {
61              lst->flags |= ALISTF_FAIL;
62              return false;
63      }
64
>>>     CID 510468:  Integer handling issues  (SIGN_EXTENSION)
>>>     Suspicious implicit sign extension: "lst->obj_size" with type "u16" (16 bits, unsigned) is promoted in "lst->obj_size * lst->alloc" to type "int" (32 bits, signed), then sign-extended to type "unsigned long" (64 bits, unsigned).  If "lst->obj_size * lst->alloc" is greater than 0x7FFFFFFF, the upper bits of the result will all be 1.
65      memcpy(new_data, lst->data, lst->obj_size * lst->alloc);
66      free(lst->data);
67
68      memset(new_data + lst->obj_size * lst->alloc, '\0',
69             lst->obj_size * (new_alloc - lst->alloc));
70      lst->alloc = new_alloc;

** CID 510467:  Integer handling issues  (CONSTANT_EXPRESSION_RESULT)
/net/tcp.c: 497 in tcp_parse_options()


________________________________________________________________________________________________________
*** CID 510467:  Integer handling issues  (CONSTANT_EXPRESSION_RESULT)
/net/tcp.c: 497 in tcp_parse_options()
491                             tsopt = (struct tcp_t_opt *)p;
492                             rmt_timestamp = tsopt->t_snd;
493                             return;
494                     }
495
496                     /* Process optional NOPs */
>>>     CID 510467:  Integer handling issues  (CONSTANT_EXPRESSION_RESULT)
>>>     "p[0] == 16843009" is always false regardless of the values of its operands. This occurs as the logical operand of "if".
497                     if (p[0] == TCP_O_NOP)
498                             p++;
499             }
500     }
501
502     static u8 tcp_state_machine(u8 tcp_flags, u32 tcp_seq_num, int
payload_len)

** CID 510466:  Control flow issues  (NO_EFFECT)
/lib/uuid.c: 256 in uuid_guid_get_bin()


________________________________________________________________________________________________________
*** CID 510466:  Control flow issues  (NO_EFFECT)
/lib/uuid.c: 256 in uuid_guid_get_bin()
250     };
251
252     int uuid_guid_get_bin(const char *guid_str, unsigned char *guid_bin)
253     {
254             int i;
255
>>>     CID 510466:  Control flow issues  (NO_EFFECT)
>>>     This less-than-zero comparison of an unsigned value is never true. "i < 0UL".
256             for (i = 0; i < ARRAY_SIZE(list_guid); i++) {
257                     if (!strcmp(list_guid[i].string, guid_str)) {
258                             memcpy(guid_bin, &list_guid[i].guid, 16);
259                             return 0;
260                     }
261             }

** CID 510465:  Uninitialized variables  (UNINIT)


________________________________________________________________________________________________________
*** CID 510465:  Uninitialized variables  (UNINIT)
/cmd/upl.c: 59 in do_upl_write()
53      struct unit_test_state uts;
54      struct abuf buf;
55      oftree tree;
56      ulong addr;
57      int ret;
58
>>>     CID 510465:  Uninitialized variables  (UNINIT)
>>>     Using uninitialized value "uts.fail_count" when calling "upl_get_test_data".
59      upl_get_test_data(&uts, upl);
60
61      log_debug("Writing UPL\n");
62      ret = upl_create_handoff_tree(upl, &tree);
63      if (ret) {
64              log_err("Failed to write (err=%dE)\n", ret);

** CID 510464:  Error handling issues  (CHECKED_RETURN)
/net/wget.c: 259 in wget_connected()


________________________________________________________________________________________________________
*** CID 510464:  Error handling issues  (CHECKED_RETURN)
/net/wget.c: 259 in wget_connected()
253
254                             pos = strstr((char *)pkt, content_len);
255                             if (!pos) {
256                                     content_length = -1;
257                             } else {
258                                     pos += sizeof(content_len) + 2;
>>>     CID 510464:  Error handling issues  (CHECKED_RETURN)
>>>     Calling "strict_strtoul" without checking return value (as is done elsewhere 8 out of 10 times).
259                                     strict_strtoul(pos, 10,
&content_length);
260                                     debug_cond(DEBUG_WGET,
261                                                "wget: Connected Len %lu\n",
262                                                content_length);
263                             }
264

** CID 510463:  Memory - illegal accesses  (OVERRUN)
/lib/lmb.c: 37 in lmb_print_region_flags()


________________________________________________________________________________________________________
*** CID 510463:  Memory - illegal accesses  (OVERRUN)
/lib/lmb.c: 37 in lmb_print_region_flags()
31     {
32      u64 bitpos;
33      const char *flag_str[] = { "none", "no-map", "no-overwrite" };
34
35      do {
36              bitpos = flags ? fls(flags) - 1 : 0;
>>>     CID 510463:  Memory - illegal accesses  (OVERRUN)
>>>     Overrunning array "flag_str" of 3 8-byte elements at element index 31 (byte offset 255) using index "bitpos" (which evaluates to 31).
37              printf("%s", flag_str[bitpos]);
38              flags &= ~(1ull << bitpos);
39              puts(flags ? ", " : "\n");
40      } while (flags);
41     }
42

** CID 510462:  Security best practices violations  (DC.WEAK_CRYPTO)
/test/dm/nand.c: 67 in run_test_nand()


________________________________________________________________________________________________________
*** CID 510462:  Security best practices violations  (DC.WEAK_CRYPTO)
/test/dm/nand.c: 67 in run_test_nand()
61      ops.ooblen = mtd->oobsize;
62      ut_assertok(mtd_read_oob(mtd, mtd->erasesize, &ops));
63      ut_asserteq(0, oob[mtd_to_nand(mtd)->badblockpos]);
64
65      /* Generate some data and write it */
66      for (i = 0; i < size / sizeof(int); i++)
>>>     CID 510462:  Security best practices violations  (DC.WEAK_CRYPTO)
>>>     "rand" should not be used for security-related applications, because linear congruential algorithms are too easy to break.
67              gold[i] = rand();
68      ut_assertok(nand_write_skip_bad(mtd, off, &length, NULL, U64_MAX,
69                                      (void *)gold, 0));
70      ut_asserteq(size, length);
71
72      /* Verify */

** CID 510461:  Code maintainability issues  (UNUSED_VALUE)
/boot/upl_write.c: 237 in add_upl_image()


________________________________________________________________________________________________________
*** CID 510461:  Code maintainability issues  (UNUSED_VALUE)
/boot/upl_write.c: 237 in add_upl_image()
231                             return log_msg_ret("sub", ret);
232
233                     ret = write_addr(upl, subnode, UPLP_LOAD, img->load);
234                     if (!ret)
235                             ret = write_size(upl, subnode,
UPLP_SIZE, img->size);
236                     if (!ret && img->offset)
>>>     CID 510461:  Code maintainability issues  (UNUSED_VALUE)
>>>     Assigning value from "ofnode_write_u32(subnode, "offset", img->offset)" to "ret" here, but that stored value is overwritten before it can be used.
237                             ret = ofnode_write_u32(subnode, UPLP_OFFSET,
238                                                    img->offset);
239                     ret = ofnode_write_string(subnode, UPLP_DESCRIPTION,
240                                               img->description);
241                     if (ret)
242                             return log_msg_ret("sim", ret);

** CID 510460:  Resource leaks  (RESOURCE_LEAK)
/fs/ext4/ext4fs.c: 216 in ext4fs_exists()


________________________________________________________________________________________________________
*** CID 510460:  Resource leaks  (RESOURCE_LEAK)
/fs/ext4/ext4fs.c: 216 in ext4fs_exists()
210             struct ext2fs_node *dirnode = NULL;
211             int filetype;
212
213             if (!filename)
214                     return 0;
215
>>>     CID 510460:  Resource leaks  (RESOURCE_LEAK)
>>>     Variable "dirnode" going out of scope leaks the storage it points to.
216             return ext4fs_find_file1(filename,
&ext4fs_root->diropen, &dirnode,
217                                      &filetype);
218     }
219
220     int ext4fs_size(const char *filename, loff_t *size)
221     {

** CID 510459:  Incorrect expression  (SIZEOF_MISMATCH)
/boot/upl_read.c: 523 in decode_upl_graphics()


________________________________________________________________________________________________________
*** CID 510459:  Incorrect expression  (SIZEOF_MISMATCH)
/boot/upl_read.c: 523 in decode_upl_graphics()
517             if (!buf) {
518                     log_warning("Node '%s': Missing 'reg' property\n",
519                                 ofnode_get_name(node));
520                     return log_msg_ret("reg", -EINVAL);
521             }
522
>>>     CID 510459:  Incorrect expression  (SIZEOF_MISMATCH)
>>>     Passing argument "buf" of type "char const *" and argument "8 /* sizeof (buf) */" to function "decode_addr_size" is suspicious.
523             len = decode_addr_size(upl, buf, sizeof(buf), &gra->reg);
524             if (len < 0)
525                     return log_msg_ret("buf", len);
526
527             ret = read_uint(node, UPLP_WIDTH, &gra->width);
528             if (!ret)

** CID 510458:  Control flow issues  (NO_EFFECT)
/lib/uuid.c: 269 in uuid_guid_get_str()


________________________________________________________________________________________________________
*** CID 510458:  Control flow issues  (NO_EFFECT)
/lib/uuid.c: 269 in uuid_guid_get_str()
263     }
264
265     const char *uuid_guid_get_str(const unsigned char *guid_bin)
266     {
267             int i;
268
>>>     CID 510458:  Control flow issues  (NO_EFFECT)
>>>     This less-than-zero comparison of an unsigned value is never true. "i < 0UL".
269             for (i = 0; i < ARRAY_SIZE(list_guid); i++) {
270                     if (!memcmp(list_guid[i].guid.b, guid_bin, 16)) {
271                             return list_guid[i].string;
272                     }
273             }
274             return NULL;

** CID 510457:    (RESOURCE_LEAK)
/tools/mkeficapsule.c: 934 in genguid()
/tools/mkeficapsule.c: 930 in genguid()
/tools/mkeficapsule.c: 924 in genguid()
/tools/mkeficapsule.c: 944 in genguid()
/tools/mkeficapsule.c: 959 in genguid()


________________________________________________________________________________________________________
*** CID 510457:    (RESOURCE_LEAK)
/tools/mkeficapsule.c: 934 in genguid()
928             if (!compatible) {
929                     fprintf(stderr, "No compatible string found in DTB\n");
930                     return -1;
931             }
932             if (strnlen(compatible, compatlen) >= compatlen) {
933                     fprintf(stderr, "Compatible string not
null-terminated\n");
>>>     CID 510457:    (RESOURCE_LEAK)
>>>     Variable "dtb" going out of scope leaks the storage it points to.
934                     return -1;
935             }
936
937             printf("Generating GUIDs for %s with namespace %s:\n",
938                    compatible, DEFAULT_NAMESPACE_GUID);
939             for (; idx < argc; idx++) {
/tools/mkeficapsule.c: 930 in genguid()
924                     return -1;
925             }
926
927             compatible = fdt_getprop(dtb, 0, "compatible", &compatlen);
928             if (!compatible) {
929                     fprintf(stderr, "No compatible string found in DTB\n");
>>>     CID 510457:    (RESOURCE_LEAK)
>>>     Variable "dtb" going out of scope leaks the storage it points to.
930                     return -1;
931             }
932             if (strnlen(compatible, compatlen) >= compatlen) {
933                     fprintf(stderr, "Compatible string not
null-terminated\n");
934                     return -1;
935             }
/tools/mkeficapsule.c: 924 in genguid()
918             if (!dtb)
919                     return -1;
920
921             ret = fdt_check_header(dtb);
922             if (ret) {
923                     fprintf(stderr, "Invalid DTB header: %d\n", ret);
>>>     CID 510457:    (RESOURCE_LEAK)
>>>     Variable "dtb" going out of scope leaks the storage it points to.
924                     return -1;
925             }
926
927             compatible = fdt_getprop(dtb, 0, "compatible", &compatlen);
928             if (!compatible) {
929                     fprintf(stderr, "No compatible string found in DTB\n");
/tools/mkeficapsule.c: 944 in genguid()
938                    compatible, DEFAULT_NAMESPACE_GUID);
939             for (; idx < argc; idx++) {
940                     memset(fw_image, 0, sizeof(fw_image));
941                     namelen = strlen(argv[idx]);
942                     if (namelen > MAX_IMAGE_NAME_LEN) {
943                             fprintf(stderr, "Image name too long:
%s\n", argv[idx]);
>>>     CID 510457:    (RESOURCE_LEAK)
>>>     Variable "dtb" going out of scope leaks the storage it points to.
944                             return -1;
945                     }
946
947                     for (int i = 0; i < namelen; i++)
948                             fw_image[i] = (uint16_t)argv[idx][i];
949
/tools/mkeficapsule.c: 959 in genguid()
953                                 NULL);
954
955                     printf("%s: ", argv[idx]);
956                     print_guid(&image_type_id);
957             }
958
>>>     CID 510457:    (RESOURCE_LEAK)
>>>     Variable "dtb" going out of scope leaks the storage it points to.
959             return 0;
960     }
961
962     /**
963      * main - main entry function of mkeficapsule
964      * @argc:       Number of arguments

** CID 510456:  Integer handling issues  (NEGATIVE_RETURNS)


________________________________________________________________________________________________________
*** CID 510456:  Integer handling issues  (NEGATIVE_RETURNS)
/boot/upl_write.c: 432 in add_upl_memres()
426                     ret = ofnode_add_subnode(mem_node, name, &node);
427                     if (ret)
428                             return log_msg_ret("memres", ret);
429
430                     len = buffer_addr_size(upl, buf, sizeof(buf),
431                                            memres->region.count,
&memres->region);
>>>     CID 510456:  Integer handling issues  (NEGATIVE_RETURNS)
>>>     "len" is passed to a parameter that cannot be negative.
432                     ret = ofnode_write_prop(node, UPLP_REG, buf, len, true);
433                     if (!ret && memres->no_map)
434                             ret = ofnode_write_bool(node, UPLP_NO_MAP,
435                                                     memres->no_map);
436                     if (ret)
437                             return log_msg_ret("lst", ret);

** CID 510455:  Memory - corruptions  (OVERLAPPING_COPY)
/fs/squashfs/sqfs.c: 971 in sqfs_opendir_nest()


________________________________________________________________________________________________________
*** CID 510455:  Memory - corruptions  (OVERLAPPING_COPY)
/fs/squashfs/sqfs.c: 971 in sqfs_opendir_nest()
965             if (le16_to_cpu(dirs->i_dir.inode_type) == SQFS_DIR_TYPE)
966                     dirs->size = le16_to_cpu(dirs->i_dir.file_size);
967             else
968                     dirs->size = le32_to_cpu(dirs->i_ldir.file_size);
969
970             /* Setup directory header */
>>>     CID 510455:  Memory - corruptions  (OVERLAPPING_COPY)
>>>     Copying 12 bytes from "dirs->table" to "dirs->dir_header", which point to overlapping memory locations.
971             memcpy(dirs->dir_header, dirs->table, SQFS_DIR_HEADER_SIZE);
972             dirs->entry_count = dirs->dir_header->count + 1;
973             dirs->size -= SQFS_DIR_HEADER_SIZE;
974
975             /* Setup entry */
976             dirs->entry = NULL;

** CID 510454:    (SIZEOF_MISMATCH)
/test/cmd/mbr.c: 280 in mbr_test_run()
/test/cmd/mbr.c: 421 in mbr_test_run()
/test/cmd/mbr.c: 351 in mbr_test_run()
/test/cmd/mbr.c: 316 in mbr_test_run()
/test/cmd/mbr.c: 386 in mbr_test_run()


________________________________________________________________________________________________________
*** CID 510454:    (SIZEOF_MISMATCH)
/test/cmd/mbr.c: 280 in mbr_test_run()
274
275             /* Make sure mmc6 is 12+ MiB in size */
276             ut_assertok(run_commandf("mmc read %lx %lx 1", ra,
277                                      (ulong)0xbffe00 / BLKSZ));
278
279             /* Test one MBR partition */
>>>     CID 510454:    (SIZEOF_MISMATCH)
>>>     Passing argument "mbr_wbuf" of type "unsigned char *" and argument "8UL /* sizeof (mbr_wbuf) */" to function "init_write_buffers" is suspicious.
280             init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf),
ebr_wbuf, sizeof(ebr_wbuf), __LINE__);
281             ut_assertok(build_mbr_parts(mbr_parts_buf,
sizeof(mbr_parts_buf), 1));
282             ut_assertok(run_commandf("write mmc 6:0 %lx 0 1", mbr_wa));
283             memset(rbuf, '\0', BLKSZ);
284             ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra));
285             ut_assertok(memcmp(mbr_wbuf, rbuf, BLKSZ));
/test/cmd/mbr.c: 421 in mbr_test_run()
415                     ut_assertf(rbuf[mbr_cmp_start + i] ==
mbr_parts_ref_p4[i],
416                             "4P MBR+0x%04X: expected %#02X,
actual: %#02X\n",
417                             mbr_cmp_start + i,
mbr_parts_ref_p4[i], rbuf[mbr_cmp_start + i]);
418             }
419
420             /* Test five MBR partitions */
>>>     CID 510454:    (SIZEOF_MISMATCH)
>>>     Passing argument "mbr_wbuf" of type "unsigned char *" and argument "8UL /* sizeof (mbr_wbuf) */" to function "init_write_buffers" is suspicious.
421             init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf),
ebr_wbuf, sizeof(ebr_wbuf), __LINE__);
422             ut_assertok(build_mbr_parts(mbr_parts_buf,
sizeof(mbr_parts_buf), 5));
423             ut_assertok(run_commandf("write mmc 6:0 %lx 0 1", mbr_wa));
424             memset(rbuf, '\0', BLKSZ);
425             ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra));
426             ut_assertok(memcmp(mbr_wbuf, rbuf, BLKSZ));
/test/cmd/mbr.c: 351 in mbr_test_run()
345                     ut_assertf(rbuf[mbr_cmp_start + i] ==
mbr_parts_ref_p2[i],
346                             "2P MBR+0x%04X: expected %#02X,
actual: %#02X\n",
347                             mbr_cmp_start + i,
mbr_parts_ref_p2[i], rbuf[mbr_cmp_start + i]);
348             }
349
350             /* Test three MBR partitions */
>>>     CID 510454:    (SIZEOF_MISMATCH)
>>>     Passing argument "mbr_wbuf" of type "unsigned char *" and argument "8UL /* sizeof (mbr_wbuf) */" to function "init_write_buffers" is suspicious.
351             init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf),
ebr_wbuf, sizeof(ebr_wbuf), __LINE__);
352             ut_assertok(build_mbr_parts(mbr_parts_buf,
sizeof(mbr_parts_buf), 3));
353             ut_assertok(run_commandf("write mmc 6:0 %lx 0 1", mbr_wa));
354             memset(rbuf, '\0', BLKSZ);
355             ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra));
356             ut_assertok(memcmp(mbr_wbuf, rbuf, BLKSZ));
/test/cmd/mbr.c: 316 in mbr_test_run()
310                     ut_assertf(rbuf[mbr_cmp_start + i] ==
mbr_parts_ref_p1[i],
311                             "1P MBR+0x%04X: expected %#02X,
actual: %#02X\n",
312                             mbr_cmp_start + i,
mbr_parts_ref_p1[i], rbuf[mbr_cmp_start + i]);
313             }
314
315             /* Test two MBR partitions */
>>>     CID 510454:    (SIZEOF_MISMATCH)
>>>     Passing argument "mbr_wbuf" of type "unsigned char *" and argument "8UL /* sizeof (mbr_wbuf) */" to function "init_write_buffers" is suspicious.
316             init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf),
ebr_wbuf, sizeof(ebr_wbuf), __LINE__);
317             ut_assertok(build_mbr_parts(mbr_parts_buf,
sizeof(mbr_parts_buf), 2));
318             ut_assertok(run_commandf("write mmc 6:0 %lx 0 1", mbr_wa));
319             memset(rbuf, '\0', BLKSZ);
320             ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra));
321             ut_assertok(memcmp(mbr_wbuf, rbuf, BLKSZ));
/test/cmd/mbr.c: 386 in mbr_test_run()
380                     ut_assertf(rbuf[mbr_cmp_start + i] ==
mbr_parts_ref_p3[i],
381                             "3P MBR+0x%04X: expected %#02X,
actual: %#02X\n",
382                             mbr_cmp_start + i,
mbr_parts_ref_p3[i], rbuf[mbr_cmp_start + i]);
383             }
384
385             /* Test four MBR partitions */
>>>     CID 510454:    (SIZEOF_MISMATCH)
>>>     Passing argument "mbr_wbuf" of type "unsigned char *" and argument "8UL /* sizeof (mbr_wbuf) */" to function "init_write_buffers" is suspicious.
386             init_write_buffers(mbr_wbuf, sizeof(mbr_wbuf),
ebr_wbuf, sizeof(ebr_wbuf), __LINE__);
387             ut_assertok(build_mbr_parts(mbr_parts_buf,
sizeof(mbr_parts_buf), 4));
388             ut_assertok(run_commandf("write mmc 6:0 %lx 0 1", mbr_wa));
389             memset(rbuf, '\0', BLKSZ);
390             ut_assertok(run_commandf("read mmc 6:0 %lx 0 1", ra));
391             ut_assertok(memcmp(mbr_wbuf, rbuf, BLKSZ));

** CID 510453:  Null pointer dereferences  (FORWARD_NULL)
/fs/squashfs/sqfs.c: 983 in sqfs_opendir_nest()


________________________________________________________________________________________________________
*** CID 510453:  Null pointer dereferences  (FORWARD_NULL)
/fs/squashfs/sqfs.c: 983 in sqfs_opendir_nest()
977             dirs->table += SQFS_DIR_HEADER_SIZE;
978
979             *dirsp = (struct fs_dir_stream *)dirs;
980
981     out:
982             for (j = 0; j < token_count; j++)
>>>     CID 510453:  Null pointer dereferences  (FORWARD_NULL)
>>>     Dereferencing null pointer "token_list".
983                     free(token_list[j]);
984             free(token_list);
985             free(pos_list);
986             free(path);
987             if (ret) {
988                     free(inode_table);

** CID 510452:  Null pointer dereferences  (FORWARD_NULL)
/fs/squashfs/sqfs.c: 1676 in sqfs_size_nest()


________________________________________________________________________________________________________
*** CID 510452:  Null pointer dereferences  (FORWARD_NULL)
/fs/squashfs/sqfs.c: 1676 in sqfs_size_nest()
1670                    printf("File not found.\n");
1671                    *size = 0;
1672                    ret = -EINVAL;
1673                    goto free_strings;
1674            }
1675
>>>     CID 510452:  Null pointer dereferences  (FORWARD_NULL)
>>>     Dereferencing null pointer "dirs->entry".
1676            i_number = dirs->dir_header->inode_number +
dirs->entry->inode_offset;
1677            ipos = sqfs_find_inode(dirs->inode_table, i_number,
sblk->inodes,
1678                                   sblk->block_size);
1679
1680            if (!ipos) {
1681                    *size = 0;

** CID 510451:    (TAINTED_SCALAR)
/fs/squashfs/sqfs.c: 1612 in sqfs_read_nest()
/fs/squashfs/sqfs.c: 1612 in sqfs_read_nest()
/fs/squashfs/sqfs.c: 1604 in sqfs_read_nest()


________________________________________________________________________________________________________
*** CID 510451:    (TAINTED_SCALAR)
/fs/squashfs/sqfs.c: 1612 in sqfs_read_nest()
1606
1607                    free(fragment_block);
1608
1609            } else if (finfo.frag && !finfo.comp) {
1610                    fragment_block = (void *)fragment + table_offset;
1611
>>>     CID 510451:    (TAINTED_SCALAR)
>>>     Using tainted variable "finfo.offset" as an index to pointer "fragment_block".
1612                    memcpy(buf + *actread,
&fragment_block[finfo.offset], finfo.size - *actread);
1613                    *actread = finfo.size;
1614            }
1615
1616     out:
1617            free(fragment);
/fs/squashfs/sqfs.c: 1612 in sqfs_read_nest()
1606
1607                    free(fragment_block);
1608
1609            } else if (finfo.frag && !finfo.comp) {
1610                    fragment_block = (void *)fragment + table_offset;
1611
>>>     CID 510451:    (TAINTED_SCALAR)
>>>     Passing tainted expression "finfo.size - *actread" to "memcpy", which uses it as an offset. [Note: The source code implementation of the function has been overridden by a builtin model.]
1612                    memcpy(buf + *actread,
&fragment_block[finfo.offset], finfo.size - *actread);
1613                    *actread = finfo.size;
1614            }
1615
1616     out:
1617            free(fragment);
/fs/squashfs/sqfs.c: 1621 in sqfs_read_nest()
1615
1616     out:
1617            free(fragment);
1618            free(datablock);
1619            free(file);
1620            free(dir);
>>>     CID 510451:    (TAINTED_SCALAR)
>>>     Passing tainted expression "*finfo.blk_sizes" to "dlfree", which uses it as an offset.
1621            free(finfo.blk_sizes);
1622            sqfs_closedir(dirsp);
1623
1624            return ret;
1625     }
1626
/fs/squashfs/sqfs.c: 1604 in sqfs_read_nest()
1598                                          frag_entry.size);
1599                    if (ret) {
1600                            free(fragment_block);
1601                            goto out;
1602                    }
1603
>>>     CID 510451:    (TAINTED_SCALAR)
>>>     Using tainted variable "finfo.offset" as an index to pointer "fragment_block".
1604                    memcpy(buf + *actread,
&fragment_block[finfo.offset], finfo.size - *actread);
1605                    *actread = finfo.size;
1606
1607                    free(fragment_block);
1608
1609            } else if (finfo.frag && !finfo.comp) {

** CID 510450:  Code maintainability issues  (UNUSED_VALUE)
/fs/squashfs/sqfs.c: 1506 in sqfs_read_nest()


________________________________________________________________________________________________________
*** CID 510450:  Code maintainability issues  (UNUSED_VALUE)
/fs/squashfs/sqfs.c: 1506 in sqfs_read_nest()
1500                    n_blks = DIV_ROUND_UP(table_size + table_offset,
1501                                          ctxt.cur_dev->blksz);
1502
1503                    /* Don't load any data for sparse blocks */
1504                    if (finfo.blk_sizes[j] == 0) {
1505                            n_blks = 0;
>>>     CID 510450:  Code maintainability issues  (UNUSED_VALUE)
>>>     Assigning value "0ULL" to "table_offset" here, but that stored value is overwritten before it can be used.
1506                            table_offset = 0;
1507                            data_buffer = NULL;
1508                            data = NULL;
1509                    } else {
1510                            data_buffer =
malloc_cache_aligned(n_blks * ctxt.cur_dev->blksz);
1511

----- End forwarded message -----

-- 
Tom

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New Defects reported by Coverity Scan for Das U-Boot
  2024-10-07 17:15 Fwd: New Defects reported by Coverity Scan for Das U-Boot Tom Rini
@ 2024-10-07 18:17 ` Richard Weinberger
  2024-10-07 20:01   ` Tom Rini
  0 siblings, 1 reply; 20+ messages in thread
From: Richard Weinberger @ 2024-10-07 18:17 UTC (permalink / raw)
  To: Tom Rini
  Cc: u-boot, Simon Glass, Joao Marcos Costa, Thomas Petazzoni,
	Jerome Forissier, Sughosh Ganu, Caleb Connolly

----- Ursprüngliche Mail -----
> Von: "Tom Rini" <trini@konsulko.com>
> An: "u-boot" <u-boot@lists.denx.de>
> CC: "Simon Glass" <sjg@chromium.org>, "Joao Marcos Costa" <jmcosta944@gmail.com>, "Thomas Petazzoni"
> <thomas.petazzoni@bootlin.com>, "Jerome Forissier" <jerome.forissier@linaro.org>, "Sughosh Ganu"
> <sughosh.ganu@linaro.org>, "Caleb Connolly" <caleb.connolly@linaro.org>, "richard" <richard@nod.at>
> Gesendet: Montag, 7. Oktober 2024 19:15:05
> Betreff: Fwd: New Defects reported by Coverity Scan for Das U-Boot

> Now that I've merged next to master, there's a number of issues to
> address.

AFAICT, all issues in fs/{ext4, squashfs} are not new.
Looks more like Coverity re-scanned these files because they have been touched
by my fixes.

Later this week I'll have more time to triage these issues and post fixes.

Thanks,
//richard

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New Defects reported by Coverity Scan for Das U-Boot
  2024-10-07 18:17 ` Richard Weinberger
@ 2024-10-07 20:01   ` Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2024-10-07 20:01 UTC (permalink / raw)
  To: Richard Weinberger
  Cc: u-boot, Simon Glass, Joao Marcos Costa, Thomas Petazzoni,
	Jerome Forissier, Sughosh Ganu, Caleb Connolly

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

On Mon, Oct 07, 2024 at 08:17:44PM +0200, Richard Weinberger wrote:
> ----- Ursprüngliche Mail -----
> > Von: "Tom Rini" <trini@konsulko.com>
> > An: "u-boot" <u-boot@lists.denx.de>
> > CC: "Simon Glass" <sjg@chromium.org>, "Joao Marcos Costa" <jmcosta944@gmail.com>, "Thomas Petazzoni"
> > <thomas.petazzoni@bootlin.com>, "Jerome Forissier" <jerome.forissier@linaro.org>, "Sughosh Ganu"
> > <sughosh.ganu@linaro.org>, "Caleb Connolly" <caleb.connolly@linaro.org>, "richard" <richard@nod.at>
> > Gesendet: Montag, 7. Oktober 2024 19:15:05
> > Betreff: Fwd: New Defects reported by Coverity Scan for Das U-Boot
> 
> > Now that I've merged next to master, there's a number of issues to
> > address.
> 
> AFAICT, all issues in fs/{ext4, squashfs} are not new.
> Looks more like Coverity re-scanned these files because they have been touched
> by my fixes.

Quite likely, yeah.

> Later this week I'll have more time to triage these issues and post fixes.

Thanks!

-- 
Tom

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New Defects reported by Coverity Scan for Das U-Boot
  2024-10-16  3:47 Fwd: " Tom Rini
@ 2024-10-16  6:12 ` Ilias Apalodimas
  2024-10-16  8:20   ` Abbarapu, Venkatesh
  2024-10-16 15:23 ` Raymond Mao
  1 sibling, 1 reply; 20+ messages in thread
From: Ilias Apalodimas @ 2024-10-16  6:12 UTC (permalink / raw)
  To: Tom Rini
  Cc: u-boot, Vignesh R, Takahiro Kuwano, Tudor Ambarus,
	Venkatesh Yadav Abbarapu, Pratyush Yadav, Ashok Reddy Soma,
	Joakim Tjernlund, Raymond Mao

HI Tom,

We'll have a look on the mbedTLS reports later today

Thanks
/Ilias

On Wed, 16 Oct 2024 at 06:47, Tom Rini <trini@konsulko.com> wrote:
>
> Hey all, here's the latest report.
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Tue, Oct 15, 2024 at 5:06 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.
>
> 22 new defect(s) introduced to Das U-Boot found with Coverity Scan.
>
>
> New defect(s) Reported-by: Coverity Scan
> Showing 20 of 22 defect(s)
>
>
> ** CID 510813:  Control flow issues  (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1652 in spi_nor_read()
>
>
> ________________________________________________________________________________________________________
> *** CID 510813:  Control flow issues  (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1652 in spi_nor_read()
> 1646                            goto read_err;
> 1647                    }
> 1648                    if (ret < 0)
> 1649                            goto read_err;
> 1650
> 1651                    if (is_ofst_odd == true) {
> >>>     CID 510813:  Control flow issues  (DEADCODE)
> >>>     Execution cannot reach this statement: "memmove(buf, buf + 1, len -...".
> 1652                            memmove(buf, (buf + 1), (len - 1));
> 1653                            *retlen += (ret - 1);
> 1654                            buf += ret - 1;
> 1655                            is_ofst_odd = false;
> 1656                    } else {
> 1657                            *retlen += ret;
>
> ** CID 510812:    (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 3573 in spi_nor_select_erase()
> /drivers/mtd/spi/spi-nor-core.c: 3584 in spi_nor_select_erase()
> /drivers/mtd/spi/spi-nor-core.c: 3610 in spi_nor_select_erase()
> /drivers/mtd/spi/spi-nor-core.c: 3597 in spi_nor_select_erase()
>
>
> ________________________________________________________________________________________________________
> *** CID 510812:    (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 3573 in spi_nor_select_erase()
> 3567                    /*
> 3568                     * In parallel-memories the erase operation is
> 3569                     * performed on both the flashes simultaneously
> 3570                     * so, double the erasesize.
> 3571                     */
> 3572                    if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>>     CID 510812:    (DEADCODE)
> >>>     Execution cannot reach this statement: "mtd->erasesize = 8192U;".
> 3573                            mtd->erasesize = 4096 * 2;
> 3574                    else
> 3575                            mtd->erasesize = 4096;
> 3576            } else if (info->flags & SECT_4K_PMC) {
> 3577                    nor->erase_opcode = SPINOR_OP_BE_4K_PMC;
> 3578                    /*
> /drivers/mtd/spi/spi-nor-core.c: 3584 in spi_nor_select_erase()
> 3578                    /*
> 3579                     * In parallel-memories the erase operation is
> 3580                     * performed on both the flashes simultaneously
> 3581                     * so, double the erasesize.
> 3582                     */
> 3583                    if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>>     CID 510812:    (DEADCODE)
> >>>     Execution cannot reach this statement: "mtd->erasesize = 8192U;".
> 3584                            mtd->erasesize = 4096 * 2;
> 3585                    else
> 3586                            mtd->erasesize = 4096;
> 3587            } else
> 3588     #endif
> 3589            {
> /drivers/mtd/spi/spi-nor-core.c: 3610 in spi_nor_select_erase()
> 3604                    /*
> 3605                     * In parallel-memories the erase operation is
> 3606                     * performed on both the flashes simultaneously
> 3607                     * so, double the erasesize.
> 3608                     */
> 3609                    if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>>     CID 510812:    (DEADCODE)
> >>>     Execution cannot reach this statement: "mtd->erasesize = 8192U;".
> 3610                            mtd->erasesize = 4096 * 2;
> 3611                    else
> 3612                            mtd->erasesize = 4096;
> 3613            }
> 3614
> 3615            return 0;
> /drivers/mtd/spi/spi-nor-core.c: 3597 in spi_nor_select_erase()
> 3591                    /*
> 3592                     * In parallel-memories the erase operation is
> 3593                     * performed on both the flashes simultaneously
> 3594                     * so, double the erasesize.
> 3595                     */
> 3596                    if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>>     CID 510812:    (DEADCODE)
> >>>     Execution cannot reach this statement: "mtd->erasesize = info->sect...".
> 3597                            mtd->erasesize = info->sector_size * 2;
> 3598                    else
> 3599                            mtd->erasesize = info->sector_size;
> 3600            }
> 3601
> 3602            if ((JEDEC_MFR(info) == SNOR_MFR_SST) && info->flags &
> SECT_4K) {
>
> ** CID 510811:    (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1134 in spi_nor_erase()
> /drivers/mtd/spi/spi-nor-core.c: 1137 in spi_nor_erase()
>
>
> ________________________________________________________________________________________________________
> *** CID 510811:    (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1134 in spi_nor_erase()
> 1128                            addr_known = false;
> 1129                            ret = -EINTR;
> 1130                            goto erase_err;
> 1131                    }
> 1132                    offset = addr;
> 1133                    if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>>     CID 510811:    (DEADCODE)
> >>>     Execution cannot reach this statement: "offset /= 2U;".
> 1134                            offset /= 2;
> 1135
> 1136                    if (nor->flags & SNOR_F_HAS_STACKED) {
> 1137                            if (offset >= (mtd->size / 2)) {
> 1138                                    offset = offset - (mtd->size / 2);
> 1139                                    nor->spi->flags |= SPI_XFER_U_PAGE;
> /drivers/mtd/spi/spi-nor-core.c: 1137 in spi_nor_erase()
> 1131                    }
> 1132                    offset = addr;
> 1133                    if (nor->flags & SNOR_F_HAS_PARALLEL)
> 1134                            offset /= 2;
> 1135
> 1136                    if (nor->flags & SNOR_F_HAS_STACKED) {
> >>>     CID 510811:    (DEADCODE)
> >>>     Execution cannot reach this statement: "if (offset >= mtd->size / 2...".
> 1137                            if (offset >= (mtd->size / 2)) {
> 1138                                    offset = offset - (mtd->size / 2);
> 1139                                    nor->spi->flags |= SPI_XFER_U_PAGE;
> 1140                            } else {
> 1141                                    nor->spi->flags &= ~SPI_XFER_U_PAGE;
> 1142                            }
>
> ** CID 510810:  Control flow issues  (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1556 in spi_nor_read_id()
>
>
> ________________________________________________________________________________________________________
> *** CID 510810:  Control flow issues  (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1556 in spi_nor_read_id()
> 1550     {
> 1551            int                     tmp;
> 1552            u8                      id[SPI_NOR_MAX_ID_LEN];
> 1553            const struct flash_info *info;
> 1554
> 1555            if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>>     CID 510810:  Control flow issues  (DEADCODE)
> >>>     Execution cannot reach this statement: "nor->spi->flags |= 0x100;".
> 1556                    nor->spi->flags |= SPI_XFER_LOWER;
> 1557
> 1558            tmp = nor->read_reg(nor, SPINOR_OP_RDID, id,
> SPI_NOR_MAX_ID_LEN);
> 1559            if (tmp < 0) {
> 1560                    dev_dbg(nor->dev, "error %d reading JEDEC ID\n", tmp);
> 1561                    return ERR_PTR(tmp);
>
> ** CID 510809:  Resource leaks  (RESOURCE_LEAK)
> /lib/mbedtls/pkcs7_parser.c: 385 in x509_populate_sinfo()
>
>
> ________________________________________________________________________________________________________
> *** CID 510809:  Resource leaks  (RESOURCE_LEAK)
> /lib/mbedtls/pkcs7_parser.c: 385 in x509_populate_sinfo()
> 379                                   signed_info);
> 380             if (ret)
> 381                     goto out_err_sinfo;
> 382
> 383     no_authattrs:
> 384             *sinfo = signed_info;
> >>>     CID 510809:  Resource leaks  (RESOURCE_LEAK)
> >>>     Variable "mctx" going out of scope leaks the storage it points to.
> 385             return 0;
> 386
> 387     out_err_sinfo:
> 388             pkcs7_free_sinfo_mbedtls_ctx(mctx);
> 389     out_no_mctx:
> 390             public_key_signature_free(s);
>
> ** CID 510808:  Control flow issues  (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 724 in spi_nor_set_4byte_opcodes()
>
>
> ________________________________________________________________________________________________________
> *** CID 510808:  Control flow issues  (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 724 in spi_nor_set_4byte_opcodes()
> 718     static void spi_nor_set_4byte_opcodes(struct spi_nor *nor,
> 719                                           const struct flash_info *info)
> 720     {
> 721             bool shift = 0;
> 722
> 723             if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>>     CID 510808:  Control flow issues  (DEADCODE)
> >>>     Execution cannot reach this statement: "shift = true;".
> 724                     shift = 1;
> 725
> 726             /* Do some manufacturer fixups first */
> 727             switch (JEDEC_MFR(info)) {
> 728             case SNOR_MFR_SPANSION:
> 729                     /* No small sector erase for 4-byte command set */
>
> ** CID 510807:  Control flow issues  (DEADCODE)
> /lib/mbedtls/external/mbedtls/library/x509_crt.c: 2750 in x509_inet_pton_ipv6()
>
>
> ________________________________________________________________________________________________________
> *** CID 510807:  Control flow issues  (DEADCODE)
> /lib/mbedtls/external/mbedtls/library/x509_crt.c: 2750 in x509_inet_pton_ipv6()
> 2744                 MBEDTLS_PUT_UINT16_BE(group, addr, nonzero_groups);
> 2745                 nonzero_groups++;
> 2746                 if (*p == '\0') {
> 2747                     break;
> 2748                 } else if (*p == '.') {
> 2749                     /* Don't accept IPv4 too early or late */
> >>>     CID 510807:  Control flow issues  (DEADCODE)
> >>>     Execution cannot reach the expression "zero_group_start == -1" inside this statement: "if ((nonzero_groups == 0 &&...".
> 2750                     if ((nonzero_groups == 0 && zero_group_start == -1) ||
> 2751                         nonzero_groups >= 7) {
> 2752                         break;
> 2753                     }
> 2754
> 2755                     /* Walk back to prior ':', then parse as IPv4-mapped */
>
> ** CID 510806:  Control flow issues  (DEADCODE)
> /lib/mbedtls/pkcs7_parser.c: 209 in authattrs_parse()
>
>
> ________________________________________________________________________________________________________
> *** CID 510806:  Control flow issues  (DEADCODE)
> /lib/mbedtls/pkcs7_parser.c: 209 in authattrs_parse()
> 203                                     return -EINVAL;
> 204                     }
> 205
> 206                     p += seq_len;
> 207             }
> 208
> >>>     CID 510806:  Control flow issues  (DEADCODE)
> >>>     Execution cannot reach the expression "ret != -96" inside this statement: "if (ret && ret != -96)
>   re...".
> 209             if (ret && ret !=  MBEDTLS_ERR_ASN1_OUT_OF_DATA)
> 210                     return ret;
> 211
> 212             msg->have_authattrs = true;
> 213
> 214             /*
>
> ** CID 510805:  Memory - illegal accesses  (OVERRUN)
> /lib/rsa/rsa-keyprop.c: 678 in rsa_gen_key_prop()
>
>
> ________________________________________________________________________________________________________
> *** CID 510805:  Memory - illegal accesses  (OVERRUN)
> /lib/rsa/rsa-keyprop.c: 678 in rsa_gen_key_prop()
> 672             (*prop)->num_bits = (rsa_key.n_sz - i) * 8;
> 673             (*prop)->modulus = malloc(rsa_key.n_sz - i);
> 674             if (!(*prop)->modulus) {
> 675                     ret = -ENOMEM;
> 676                     goto out;
> 677             }
> >>>     CID 510805:  Memory - illegal accesses  (OVERRUN)
> >>>     Overrunning dynamic array "rsa_key.n" at offset corresponding to index variable "i".
> 678             memcpy((void *)(*prop)->modulus, &rsa_key.n[i],
> rsa_key.n_sz - i);
> 679
> 680             n = calloc(sizeof(uint32_t), 1 + ((*prop)->num_bits >> 5));
> 681             rr = calloc(sizeof(uint32_t), 1 + (((*prop)->num_bits
> * 2) >> 5));
> 682             rrtmp = calloc(sizeof(uint32_t), 2 +
> (((*prop)->num_bits * 2) >> 5));
> 683             if (!n || !rr || !rrtmp) {
>
> ** CID 510804:  Integer handling issues  (CONSTANT_EXPRESSION_RESULT)
> /drivers/mtd/spi/spi-nor-core.c: 1556 in spi_nor_read_id()
>
>
> ________________________________________________________________________________________________________
> *** CID 510804:  Integer handling issues  (CONSTANT_EXPRESSION_RESULT)
> /drivers/mtd/spi/spi-nor-core.c: 1556 in spi_nor_read_id()
> 1550     {
> 1551            int                     tmp;
> 1552            u8                      id[SPI_NOR_MAX_ID_LEN];
> 1553            const struct flash_info *info;
> 1554
> 1555            if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>>     CID 510804:  Integer handling issues  (CONSTANT_EXPRESSION_RESULT)
> >>>     In "nor->spi->flags |= 256 /* 1 << 8 */", wider "256 /* 1 << 8 */" has high-order bits (0x100) that don't affect the narrower left-hand side.
> 1556                    nor->spi->flags |= SPI_XFER_LOWER;
> 1557
> 1558            tmp = nor->read_reg(nor, SPINOR_OP_RDID, id,
> SPI_NOR_MAX_ID_LEN);
> 1559            if (tmp < 0) {
> 1560                    dev_dbg(nor->dev, "error %d reading JEDEC ID\n", tmp);
> 1561                    return ERR_PTR(tmp);
>
> ** CID 510803:  Code maintainability issues  (UNUSED_VALUE)
> /drivers/mtd/spi/spi-nor-core.c: 1138 in spi_nor_erase()
>
>
> ________________________________________________________________________________________________________
> *** CID 510803:  Code maintainability issues  (UNUSED_VALUE)
> /drivers/mtd/spi/spi-nor-core.c: 1138 in spi_nor_erase()
> 1132                    offset = addr;
> 1133                    if (nor->flags & SNOR_F_HAS_PARALLEL)
> 1134                            offset /= 2;
> 1135
> 1136                    if (nor->flags & SNOR_F_HAS_STACKED) {
> 1137                            if (offset >= (mtd->size / 2)) {
> >>>     CID 510803:  Code maintainability issues  (UNUSED_VALUE)
> >>>     Assigning value from "offset - mtd->size / 2ULL" to "offset" here, but that stored value is overwritten before it can be used.
> 1138                                    offset = offset - (mtd->size / 2);
> 1139                                    nor->spi->flags |= SPI_XFER_U_PAGE;
> 1140                            } else {
> 1141                                    nor->spi->flags &= ~SPI_XFER_U_PAGE;
> 1142                            }
> 1143                    }
>
> ** CID 510802:  Control flow issues  (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 504 in read_sr()
>
>
> ________________________________________________________________________________________________________
> *** CID 510802:  Control flow issues  (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 504 in read_sr()
> 498              * discard the second byte.
> 499              */
> 500             if (spi_nor_protocol_is_dtr(nor->reg_proto))
> 501                     op.data.nbytes = 2;
> 502
> 503             if (nor->flags & SNOR_F_HAS_PARALLEL) {
> >>>     CID 510802:  Control flow issues  (DEADCODE)
> >>>     Execution cannot reach this statement: "op.data.nbytes = 2U;".
> 504                     op.data.nbytes = 2;
> 505                     ret = spi_nor_read_write_reg(nor, &op, &val[0]);
> 506                     if (ret < 0) {
> 507                             pr_debug("error %d reading SR\n", (int)ret);
> 508                             return ret;
> 509                     }
>
> ** CID 510801:  Null pointer dereferences  (FORWARD_NULL)
>
>
> ________________________________________________________________________________________________________
> *** CID 510801:  Null pointer dereferences  (FORWARD_NULL)
> /lib/ecdsa/ecdsa-libcrypto.c: 365 in ecdsa_add_verify_data()
> 359             struct signer ctx;
> 360             int ret;
> 361
> 362             fdt_key_name = info->keyname ? info->keyname : "default-key";
> 363             ret = prepare_ctx(&ctx, info);
> 364             if (ret >= 0) {
> >>>     CID 510801:  Null pointer dereferences  (FORWARD_NULL)
> >>>     Passing "info" to "do_add", which dereferences null "info->keyname".
> 365                     ret = do_add(&ctx, fdt, fdt_key_name, info);
> 366                     if (ret < 0)
> 367                             ret = ret == -FDT_ERR_NOSPACE ? -ENOSPC : -EIO;
> 368             }
> 369
> 370             free_ctx(&ctx);
> 371             return ret;
>
> ** CID 510800:    (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1620 in spi_nor_read()
> /drivers/mtd/spi/spi-nor-core.c: 1590 in spi_nor_read()
> /drivers/mtd/spi/spi-nor-core.c: 1611 in spi_nor_read()
> /drivers/mtd/spi/spi-nor-core.c: 1600 in spi_nor_read()
>
>
> ________________________________________________________________________________________________________
> *** CID 510800:    (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1620 in spi_nor_read()
> 1614                            } else {
> 1615                                    nor->spi->flags &= ~SPI_XFER_U_PAGE;
> 1616                            }
> 1617                    }
> 1618
> 1619                    if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>>     CID 510800:    (DEADCODE)
> >>>     Execution cannot reach this statement: "offset /= 2LL;".
> 1620                            offset /= 2;
> 1621
> 1622                    if (nor->addr_width == 3) {
> 1623     #ifdef CONFIG_SPI_FLASH_BAR
> 1624                            ret = write_bar(nor, offset);
> 1625                            if (ret < 0)
> /drivers/mtd/spi/spi-nor-core.c: 1590 in spi_nor_read()
> 1584            u32 rem_bank_len = 0;
> 1585            u8 bank;
> 1586            bool is_ofst_odd = false;
> 1587
> 1588            dev_dbg(nor->dev, "from 0x%08x, len %zd\n", (u32)from, len);
> 1589
> >>>     CID 510800:    (DEADCODE)
> >>>     Execution cannot reach the expression "offset & 1LL" inside this statement: "if (nor->flags & SNOR_F_HAS...".
> 1590            if ((nor->flags & SNOR_F_HAS_PARALLEL) && (offset & 1)) {
> 1591                /* We can hit this case when we use file system
> like ubifs */
> 1592                    from--;
> 1593                    len++;
> 1594                    is_ofst_odd = true;
> 1595            }
> /drivers/mtd/spi/spi-nor-core.c: 1611 in spi_nor_read()
> 1605                                    rem_bank_len = (SZ_16M * (bank
> + 1)) - from;
> 1606                            }
> 1607                    }
> 1608                    offset = from;
> 1609
> 1610                    if (nor->flags & SNOR_F_HAS_STACKED) {
> >>>     CID 510800:    (DEADCODE)
> >>>     Execution cannot reach this statement: "if (offset >= mtd->size / 2...".
> 1611                            if (offset >= (mtd->size / 2)) {
> 1612                                    offset = offset - (mtd->size / 2);
> 1613                                    nor->spi->flags |= SPI_XFER_U_PAGE;
> 1614                            } else {
> 1615                                    nor->spi->flags &= ~SPI_XFER_U_PAGE;
> 1616                            }
> /drivers/mtd/spi/spi-nor-core.c: 1600 in spi_nor_read()
> 1594                    is_ofst_odd = true;
> 1595            }
> 1596
> 1597            while (len) {
> 1598                    if (nor->addr_width == 3) {
> 1599                            if (nor->flags & SNOR_F_HAS_PARALLEL) {
> >>>     CID 510800:    (DEADCODE)
> >>>     Execution cannot reach this statement: "bank = (u32)from / 33554432U;".
> 1600                                    bank = (u32)from / (SZ_16M << 0x01);
> 1601                                    rem_bank_len = ((SZ_16M << 0x01) *
> 1602                                            (bank + 1)) - from;
> 1603                            } else {
> 1604                                    bank = (u32)from / SZ_16M;
> 1605                                    rem_bank_len = (SZ_16M * (bank
> + 1)) - from;
>
> ** CID 510799:    (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1971 in spi_nor_write()
> /drivers/mtd/spi/spi-nor-core.c: 2007 in spi_nor_write()
> /drivers/mtd/spi/spi-nor-core.c: 2004 in spi_nor_write()
>
>
> ________________________________________________________________________________________________________
> *** CID 510799:    (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1971 in spi_nor_write()
> 1965                    return 0;
> 1966
> 1967            /*
> 1968             * Cannot write to odd offset in parallel mode,
> 1969             * so write 2 bytes first
> 1970             */
> >>>     CID 510799:    (DEADCODE)
> >>>     Execution cannot reach the expression "to & 1LL" inside this statement: "if (nor->flags & SNOR_F_HAS...".
> 1971            if ((nor->flags & SNOR_F_HAS_PARALLEL) && (to & 1)) {
> 1972                    u8 two[2] = {0xff, buf[0]};
> 1973                    size_t local_retlen;
> 1974
> 1975                    ret = spi_nor_write(mtd, to & ~1, 2,
> &local_retlen, two);
> 1976                    if (ret < 0)
> /drivers/mtd/spi/spi-nor-core.c: 2007 in spi_nor_write()
> 2001                    }
> 2002                    offset = (to + i);
> 2003                    if (nor->flags & SNOR_F_HAS_PARALLEL)
> 2004                            offset /= 2;
> 2005
> 2006                    if (nor->flags & SNOR_F_HAS_STACKED) {
> >>>     CID 510799:    (DEADCODE)
> >>>     Execution cannot reach this statement: "if (offset >= mtd->size / 2...".
> 2007                            if (offset >= (mtd->size / 2)) {
> 2008                                    offset = offset - (mtd->size / 2);
> 2009                                    nor->spi->flags |= SPI_XFER_U_PAGE;
> 2010                            } else {
> 2011                                    nor->spi->flags &= ~SPI_XFER_U_PAGE;
> 2012                            }
> /drivers/mtd/spi/spi-nor-core.c: 2004 in spi_nor_write()
> 1998                            u64 aux = addr;
> 1999
> 2000                            page_offset = do_div(aux, nor->page_size);
> 2001                    }
> 2002                    offset = (to + i);
> 2003                    if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>>     CID 510799:    (DEADCODE)
> >>>     Execution cannot reach this statement: "offset /= 2U;".
> 2004                            offset /= 2;
> 2005
> 2006                    if (nor->flags & SNOR_F_HAS_STACKED) {
> 2007                            if (offset >= (mtd->size / 2)) {
> 2008                                    offset = offset - (mtd->size / 2);
> 2009                                    nor->spi->flags |= SPI_XFER_U_PAGE;
>
> ** CID 510798:  Resource leaks  (RESOURCE_LEAK)
> /lib/mbedtls/x509_cert_parser.c: 220 in x509_populate_signature_params()
>
>
> ________________________________________________________________________________________________________
> *** CID 510798:  Resource leaks  (RESOURCE_LEAK)
> /lib/mbedtls/x509_cert_parser.c: 220 in x509_populate_signature_params()
> 214             }
> 215
> 216             ret = hash_calculate(s->hash_algo, &region, 1, s->digest);
> 217             if (!ret)
> 218                     *sig = s;
> 219
> >>>     CID 510798:  Resource leaks  (RESOURCE_LEAK)
> >>>     Variable "s" going out of scope leaks the storage it points to.
> 220             return ret;
> 221
> 222     error_sig:
> 223             public_key_signature_free(s);
> 224             return ret;
> 225     }
>
> ** CID 510797:    (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 4628 in spi_nor_scan()
> /drivers/mtd/spi/spi-nor-core.c: 4598 in spi_nor_scan()
>
>
> ________________________________________________________________________________________________________
> *** CID 510797:    (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 4628 in spi_nor_scan()
> 4622            /* Send all the required SPI flash commands to
> initialize device */
> 4623            ret = spi_nor_init(nor);
> 4624            if (ret)
> 4625                    return ret;
> 4626
> 4627            if (nor->flags & SNOR_F_HAS_STACKED) {
> >>>     CID 510797:    (DEADCODE)
> >>>     Execution cannot reach this statement: "nor->spi->flags |= 0x10UL;".
> 4628                    nor->spi->flags |= SPI_XFER_U_PAGE;
> 4629                    ret = spi_nor_init(nor);
> 4630                    if (ret)
> 4631                            return ret;
> 4632                    nor->spi->flags &= ~SPI_XFER_U_PAGE;
> 4633            }
> /drivers/mtd/spi/spi-nor-core.c: 4598 in spi_nor_scan()
> 4592                    nor->addr_width = info->addr_width;
> 4593            } else {
> 4594                    nor->addr_width = 3;
> 4595            }
> 4596
> 4597            if (nor->flags & (SNOR_F_HAS_PARALLEL | SNOR_F_HAS_STACKED))
> >>>     CID 510797:    (DEADCODE)
> >>>     Execution cannot reach this statement: "shift = true;".
> 4598                    shift = 1;
> 4599            if (nor->addr_width == 3 && (mtd->size >> shift) > SZ_16M) {
> 4600     #ifndef CONFIG_SPI_FLASH_BAR
> 4601                    /* enable 4-byte addressing if the device
> exceeds 16MiB */
> 4602                    nor->addr_width = 4;
> 4603                    if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
>
> ** CID 510796:  Insecure data handling  (TAINTED_SCALAR)
>
>
> ________________________________________________________________________________________________________
> *** CID 510796:  Insecure data handling  (TAINTED_SCALAR)
> /lib/mbedtls/external/mbedtls/library/rsa.c: 1316 in rsa_prepare_blinding()
> 1310             }
> 1311
> 1312             MBEDTLS_MPI_CHK(mbedtls_mpi_fill_random(&ctx->Vf,
> ctx->len - 1, f_rng, p_rng));
> 1313
> 1314             /* Compute Vf^-1 as R * (R Vf)^-1 to avoid leaks from
> inv_mod. */
> 1315             MBEDTLS_MPI_CHK(mbedtls_mpi_fill_random(&R, ctx->len
> - 1, f_rng, p_rng));
> >>>     CID 510796:  Insecure data handling  (TAINTED_SCALAR)
> >>>     Passing tainted expression "*ctx->Vf.p" to "mbedtls_mpi_mul_mpi", which uses it as an offset.
> 1316             MBEDTLS_MPI_CHK(mbedtls_mpi_mul_mpi(&ctx->Vi, &ctx->Vf, &R));
> 1317             MBEDTLS_MPI_CHK(mbedtls_mpi_mod_mpi(&ctx->Vi,
> &ctx->Vi, &ctx->N));
> 1318
> 1319             /* At this point, Vi is invertible mod N if and only
> if both Vf and R
> 1320              * are invertible mod N. If one of them isn't, we
> don't need to know
> 1321              * which one, we just loop and choose new values for
> both of them.
>
> ** CID 510795:  Control flow issues  (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 4271 in spi_nor_init()
>
>
> ________________________________________________________________________________________________________
> *** CID 510795:  Control flow issues  (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 4271 in spi_nor_init()
> 4265
> 4266     static int spi_nor_init(struct spi_nor *nor)
> 4267     {
> 4268            int err;
> 4269
> 4270            if (nor->flags & SNOR_F_HAS_PARALLEL)
> >>>     CID 510795:  Control flow issues  (DEADCODE)
> >>>     Execution cannot reach this statement: "nor->spi->flags |= 3UL;".
> 4271                    nor->spi->flags |= SPI_NOR_ENABLE_MULTI_CS;
> 4272
> 4273            err = spi_nor_octal_dtr_enable(nor);
> 4274            if (err) {
> 4275                    dev_dbg(nor->dev, "Octal DTR mode not supported\n");
> 4276                    return err;
>
> ** CID 510794:  Control flow issues  (NO_EFFECT)
> /lib/mbedtls/x509_cert_parser.c: 78 in x509_populate_dn_name_string()
>
>
> ________________________________________________________________________________________________________
> *** CID 510794:  Control flow issues  (NO_EFFECT)
> /lib/mbedtls/x509_cert_parser.c: 78 in x509_populate_dn_name_string()
> 72      do {
> 73              name_str = kzalloc(len, GFP_KERNEL);
> 74              if (!name_str)
> 75                      return NULL;
> 76
> 77              wb = mbedtls_x509_dn_gets(name_str, len, name);
> >>>     CID 510794:  Control flow issues  (NO_EFFECT)
> >>>     This less-than-zero comparison of an unsigned value is never true. "wb < 0UL".
> 78              if (wb < 0) {
> 79                      pr_err("Get DN string failed, ret:-0x%04x\n",
> 80                             (unsigned int)-wb);
> 81                      kfree(name_str);
> 82                      len = len * 2; /* Try with a bigger buffer */
> 83              }
>
> ----- End forwarded message -----
>
> --
> Tom

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: New Defects reported by Coverity Scan for Das U-Boot
  2024-10-16  6:12 ` Ilias Apalodimas
@ 2024-10-16  8:20   ` Abbarapu, Venkatesh
  0 siblings, 0 replies; 20+ messages in thread
From: Abbarapu, Venkatesh @ 2024-10-16  8:20 UTC (permalink / raw)
  To: Ilias Apalodimas, Tom Rini
  Cc: u-boot@lists.denx.de, Vignesh R, Takahiro Kuwano, Tudor Ambarus,
	Pratyush Yadav, Ashok Reddy Soma, Joakim Tjernlund, Raymond Mao

Hi Tom,
I will try to check for the spi nor core issues mentioned below.

Thanks
Venkatesh

> -----Original Message-----
> From: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Sent: Wednesday, October 16, 2024 11:43 AM
> To: Tom Rini <trini@konsulko.com>
> Cc: u-boot@lists.denx.de; Vignesh R <vigneshr@ti.com>; Takahiro Kuwano
> <Takahiro.Kuwano@infineon.com>; Tudor Ambarus <tudor.ambarus@linaro.org>;
> Abbarapu, Venkatesh <venkatesh.abbarapu@amd.com>; Pratyush Yadav
> <p.yadav@ti.com>; Ashok Reddy Soma <ashok.reddy.soma@amd.com>; Joakim
> Tjernlund <joakim.tjernlund@infinera.com>; Raymond Mao
> <raymond.mao@linaro.org>
> Subject: Re: New Defects reported by Coverity Scan for Das U-Boot
> 
> HI Tom,
> 
> We'll have a look on the mbedTLS reports later today
> 
> Thanks
> /Ilias
> 
> On Wed, 16 Oct 2024 at 06:47, Tom Rini <trini@konsulko.com> wrote:
> >
> > Hey all, here's the latest report.
> >
> > ---------- Forwarded message ---------
> > From: <scan-admin@coverity.com>
> > Date: Tue, Oct 15, 2024 at 5:06 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.
> >
> > 22 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> >
> >
> > New defect(s) Reported-by: Coverity Scan Showing 20 of 22 defect(s)
> >
> >
> > ** CID 510813:  Control flow issues  (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 1652 in spi_nor_read()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510813:  Control flow issues  (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 1652 in spi_nor_read()
> > 1646                            goto read_err;
> > 1647                    }
> > 1648                    if (ret < 0)
> > 1649                            goto read_err;
> > 1650
> > 1651                    if (is_ofst_odd == true) {
> > >>>     CID 510813:  Control flow issues  (DEADCODE)
> > >>>     Execution cannot reach this statement: "memmove(buf, buf + 1, len -...".
> > 1652                            memmove(buf, (buf + 1), (len - 1));
> > 1653                            *retlen += (ret - 1);
> > 1654                            buf += ret - 1;
> > 1655                            is_ofst_odd = false;
> > 1656                    } else {
> > 1657                            *retlen += ret;
> >
> > ** CID 510812:    (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 3573 in spi_nor_select_erase()
> > /drivers/mtd/spi/spi-nor-core.c: 3584 in spi_nor_select_erase()
> > /drivers/mtd/spi/spi-nor-core.c: 3610 in spi_nor_select_erase()
> > /drivers/mtd/spi/spi-nor-core.c: 3597 in spi_nor_select_erase()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510812:    (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 3573 in spi_nor_select_erase()
> > 3567                    /*
> > 3568                     * In parallel-memories the erase operation is
> > 3569                     * performed on both the flashes simultaneously
> > 3570                     * so, double the erasesize.
> > 3571                     */
> > 3572                    if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>>     CID 510812:    (DEADCODE)
> > >>>     Execution cannot reach this statement: "mtd->erasesize = 8192U;".
> > 3573                            mtd->erasesize = 4096 * 2;
> > 3574                    else
> > 3575                            mtd->erasesize = 4096;
> > 3576            } else if (info->flags & SECT_4K_PMC) {
> > 3577                    nor->erase_opcode = SPINOR_OP_BE_4K_PMC;
> > 3578                    /*
> > /drivers/mtd/spi/spi-nor-core.c: 3584 in spi_nor_select_erase()
> > 3578                    /*
> > 3579                     * In parallel-memories the erase operation is
> > 3580                     * performed on both the flashes simultaneously
> > 3581                     * so, double the erasesize.
> > 3582                     */
> > 3583                    if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>>     CID 510812:    (DEADCODE)
> > >>>     Execution cannot reach this statement: "mtd->erasesize = 8192U;".
> > 3584                            mtd->erasesize = 4096 * 2;
> > 3585                    else
> > 3586                            mtd->erasesize = 4096;
> > 3587            } else
> > 3588     #endif
> > 3589            {
> > /drivers/mtd/spi/spi-nor-core.c: 3610 in spi_nor_select_erase()
> > 3604                    /*
> > 3605                     * In parallel-memories the erase operation is
> > 3606                     * performed on both the flashes simultaneously
> > 3607                     * so, double the erasesize.
> > 3608                     */
> > 3609                    if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>>     CID 510812:    (DEADCODE)
> > >>>     Execution cannot reach this statement: "mtd->erasesize = 8192U;".
> > 3610                            mtd->erasesize = 4096 * 2;
> > 3611                    else
> > 3612                            mtd->erasesize = 4096;
> > 3613            }
> > 3614
> > 3615            return 0;
> > /drivers/mtd/spi/spi-nor-core.c: 3597 in spi_nor_select_erase()
> > 3591                    /*
> > 3592                     * In parallel-memories the erase operation is
> > 3593                     * performed on both the flashes simultaneously
> > 3594                     * so, double the erasesize.
> > 3595                     */
> > 3596                    if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>>     CID 510812:    (DEADCODE)
> > >>>     Execution cannot reach this statement: "mtd->erasesize = info->sect...".
> > 3597                            mtd->erasesize = info->sector_size * 2;
> > 3598                    else
> > 3599                            mtd->erasesize = info->sector_size;
> > 3600            }
> > 3601
> > 3602            if ((JEDEC_MFR(info) == SNOR_MFR_SST) && info->flags &
> > SECT_4K) {
> >
> > ** CID 510811:    (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 1134 in spi_nor_erase()
> > /drivers/mtd/spi/spi-nor-core.c: 1137 in spi_nor_erase()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510811:    (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 1134 in spi_nor_erase()
> > 1128                            addr_known = false;
> > 1129                            ret = -EINTR;
> > 1130                            goto erase_err;
> > 1131                    }
> > 1132                    offset = addr;
> > 1133                    if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>>     CID 510811:    (DEADCODE)
> > >>>     Execution cannot reach this statement: "offset /= 2U;".
> > 1134                            offset /= 2;
> > 1135
> > 1136                    if (nor->flags & SNOR_F_HAS_STACKED) {
> > 1137                            if (offset >= (mtd->size / 2)) {
> > 1138                                    offset = offset - (mtd->size / 2);
> > 1139                                    nor->spi->flags |= SPI_XFER_U_PAGE;
> > /drivers/mtd/spi/spi-nor-core.c: 1137 in spi_nor_erase()
> > 1131                    }
> > 1132                    offset = addr;
> > 1133                    if (nor->flags & SNOR_F_HAS_PARALLEL)
> > 1134                            offset /= 2;
> > 1135
> > 1136                    if (nor->flags & SNOR_F_HAS_STACKED) {
> > >>>     CID 510811:    (DEADCODE)
> > >>>     Execution cannot reach this statement: "if (offset >= mtd->size / 2...".
> > 1137                            if (offset >= (mtd->size / 2)) {
> > 1138                                    offset = offset - (mtd->size / 2);
> > 1139                                    nor->spi->flags |= SPI_XFER_U_PAGE;
> > 1140                            } else {
> > 1141                                    nor->spi->flags &= ~SPI_XFER_U_PAGE;
> > 1142                            }
> >
> > ** CID 510810:  Control flow issues  (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 1556 in spi_nor_read_id()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510810:  Control flow issues  (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 1556 in spi_nor_read_id()
> > 1550     {
> > 1551            int                     tmp;
> > 1552            u8                      id[SPI_NOR_MAX_ID_LEN];
> > 1553            const struct flash_info *info;
> > 1554
> > 1555            if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>>     CID 510810:  Control flow issues  (DEADCODE)
> > >>>     Execution cannot reach this statement: "nor->spi->flags |= 0x100;".
> > 1556                    nor->spi->flags |= SPI_XFER_LOWER;
> > 1557
> > 1558            tmp = nor->read_reg(nor, SPINOR_OP_RDID, id,
> > SPI_NOR_MAX_ID_LEN);
> > 1559            if (tmp < 0) {
> > 1560                    dev_dbg(nor->dev, "error %d reading JEDEC ID\n", tmp);
> > 1561                    return ERR_PTR(tmp);
> >
> > ** CID 510809:  Resource leaks  (RESOURCE_LEAK)
> > /lib/mbedtls/pkcs7_parser.c: 385 in x509_populate_sinfo()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510809:  Resource leaks  (RESOURCE_LEAK)
> > /lib/mbedtls/pkcs7_parser.c: 385 in x509_populate_sinfo()
> > 379                                   signed_info);
> > 380             if (ret)
> > 381                     goto out_err_sinfo;
> > 382
> > 383     no_authattrs:
> > 384             *sinfo = signed_info;
> > >>>     CID 510809:  Resource leaks  (RESOURCE_LEAK)
> > >>>     Variable "mctx" going out of scope leaks the storage it points to.
> > 385             return 0;
> > 386
> > 387     out_err_sinfo:
> > 388             pkcs7_free_sinfo_mbedtls_ctx(mctx);
> > 389     out_no_mctx:
> > 390             public_key_signature_free(s);
> >
> > ** CID 510808:  Control flow issues  (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 724 in spi_nor_set_4byte_opcodes()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510808:  Control flow issues  (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 724 in spi_nor_set_4byte_opcodes()
> > 718     static void spi_nor_set_4byte_opcodes(struct spi_nor *nor,
> > 719                                           const struct flash_info *info)
> > 720     {
> > 721             bool shift = 0;
> > 722
> > 723             if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>>     CID 510808:  Control flow issues  (DEADCODE)
> > >>>     Execution cannot reach this statement: "shift = true;".
> > 724                     shift = 1;
> > 725
> > 726             /* Do some manufacturer fixups first */
> > 727             switch (JEDEC_MFR(info)) {
> > 728             case SNOR_MFR_SPANSION:
> > 729                     /* No small sector erase for 4-byte command set */
> >
> > ** CID 510807:  Control flow issues  (DEADCODE)
> > /lib/mbedtls/external/mbedtls/library/x509_crt.c: 2750 in x509_inet_pton_ipv6()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510807:  Control flow issues  (DEADCODE)
> > /lib/mbedtls/external/mbedtls/library/x509_crt.c: 2750 in x509_inet_pton_ipv6()
> > 2744                 MBEDTLS_PUT_UINT16_BE(group, addr, nonzero_groups);
> > 2745                 nonzero_groups++;
> > 2746                 if (*p == '\0') {
> > 2747                     break;
> > 2748                 } else if (*p == '.') {
> > 2749                     /* Don't accept IPv4 too early or late */
> > >>>     CID 510807:  Control flow issues  (DEADCODE)
> > >>>     Execution cannot reach the expression "zero_group_start == -1" inside this
> statement: "if ((nonzero_groups == 0 &&...".
> > 2750                     if ((nonzero_groups == 0 && zero_group_start == -1) ||
> > 2751                         nonzero_groups >= 7) {
> > 2752                         break;
> > 2753                     }
> > 2754
> > 2755                     /* Walk back to prior ':', then parse as IPv4-mapped */
> >
> > ** CID 510806:  Control flow issues  (DEADCODE)
> > /lib/mbedtls/pkcs7_parser.c: 209 in authattrs_parse()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510806:  Control flow issues  (DEADCODE)
> > /lib/mbedtls/pkcs7_parser.c: 209 in authattrs_parse()
> > 203                                     return -EINVAL;
> > 204                     }
> > 205
> > 206                     p += seq_len;
> > 207             }
> > 208
> > >>>     CID 510806:  Control flow issues  (DEADCODE)
> > >>>     Execution cannot reach the expression "ret != -96" inside this statement: "if
> (ret && ret != -96)
> >   re...".
> > 209             if (ret && ret !=  MBEDTLS_ERR_ASN1_OUT_OF_DATA)
> > 210                     return ret;
> > 211
> > 212             msg->have_authattrs = true;
> > 213
> > 214             /*
> >
> > ** CID 510805:  Memory - illegal accesses  (OVERRUN)
> > /lib/rsa/rsa-keyprop.c: 678 in rsa_gen_key_prop()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510805:  Memory - illegal accesses  (OVERRUN)
> > /lib/rsa/rsa-keyprop.c: 678 in rsa_gen_key_prop()
> > 672             (*prop)->num_bits = (rsa_key.n_sz - i) * 8;
> > 673             (*prop)->modulus = malloc(rsa_key.n_sz - i);
> > 674             if (!(*prop)->modulus) {
> > 675                     ret = -ENOMEM;
> > 676                     goto out;
> > 677             }
> > >>>     CID 510805:  Memory - illegal accesses  (OVERRUN)
> > >>>     Overrunning dynamic array "rsa_key.n" at offset corresponding to index
> variable "i".
> > 678             memcpy((void *)(*prop)->modulus, &rsa_key.n[i],
> > rsa_key.n_sz - i);
> > 679
> > 680             n = calloc(sizeof(uint32_t), 1 + ((*prop)->num_bits >> 5));
> > 681             rr = calloc(sizeof(uint32_t), 1 + (((*prop)->num_bits
> > * 2) >> 5));
> > 682             rrtmp = calloc(sizeof(uint32_t), 2 +
> > (((*prop)->num_bits * 2) >> 5));
> > 683             if (!n || !rr || !rrtmp) {
> >
> > ** CID 510804:  Integer handling issues  (CONSTANT_EXPRESSION_RESULT)
> > /drivers/mtd/spi/spi-nor-core.c: 1556 in spi_nor_read_id()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510804:  Integer handling issues  (CONSTANT_EXPRESSION_RESULT)
> > /drivers/mtd/spi/spi-nor-core.c: 1556 in spi_nor_read_id()
> > 1550     {
> > 1551            int                     tmp;
> > 1552            u8                      id[SPI_NOR_MAX_ID_LEN];
> > 1553            const struct flash_info *info;
> > 1554
> > 1555            if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>>     CID 510804:  Integer handling issues
> (CONSTANT_EXPRESSION_RESULT)
> > >>>     In "nor->spi->flags |= 256 /* 1 << 8 */", wider "256 /* 1 << 8 */" has high-
> order bits (0x100) that don't affect the narrower left-hand side.
> > 1556                    nor->spi->flags |= SPI_XFER_LOWER;
> > 1557
> > 1558            tmp = nor->read_reg(nor, SPINOR_OP_RDID, id,
> > SPI_NOR_MAX_ID_LEN);
> > 1559            if (tmp < 0) {
> > 1560                    dev_dbg(nor->dev, "error %d reading JEDEC ID\n", tmp);
> > 1561                    return ERR_PTR(tmp);
> >
> > ** CID 510803:  Code maintainability issues  (UNUSED_VALUE)
> > /drivers/mtd/spi/spi-nor-core.c: 1138 in spi_nor_erase()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510803:  Code maintainability issues  (UNUSED_VALUE)
> > /drivers/mtd/spi/spi-nor-core.c: 1138 in spi_nor_erase()
> > 1132                    offset = addr;
> > 1133                    if (nor->flags & SNOR_F_HAS_PARALLEL)
> > 1134                            offset /= 2;
> > 1135
> > 1136                    if (nor->flags & SNOR_F_HAS_STACKED) {
> > 1137                            if (offset >= (mtd->size / 2)) {
> > >>>     CID 510803:  Code maintainability issues  (UNUSED_VALUE)
> > >>>     Assigning value from "offset - mtd->size / 2ULL" to "offset" here, but that
> stored value is overwritten before it can be used.
> > 1138                                    offset = offset - (mtd->size / 2);
> > 1139                                    nor->spi->flags |= SPI_XFER_U_PAGE;
> > 1140                            } else {
> > 1141                                    nor->spi->flags &= ~SPI_XFER_U_PAGE;
> > 1142                            }
> > 1143                    }
> >
> > ** CID 510802:  Control flow issues  (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 504 in read_sr()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510802:  Control flow issues  (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 504 in read_sr()
> > 498              * discard the second byte.
> > 499              */
> > 500             if (spi_nor_protocol_is_dtr(nor->reg_proto))
> > 501                     op.data.nbytes = 2;
> > 502
> > 503             if (nor->flags & SNOR_F_HAS_PARALLEL) {
> > >>>     CID 510802:  Control flow issues  (DEADCODE)
> > >>>     Execution cannot reach this statement: "op.data.nbytes = 2U;".
> > 504                     op.data.nbytes = 2;
> > 505                     ret = spi_nor_read_write_reg(nor, &op, &val[0]);
> > 506                     if (ret < 0) {
> > 507                             pr_debug("error %d reading SR\n", (int)ret);
> > 508                             return ret;
> > 509                     }
> >
> > ** CID 510801:  Null pointer dereferences  (FORWARD_NULL)
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510801:  Null pointer dereferences  (FORWARD_NULL)
> > /lib/ecdsa/ecdsa-libcrypto.c: 365 in ecdsa_add_verify_data()
> > 359             struct signer ctx;
> > 360             int ret;
> > 361
> > 362             fdt_key_name = info->keyname ? info->keyname : "default-key";
> > 363             ret = prepare_ctx(&ctx, info);
> > 364             if (ret >= 0) {
> > >>>     CID 510801:  Null pointer dereferences  (FORWARD_NULL)
> > >>>     Passing "info" to "do_add", which dereferences null "info->keyname".
> > 365                     ret = do_add(&ctx, fdt, fdt_key_name, info);
> > 366                     if (ret < 0)
> > 367                             ret = ret == -FDT_ERR_NOSPACE ? -ENOSPC : -EIO;
> > 368             }
> > 369
> > 370             free_ctx(&ctx);
> > 371             return ret;
> >
> > ** CID 510800:    (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 1620 in spi_nor_read()
> > /drivers/mtd/spi/spi-nor-core.c: 1590 in spi_nor_read()
> > /drivers/mtd/spi/spi-nor-core.c: 1611 in spi_nor_read()
> > /drivers/mtd/spi/spi-nor-core.c: 1600 in spi_nor_read()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510800:    (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 1620 in spi_nor_read()
> > 1614                            } else {
> > 1615                                    nor->spi->flags &= ~SPI_XFER_U_PAGE;
> > 1616                            }
> > 1617                    }
> > 1618
> > 1619                    if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>>     CID 510800:    (DEADCODE)
> > >>>     Execution cannot reach this statement: "offset /= 2LL;".
> > 1620                            offset /= 2;
> > 1621
> > 1622                    if (nor->addr_width == 3) {
> > 1623     #ifdef CONFIG_SPI_FLASH_BAR
> > 1624                            ret = write_bar(nor, offset);
> > 1625                            if (ret < 0)
> > /drivers/mtd/spi/spi-nor-core.c: 1590 in spi_nor_read()
> > 1584            u32 rem_bank_len = 0;
> > 1585            u8 bank;
> > 1586            bool is_ofst_odd = false;
> > 1587
> > 1588            dev_dbg(nor->dev, "from 0x%08x, len %zd\n", (u32)from, len);
> > 1589
> > >>>     CID 510800:    (DEADCODE)
> > >>>     Execution cannot reach the expression "offset & 1LL" inside this statement:
> "if (nor->flags & SNOR_F_HAS...".
> > 1590            if ((nor->flags & SNOR_F_HAS_PARALLEL) && (offset & 1)) {
> > 1591                /* We can hit this case when we use file system
> > like ubifs */
> > 1592                    from--;
> > 1593                    len++;
> > 1594                    is_ofst_odd = true;
> > 1595            }
> > /drivers/mtd/spi/spi-nor-core.c: 1611 in spi_nor_read()
> > 1605                                    rem_bank_len = (SZ_16M * (bank
> > + 1)) - from;
> > 1606                            }
> > 1607                    }
> > 1608                    offset = from;
> > 1609
> > 1610                    if (nor->flags & SNOR_F_HAS_STACKED) {
> > >>>     CID 510800:    (DEADCODE)
> > >>>     Execution cannot reach this statement: "if (offset >= mtd->size / 2...".
> > 1611                            if (offset >= (mtd->size / 2)) {
> > 1612                                    offset = offset - (mtd->size / 2);
> > 1613                                    nor->spi->flags |= SPI_XFER_U_PAGE;
> > 1614                            } else {
> > 1615                                    nor->spi->flags &= ~SPI_XFER_U_PAGE;
> > 1616                            }
> > /drivers/mtd/spi/spi-nor-core.c: 1600 in spi_nor_read()
> > 1594                    is_ofst_odd = true;
> > 1595            }
> > 1596
> > 1597            while (len) {
> > 1598                    if (nor->addr_width == 3) {
> > 1599                            if (nor->flags & SNOR_F_HAS_PARALLEL) {
> > >>>     CID 510800:    (DEADCODE)
> > >>>     Execution cannot reach this statement: "bank = (u32)from / 33554432U;".
> > 1600                                    bank = (u32)from / (SZ_16M << 0x01);
> > 1601                                    rem_bank_len = ((SZ_16M << 0x01) *
> > 1602                                            (bank + 1)) - from;
> > 1603                            } else {
> > 1604                                    bank = (u32)from / SZ_16M;
> > 1605                                    rem_bank_len = (SZ_16M * (bank
> > + 1)) - from;
> >
> > ** CID 510799:    (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 1971 in spi_nor_write()
> > /drivers/mtd/spi/spi-nor-core.c: 2007 in spi_nor_write()
> > /drivers/mtd/spi/spi-nor-core.c: 2004 in spi_nor_write()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510799:    (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 1971 in spi_nor_write()
> > 1965                    return 0;
> > 1966
> > 1967            /*
> > 1968             * Cannot write to odd offset in parallel mode,
> > 1969             * so write 2 bytes first
> > 1970             */
> > >>>     CID 510799:    (DEADCODE)
> > >>>     Execution cannot reach the expression "to & 1LL" inside this statement: "if
> (nor->flags & SNOR_F_HAS...".
> > 1971            if ((nor->flags & SNOR_F_HAS_PARALLEL) && (to & 1)) {
> > 1972                    u8 two[2] = {0xff, buf[0]};
> > 1973                    size_t local_retlen;
> > 1974
> > 1975                    ret = spi_nor_write(mtd, to & ~1, 2,
> > &local_retlen, two);
> > 1976                    if (ret < 0)
> > /drivers/mtd/spi/spi-nor-core.c: 2007 in spi_nor_write()
> > 2001                    }
> > 2002                    offset = (to + i);
> > 2003                    if (nor->flags & SNOR_F_HAS_PARALLEL)
> > 2004                            offset /= 2;
> > 2005
> > 2006                    if (nor->flags & SNOR_F_HAS_STACKED) {
> > >>>     CID 510799:    (DEADCODE)
> > >>>     Execution cannot reach this statement: "if (offset >= mtd->size / 2...".
> > 2007                            if (offset >= (mtd->size / 2)) {
> > 2008                                    offset = offset - (mtd->size / 2);
> > 2009                                    nor->spi->flags |= SPI_XFER_U_PAGE;
> > 2010                            } else {
> > 2011                                    nor->spi->flags &= ~SPI_XFER_U_PAGE;
> > 2012                            }
> > /drivers/mtd/spi/spi-nor-core.c: 2004 in spi_nor_write()
> > 1998                            u64 aux = addr;
> > 1999
> > 2000                            page_offset = do_div(aux, nor->page_size);
> > 2001                    }
> > 2002                    offset = (to + i);
> > 2003                    if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>>     CID 510799:    (DEADCODE)
> > >>>     Execution cannot reach this statement: "offset /= 2U;".
> > 2004                            offset /= 2;
> > 2005
> > 2006                    if (nor->flags & SNOR_F_HAS_STACKED) {
> > 2007                            if (offset >= (mtd->size / 2)) {
> > 2008                                    offset = offset - (mtd->size / 2);
> > 2009                                    nor->spi->flags |= SPI_XFER_U_PAGE;
> >
> > ** CID 510798:  Resource leaks  (RESOURCE_LEAK)
> > /lib/mbedtls/x509_cert_parser.c: 220 in x509_populate_signature_params()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510798:  Resource leaks  (RESOURCE_LEAK)
> > /lib/mbedtls/x509_cert_parser.c: 220 in x509_populate_signature_params()
> > 214             }
> > 215
> > 216             ret = hash_calculate(s->hash_algo, &region, 1, s->digest);
> > 217             if (!ret)
> > 218                     *sig = s;
> > 219
> > >>>     CID 510798:  Resource leaks  (RESOURCE_LEAK)
> > >>>     Variable "s" going out of scope leaks the storage it points to.
> > 220             return ret;
> > 221
> > 222     error_sig:
> > 223             public_key_signature_free(s);
> > 224             return ret;
> > 225     }
> >
> > ** CID 510797:    (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 4628 in spi_nor_scan()
> > /drivers/mtd/spi/spi-nor-core.c: 4598 in spi_nor_scan()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510797:    (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 4628 in spi_nor_scan()
> > 4622            /* Send all the required SPI flash commands to
> > initialize device */
> > 4623            ret = spi_nor_init(nor);
> > 4624            if (ret)
> > 4625                    return ret;
> > 4626
> > 4627            if (nor->flags & SNOR_F_HAS_STACKED) {
> > >>>     CID 510797:    (DEADCODE)
> > >>>     Execution cannot reach this statement: "nor->spi->flags |= 0x10UL;".
> > 4628                    nor->spi->flags |= SPI_XFER_U_PAGE;
> > 4629                    ret = spi_nor_init(nor);
> > 4630                    if (ret)
> > 4631                            return ret;
> > 4632                    nor->spi->flags &= ~SPI_XFER_U_PAGE;
> > 4633            }
> > /drivers/mtd/spi/spi-nor-core.c: 4598 in spi_nor_scan()
> > 4592                    nor->addr_width = info->addr_width;
> > 4593            } else {
> > 4594                    nor->addr_width = 3;
> > 4595            }
> > 4596
> > 4597            if (nor->flags & (SNOR_F_HAS_PARALLEL |
> SNOR_F_HAS_STACKED))
> > >>>     CID 510797:    (DEADCODE)
> > >>>     Execution cannot reach this statement: "shift = true;".
> > 4598                    shift = 1;
> > 4599            if (nor->addr_width == 3 && (mtd->size >> shift) > SZ_16M) {
> > 4600     #ifndef CONFIG_SPI_FLASH_BAR
> > 4601                    /* enable 4-byte addressing if the device
> > exceeds 16MiB */
> > 4602                    nor->addr_width = 4;
> > 4603                    if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
> >
> > ** CID 510796:  Insecure data handling  (TAINTED_SCALAR)
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510796:  Insecure data handling  (TAINTED_SCALAR)
> > /lib/mbedtls/external/mbedtls/library/rsa.c: 1316 in rsa_prepare_blinding()
> > 1310             }
> > 1311
> > 1312             MBEDTLS_MPI_CHK(mbedtls_mpi_fill_random(&ctx->Vf,
> > ctx->len - 1, f_rng, p_rng));
> > 1313
> > 1314             /* Compute Vf^-1 as R * (R Vf)^-1 to avoid leaks from
> > inv_mod. */
> > 1315             MBEDTLS_MPI_CHK(mbedtls_mpi_fill_random(&R, ctx->len
> > - 1, f_rng, p_rng));
> > >>>     CID 510796:  Insecure data handling  (TAINTED_SCALAR)
> > >>>     Passing tainted expression "*ctx->Vf.p" to "mbedtls_mpi_mul_mpi", which
> uses it as an offset.
> > 1316             MBEDTLS_MPI_CHK(mbedtls_mpi_mul_mpi(&ctx->Vi, &ctx->Vf,
> &R));
> > 1317             MBEDTLS_MPI_CHK(mbedtls_mpi_mod_mpi(&ctx->Vi,
> > &ctx->Vi, &ctx->N));
> > 1318
> > 1319             /* At this point, Vi is invertible mod N if and only
> > if both Vf and R
> > 1320              * are invertible mod N. If one of them isn't, we
> > don't need to know
> > 1321              * which one, we just loop and choose new values for
> > both of them.
> >
> > ** CID 510795:  Control flow issues  (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 4271 in spi_nor_init()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510795:  Control flow issues  (DEADCODE)
> > /drivers/mtd/spi/spi-nor-core.c: 4271 in spi_nor_init()
> > 4265
> > 4266     static int spi_nor_init(struct spi_nor *nor)
> > 4267     {
> > 4268            int err;
> > 4269
> > 4270            if (nor->flags & SNOR_F_HAS_PARALLEL)
> > >>>     CID 510795:  Control flow issues  (DEADCODE)
> > >>>     Execution cannot reach this statement: "nor->spi->flags |= 3UL;".
> > 4271                    nor->spi->flags |= SPI_NOR_ENABLE_MULTI_CS;
> > 4272
> > 4273            err = spi_nor_octal_dtr_enable(nor);
> > 4274            if (err) {
> > 4275                    dev_dbg(nor->dev, "Octal DTR mode not supported\n");
> > 4276                    return err;
> >
> > ** CID 510794:  Control flow issues  (NO_EFFECT)
> > /lib/mbedtls/x509_cert_parser.c: 78 in x509_populate_dn_name_string()
> >
> >
> >
> ___________________________________________________________________
> _____________________________________
> > *** CID 510794:  Control flow issues  (NO_EFFECT)
> > /lib/mbedtls/x509_cert_parser.c: 78 in x509_populate_dn_name_string()
> > 72      do {
> > 73              name_str = kzalloc(len, GFP_KERNEL);
> > 74              if (!name_str)
> > 75                      return NULL;
> > 76
> > 77              wb = mbedtls_x509_dn_gets(name_str, len, name);
> > >>>     CID 510794:  Control flow issues  (NO_EFFECT)
> > >>>     This less-than-zero comparison of an unsigned value is never true. "wb <
> 0UL".
> > 78              if (wb < 0) {
> > 79                      pr_err("Get DN string failed, ret:-0x%04x\n",
> > 80                             (unsigned int)-wb);
> > 81                      kfree(name_str);
> > 82                      len = len * 2; /* Try with a bigger buffer */
> > 83              }
> >
> > ----- End forwarded message -----
> >
> > --
> > Tom

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New Defects reported by Coverity Scan for Das U-Boot
  2024-10-16  3:47 Fwd: " Tom Rini
  2024-10-16  6:12 ` Ilias Apalodimas
@ 2024-10-16 15:23 ` Raymond Mao
  1 sibling, 0 replies; 20+ messages in thread
From: Raymond Mao @ 2024-10-16 15:23 UTC (permalink / raw)
  To: Tom Rini
  Cc: u-boot, Vignesh R, Takahiro Kuwano, Tudor Ambarus,
	Venkatesh Yadav Abbarapu, Pratyush Yadav, Ashok Reddy Soma,
	Joakim Tjernlund, Ilias Apalodimas

Hi Tom,

On Tue, 15 Oct 2024 at 23:47, Tom Rini <trini@konsulko.com> wrote:

> Hey all, here's the latest report.
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Tue, Oct 15, 2024 at 5:06 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.
>
> 22 new defect(s) introduced to Das U-Boot found with Coverity Scan.
>
>
> New defect(s) Reported-by: Coverity Scan
> Showing 20 of 22 defect(s)
>
> [snip]


>
> *** CID 510809:  Resource leaks  (RESOURCE_LEAK)
> /lib/mbedtls/pkcs7_parser.c: 385 in x509_populate_sinfo()
> 379                                   signed_info);
> 380             if (ret)
> 381                     goto out_err_sinfo;
> 382
> 383     no_authattrs:
> 384             *sinfo = signed_info;
> >>>     CID 510809:  Resource leaks  (RESOURCE_LEAK)
> >>>     Variable "mctx" going out of scope leaks the storage it points to.
> 385             return 0;
> 386
> 387     out_err_sinfo:
> 388             pkcs7_free_sinfo_mbedtls_ctx(mctx);
> 389     out_no_mctx:
> 390             public_key_signature_free(s);
>
> I will post a patch to fix this defect  .
[snip]


> *** CID 510807:  Control flow issues  (DEADCODE)
> /lib/mbedtls/external/mbedtls/library/x509_crt.c: 2750 in
> x509_inet_pton_ipv6()
> 2744                 MBEDTLS_PUT_UINT16_BE(group, addr, nonzero_groups);
> 2745                 nonzero_groups++;
> 2746                 if (*p == '\0') {
> 2747                     break;
> 2748                 } else if (*p == '.') {
> 2749                     /* Don't accept IPv4 too early or late */
> >>>     CID 510807:  Control flow issues  (DEADCODE)
> >>>     Execution cannot reach the expression "zero_group_start == -1"
> inside this statement: "if ((nonzero_groups == 0 &&...".
> 2750                     if ((nonzero_groups == 0 && zero_group_start ==
> -1) ||
> 2751                         nonzero_groups >= 7) {
> 2752                         break;
> 2753                     }
> 2754
> 2755                     /* Walk back to prior ':', then parse as
> IPv4-mapped */
>
> This one belongs to MbedTLS lib itself, I will send a separate patch to
the MbedTLS project.
[snip]

*** CID 510806:  Control flow issues  (DEADCODE)
> /lib/mbedtls/pkcs7_parser.c: 209 in authattrs_parse()
> 203                                     return -EINVAL;
> 204                     }
> 205
> 206                     p += seq_len;
> 207             }
> 208
> >>>     CID 510806:  Control flow issues  (DEADCODE)
> >>>     Execution cannot reach the expression "ret != -96" inside this
> statement: "if (ret && ret != -96)
>   re...".
> 209             if (ret && ret !=  MBEDTLS_ERR_ASN1_OUT_OF_DATA)
> 210                     return ret;
> 211
> 212             msg->have_authattrs = true;
> 213
> 214             /*
>
> I will post a patch to fix this defect  .
[snip]


> *** CID 510798:  Resource leaks  (RESOURCE_LEAK)
> /lib/mbedtls/x509_cert_parser.c: 220 in x509_populate_signature_params()
> 214             }
> 215
> 216             ret = hash_calculate(s->hash_algo, &region, 1, s->digest);
> 217             if (!ret)
> 218                     *sig = s;
> 219
> >>>     CID 510798:  Resource leaks  (RESOURCE_LEAK)
> >>>     Variable "s" going out of scope leaks the storage it points to.
> 220             return ret;
> 221
> 222     error_sig:
> 223             public_key_signature_free(s);
> 224             return ret;
> 225     }
>
> This is false-positive, we need to keep the memory pointed by 's' as the
signature
returned to the caller.
[snip]

*** CID 510796:  Insecure data handling  (TAINTED_SCALAR)
> /lib/mbedtls/external/mbedtls/library/rsa.c: 1316 in rsa_prepare_blinding()
> 1310             }
> 1311
> 1312             MBEDTLS_MPI_CHK(mbedtls_mpi_fill_random(&ctx->Vf,
> ctx->len - 1, f_rng, p_rng));
> 1313
> 1314             /* Compute Vf^-1 as R * (R Vf)^-1 to avoid leaks from
> inv_mod. */
> 1315             MBEDTLS_MPI_CHK(mbedtls_mpi_fill_random(&R, ctx->len
> - 1, f_rng, p_rng));
> >>>     CID 510796:  Insecure data handling  (TAINTED_SCALAR)
> >>>     Passing tainted expression "*ctx->Vf.p" to "mbedtls_mpi_mul_mpi",
> which uses it as an offset.
> 1316             MBEDTLS_MPI_CHK(mbedtls_mpi_mul_mpi(&ctx->Vi, &ctx->Vf,
> &R));
> 1317             MBEDTLS_MPI_CHK(mbedtls_mpi_mod_mpi(&ctx->Vi,
> &ctx->Vi, &ctx->N));
> 1318
> 1319             /* At this point, Vi is invertible mod N if and only
> if both Vf and R
> 1320              * are invertible mod N. If one of them isn't, we
> don't need to know
> 1321              * which one, we just loop and choose new values for
> both of them.
>
> This one belongs to MbedTLS lib itself, I will send a separate patch to
the MbedTLS project.
[snip]

*** CID 510794:  Control flow issues  (NO_EFFECT)
> /lib/mbedtls/x509_cert_parser.c: 78 in x509_populate_dn_name_string()
> 72      do {
> 73              name_str = kzalloc(len, GFP_KERNEL);
> 74              if (!name_str)
> 75                      return NULL;
> 76
> 77              wb = mbedtls_x509_dn_gets(name_str, len, name);
> >>>     CID 510794:  Control flow issues  (NO_EFFECT)
> >>>     This less-than-zero comparison of an unsigned value is never true.
> "wb < 0UL".
> 78              if (wb < 0) {
> 79                      pr_err("Get DN string failed, ret:-0x%04x\n",
> 80                             (unsigned int)-wb);
> 81                      kfree(name_str);
> 82                      len = len * 2; /* Try with a bigger buffer */
> 83              }
>
> I will post a patch to fix this defect.

Thanks a lot for the report.

Regards,
Raymond

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: New Defects reported by Coverity Scan for Das U-Boot
  2024-12-31 13:55 Fwd: " Tom Rini
@ 2025-01-01 10:50 ` Abbarapu, Venkatesh
  2025-01-02 16:59   ` Tom Rini
  0 siblings, 1 reply; 20+ messages in thread
From: Abbarapu, Venkatesh @ 2025-01-01 10:50 UTC (permalink / raw)
  To: Tom Rini, u-boot@lists.denx.de

Hi Tom,
These defects won't appear when SPI_STACKED_PARALLEL config is enabled. We can mark these as false positives.

Thanks
Venkatesh
> -----Original Message-----
> From: Tom Rini <trini@konsulko.com>
> Sent: Tuesday, December 31, 2024 7:25 PM
> To: u-boot@lists.denx.de; Abbarapu, Venkatesh <venkatesh.abbarapu@amd.com>
> Subject: Fwd: New Defects reported by Coverity Scan for Das U-Boot
> 
> Hey all, here's the latest report.
> 
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Dec 30, 2024, 10:44 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.
> 
> 2 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> 1 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 2 of 2 defect(s)
> 
> 
> ** CID 528528:  Control flow issues  (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1644 in spi_nor_read()
> 
> 
> ___________________________________________________________________
> _____________________________________
> *** CID 528528:  Control flow issues  (DEADCODE)
> /drivers/mtd/spi/spi-nor-core.c: 1644 in spi_nor_read()
> 1638                            read_len = len;
> 1639                    else
> 1640                            read_len = rem_bank_len;
> 1641     #endif
> 1642
> 1643                    if (read_len == 0)
> >>>     CID 528528:  Control flow issues  (DEADCODE)
> >>>     Execution cannot reach this statement: "return -5;".
> 1644                            return -EIO;
> 1645
> 1646                    ret = nor->read(nor, offset, read_len, buf);
> 1647                    if (ret == 0) {
> 1648                            /* We shouldn't see 0-length reads */
> 1649                            ret = -EIO;
> 
> ** CID 528527:  Code maintainability issues  (UNUSED_VALUE)
> /drivers/mtd/spi/spi-nor-core.c: 1613 in spi_nor_read()
> 
> 
> ___________________________________________________________________
> _____________________________________
> *** CID 528527:  Code maintainability issues  (UNUSED_VALUE)
> /drivers/mtd/spi/spi-nor-core.c: 1613 in spi_nor_read()
> 1607                            }
> 1608                            rem_bank_len = SZ_16M * (bank + 1);
> 1609                            if
> (CONFIG_IS_ENABLED(SPI_STACKED_PARALLEL)) {
> 1610                                    if (nor->flags &
> SNOR_F_HAS_PARALLEL)
> 1611                                            rem_bank_len *= 2;
> 1612                            }
> >>>     CID 528527:  Code maintainability issues  (UNUSED_VALUE)
> >>>     Assigning value from "rem_bank_len - from" to "rem_bank_len"
> >>> here,
> but that stored value is overwritten before it can be used.
> 1613                            rem_bank_len -= from;
> 1614                    }
> 1615
> 1616                    if (CONFIG_IS_ENABLED(SPI_STACKED_PARALLEL)) {
> 1617                            if (nor->flags & SNOR_F_HAS_STACKED) {
> 1618                                    stack_shift = 1;
> 
> 
> ----- End forwarded message -----
> 
> --
> Tom

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New Defects reported by Coverity Scan for Das U-Boot
  2025-01-01 10:50 ` Abbarapu, Venkatesh
@ 2025-01-02 16:59   ` Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2025-01-02 16:59 UTC (permalink / raw)
  To: Abbarapu, Venkatesh; +Cc: u-boot@lists.denx.de

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

On Wed, Jan 01, 2025 at 10:50:02AM +0000, Abbarapu, Venkatesh wrote:

> Hi Tom,
> These defects won't appear when SPI_STACKED_PARALLEL config is enabled. We can mark these as false positives.

OK, thanks.

-- 
Tom

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New Defects reported by Coverity Scan for Das U-Boot
  2025-02-10 22:26 Fwd: " Tom Rini
@ 2025-02-11 22:24 ` Raymond Mao
  2025-02-11 22:30   ` Tom Rini
  0 siblings, 1 reply; 20+ messages in thread
From: Raymond Mao @ 2025-02-11 22:24 UTC (permalink / raw)
  To: Tom Rini; +Cc: u-boot, Heiko Schocher, Ilias Apalodimas

Hi Tom,

On Mon, 10 Feb 2025 at 17:26, Tom Rini <trini@konsulko.com> wrote:
>
> Here's the latest report.
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Feb 10, 2025 at 4:12 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.
>
> 3 new defect(s) introduced to Das U-Boot found with Coverity Scan.
>
>
> New defect(s) Reported-by: Coverity Scan
> Showing 3 of 3 defect(s)
>
>
> ** CID 541281:  Insecure data handling  (TAINTED_SCALAR)
> /lib/tpm-v2.c: 77 in tpm2_scan_masks()
>
>
> ________________________________________________________________________________________________________
> *** CID 541281:  Insecure data handling  (TAINTED_SCALAR)
> /lib/tpm-v2.c: 77 in tpm2_scan_masks()
> 71      *mask = 0;
> 72
> 73      rc = tpm2_get_pcr_info(dev, &pcrs);
> 74      if (rc)
> 75              return rc;
> 76
> >>>     CID 541281:  Insecure data handling  (TAINTED_SCALAR)
> >>>     Using tainted variable "pcrs.count" as a loop boundary.

We don't need to check the pcrs.count here, since tpm2_get_pcr_info()
will post an error if pcrs.count is not in a value from 1 to 4.
This is hardcoded in tpm2_get_pcr_info(), please see below:
```
/*
* We only support 4 algorithms for now so check against that
* instead of TPM2_NUM_PCR_BANKS
*/
if (pcrs->count > 4 || pcrs->count < 1) {
printf("%s: too many pcrs: %u\n", __func__, pcrs->count);
return -EMSGSIZE;
}
```

> 77      for (i = 0; i < pcrs.count; i++) {
> 78              struct tpms_pcr_selection *sel = &pcrs.selection[i];
> 79              size_t j;
> 80              u32 hash_mask = 0;
> 81
> 82              for (j = 0; j < ARRAY_SIZE(hash_algo_list); j++) {
>
> ** CID 541280:  Insecure data handling  (TAINTED_SCALAR)
> /cmd/tpm-v2.c: 307 in do_tpm2_pcrallocate()
>
>
> ________________________________________________________________________________________________________
> *** CID 541280:  Insecure data handling  (TAINTED_SCALAR)
> /cmd/tpm-v2.c: 307 in do_tpm2_pcrallocate()
> 301                      * first call
> 302                      */
> 303                     ret = tpm2_get_pcr_info(dev, &pcr);
> 304                     if (ret)
> 305                             return ret;
> 306
> >>>     CID 541280:  Insecure data handling  (TAINTED_SCALAR)
> >>>     Using tainted variable "pcr.count" as a loop boundary.

Ditto.

Regards,
Raymond

> 307                     for (i = 0; i < pcr.count; i++) {
> 308                             struct tpms_pcr_selection *sel =
> &pcr.selection[i];
> 309                             const char *name;
> 310
> 311                             if (!tpm2_is_active_bank(sel))
> 312                                     continue;
>
> ** CID 541279:    (TAINTED_SCALAR)
> /drivers/led/led-uclass.c: 284 in led_get_function_name()
> /drivers/led/led-uclass.c: 279 in led_get_function_name()
>
>
> ________________________________________________________________________________________________________
> *** CID 541279:    (TAINTED_SCALAR)
> /drivers/led/led-uclass.c: 284 in led_get_function_name()
> 278                     if (!ret) {
> 279                             snprintf(uc_plat->name, LED_MAX_NAME_SIZE,
> 280                                      "%s:%s-%d",
> 281                                      cp ? "" : led_colors[color],
> 282                                      func ? func : "", enumerator);
> 283                     } else {
> >>>     CID 541279:    (TAINTED_SCALAR)
> >>>     Using tainted variable "color" as an index into an array
> "led_colors".
> 284                             snprintf(uc_plat->name, LED_MAX_NAME_SIZE,
> 285                                      "%s:%s",
> 286                                      cp ? "" : led_colors[color],
> 287                                      func ? func : "");
> 288                     }
> 289                     uc_plat->label = uc_plat->name;
> /drivers/led/led-uclass.c: 279 in led_get_function_name()
> 273             /* Now try to detect function label name */
> 274             func = dev_read_string(dev, "function");
> 275             cp = dev_read_u32(dev, "color", &color);
> 276             if (cp == 0 || func) {
> 277                     ret = dev_read_u32(dev, "function-enumerator",
> &enumerator);
> 278                     if (!ret) {
> >>>     CID 541279:    (TAINTED_SCALAR)
> >>>     Using tainted variable "color" as an index into an array
> "led_colors".
> 279                             snprintf(uc_plat->name, LED_MAX_NAME_SIZE,
> 280                                      "%s:%s-%d",
> 281                                      cp ? "" : led_colors[color],
> 282                                      func ? func : "", enumerator);
> 283                     } else {
> 284                             snprintf(uc_plat->name, LED_MAX_NAME_SIZE,
>
>
> ----- End forwarded message -----
>
> --
> Tom

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New Defects reported by Coverity Scan for Das U-Boot
  2025-02-11 22:24 ` Raymond Mao
@ 2025-02-11 22:30   ` Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2025-02-11 22:30 UTC (permalink / raw)
  To: Raymond Mao; +Cc: u-boot, Heiko Schocher, Ilias Apalodimas

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

On Tue, Feb 11, 2025 at 05:24:02PM -0500, Raymond Mao wrote:
> Hi Tom,
> 
> On Mon, 10 Feb 2025 at 17:26, Tom Rini <trini@konsulko.com> wrote:
> >
> > Here's the latest report.
> >
> > ---------- Forwarded message ---------
> > From: <scan-admin@coverity.com>
> > Date: Mon, Feb 10, 2025 at 4:12 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.
> >
> > 3 new defect(s) introduced to Das U-Boot found with Coverity Scan.
> >
> >
> > New defect(s) Reported-by: Coverity Scan
> > Showing 3 of 3 defect(s)
> >
> >
> > ** CID 541281:  Insecure data handling  (TAINTED_SCALAR)
> > /lib/tpm-v2.c: 77 in tpm2_scan_masks()
> >
> >
> > ________________________________________________________________________________________________________
> > *** CID 541281:  Insecure data handling  (TAINTED_SCALAR)
> > /lib/tpm-v2.c: 77 in tpm2_scan_masks()
> > 71      *mask = 0;
> > 72
> > 73      rc = tpm2_get_pcr_info(dev, &pcrs);
> > 74      if (rc)
> > 75              return rc;
> > 76
> > >>>     CID 541281:  Insecure data handling  (TAINTED_SCALAR)
> > >>>     Using tainted variable "pcrs.count" as a loop boundary.
> 
> We don't need to check the pcrs.count here, since tpm2_get_pcr_info()
> will post an error if pcrs.count is not in a value from 1 to 4.
> This is hardcoded in tpm2_get_pcr_info(), please see below:
> ```
> /*
> * We only support 4 algorithms for now so check against that
> * instead of TPM2_NUM_PCR_BANKS
> */
> if (pcrs->count > 4 || pcrs->count < 1) {
> printf("%s: too many pcrs: %u\n", __func__, pcrs->count);
> return -EMSGSIZE;
> }
> ```
> 
> > 77      for (i = 0; i < pcrs.count; i++) {
> > 78              struct tpms_pcr_selection *sel = &pcrs.selection[i];
> > 79              size_t j;
> > 80              u32 hash_mask = 0;
> > 81
> > 82              for (j = 0; j < ARRAY_SIZE(hash_algo_list); j++) {
> >
> > ** CID 541280:  Insecure data handling  (TAINTED_SCALAR)
> > /cmd/tpm-v2.c: 307 in do_tpm2_pcrallocate()
> >
> >
> > ________________________________________________________________________________________________________
> > *** CID 541280:  Insecure data handling  (TAINTED_SCALAR)
> > /cmd/tpm-v2.c: 307 in do_tpm2_pcrallocate()
> > 301                      * first call
> > 302                      */
> > 303                     ret = tpm2_get_pcr_info(dev, &pcr);
> > 304                     if (ret)
> > 305                             return ret;
> > 306
> > >>>     CID 541280:  Insecure data handling  (TAINTED_SCALAR)
> > >>>     Using tainted variable "pcr.count" as a loop boundary.
> 
> Ditto.

OK, thanks!

-- 
Tom

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New Defects reported by Coverity Scan for Das U-Boot
  2025-07-08 14:10 Fwd: " Tom Rini
@ 2025-07-09  9:13 ` Sughosh Ganu
  0 siblings, 0 replies; 20+ messages in thread
From: Sughosh Ganu @ 2025-07-09  9:13 UTC (permalink / raw)
  To: Tom Rini
  Cc: u-boot, Simon Glass, Heinrich Schuchardt, Ilias Apalodimas,
	Marek Vasut, Ying-Chun Liu (PaulLiu), Aristo Chen,
	Rasmus Villemoes, Sean Edmond, Miquel Raynal

On Tue, 8 Jul 2025 at 19:40, Tom Rini <trini@konsulko.com> wrote:
>
> Hey all,
>
> Good news, Coverity Scan resumed putting information in the email
> report. Bad news, 20 new issues now that next has been merged.
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Jul 7, 2025 at 5:39 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:* 20
>    - 6 defect(s), reported by Coverity Scan earlier, were marked fixed in
>    the recent build analyzed by Coverity Scan.
>    - *Defects Shown:* Showing 20 of 20 defect(s)
>
> Defect Details
>

[...]

> _____________________________________________________________________________________________
> *** CID 569481:         Control flow issues  (MISSING_BREAK)
> /lib/lmb.c: 763             in lmb_alloc_mem()
> 757                     return 0;
> 758
> 759             if (!addr)
> 760                     return -EINVAL;
> 761
> 762             switch (type) {
> >>>     CID 569481:         Control flow issues  (MISSING_BREAK)
> >>>     The case for value "LMB_MEM_ALLOC_ANY" is not terminated by a "break" statement.

The missing break is on purpose, so this is not an issue. Nonetheless,
Heinrich has sent a patch [1] to put a fallthrough statement here.
Thanks.

-sughosh

[1] - https://patchwork.ozlabs.org/project/uboot/patch/20250708121251.83980-1-heinrich.schuchardt@canonical.com/


> 763             case LMB_MEM_ALLOC_ANY:
> 764                     *addr = LMB_ALLOC_ANYWHERE;
> 765             case LMB_MEM_ALLOC_MAX:
> 766                     ret = _lmb_alloc_base(size, align, addr, flags);
> 767                     break;
> 768             case LMB_MEM_ALLOC_ADDR:
>
>
>
> View Defects in Coverity Scan
> <https://scan.coverity.com/projects/das-u-boot?tab=overview>
>
> Best regards,
>
> The Coverity Scan Admin Team
>
> ----- End forwarded message -----
>
> --
> Tom

^ permalink raw reply	[flat|nested] 20+ messages in thread

* RE: New Defects reported by Coverity Scan for Das U-Boot
  2025-08-06 18:35 Fwd: " Tom Rini
@ 2025-08-07  1:50 ` Maniyam, Dinesh
  0 siblings, 0 replies; 20+ messages in thread
From: Maniyam, Dinesh @ 2025-08-07  1:50 UTC (permalink / raw)
  To: Tom Rini, u-boot@lists.denx.de, Heiko Schocher

Hi Tom

> -----Original Message-----
> From: Tom Rini <trini@konsulko.com>
> Sent: Thursday, 7 August 2025 2:36 am
> To: u-boot@lists.denx.de; Heiko Schocher <hs@denx.de>; Maniyam, Dinesh
> <dinesh.maniyam@altera.com>
> Subject: Fwd: New Defects reported by Coverity Scan for Das U-Boot
> 
> Here's the latest report. Lets get these new issues addressed ASAP please,
> thanks.

I will work on resolving the issues.

Thanks!

> 
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Wed, Aug 6, 2025 at 12:23 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:* 8
>    - 4 defect(s), reported by Coverity Scan earlier, were marked fixed in
>    the recent build analyzed by Coverity Scan.
>    - *Defects Shown:* Showing 8 of 8 defect(s)
> 
> Defect Details
> 
> ** CID 583812:       Integer handling issues  (BAD_SHIFT)
> /drivers/i3c/master/dw-i3c-master.c: 1001           in dw_i3c_probe()
> 
> 
> _________________________________________________________________
> ____________________________
> *** CID 583812:         Integer handling issues  (BAD_SHIFT)
> /drivers/i3c/master/dw-i3c-master.c: 1001             in dw_i3c_probe()
> 995     	ret = readl(master->regs + DATA_BUFFER_STATUS_LEVEL);
> 996     	master->caps.datafifodepth = DATA_BUFFER_STATUS_LEVEL_TX(ret);
> 997
> 998     	ret = readl(master->regs + DEVICE_ADDR_TABLE_POINTER);
> 999     	master->datstartaddr = ret;
> 1000     	master->maxdevs = ret >> 16;
> >>>     CID 583812:         Integer handling issues  (BAD_SHIFT)
> >>>     In expression "0xffffffffffffffffUL >> 63 - (master->maxdevs - 1)", right
> shifting by more than 63 bits has undefined behavior.  The shift amount, "63 -
> (master->maxdevs - 1)", is 64.
> 1001     	master->free_pos = GENMASK(master->maxdevs - 1, 0);
> 1002
> 1003     	ret = i3c_master_register(&master->base, dev,
> 1004     				  &dw_mipi_i3c_ops, false);
> 1005     	if (ret)
> 1006     		goto err_assert_rst;
> 
> ** CID 583811:         (RESOURCE_LEAK)
> /drivers/i3c/master.c: 1610           in of_i3c_master_add_i3c_boardinfo()
> /drivers/i3c/master.c: 1586           in of_i3c_master_add_i3c_boardinfo()
> /drivers/i3c/master.c: 1591           in of_i3c_master_add_i3c_boardinfo()
> /drivers/i3c/master.c: 1598           in of_i3c_master_add_i3c_boardinfo()
> /drivers/i3c/master.c: 1603           in of_i3c_master_add_i3c_boardinfo()
> 
> 
> _________________________________________________________________
> ____________________________
> *** CID 583811:           (RESOURCE_LEAK)
> /drivers/i3c/master.c: 1610             in of_i3c_master_add_i3c_boardinfo()
> 1604     	}
> 1605
> 1606     	boardinfo->pid = ((u64)reg[1] << 32) | reg[2];
> 1607
> 1608     	if ((boardinfo->pid & GENMASK_ULL(63, 48)) ||
> 1609     	    I3C_PID_RND_LOWER_32BITS(boardinfo->pid))
> >>>     CID 583811:           (RESOURCE_LEAK)
> >>>     Variable "boardinfo" going out of scope leaks the storage it points to.
> 1610     		return -EINVAL;
> 1611
> 1612     	boardinfo->init_dyn_addr = init_dyn_addr;
> 1613     	boardinfo->of_node = node;
> 1614     	list_add_tail(&boardinfo->node, &master->boardinfo.i3c);
> 1615
> /drivers/i3c/master.c: 1586             in of_i3c_master_add_i3c_boardinfo()
> 1580     	boardinfo = devm_kzalloc(dev, sizeof(*boardinfo), GFP_KERNEL);
> 1581     	if (!boardinfo)
> 1582     		return -ENOMEM;
> 1583
> 1584     	if (reg[0]) {
> 1585     		if (reg[0] > I3C_MAX_ADDR)
> >>>     CID 583811:           (RESOURCE_LEAK)
> >>>     Variable "boardinfo" going out of scope leaks the storage it points to.
> 1586     			return -EINVAL;
> 1587
> 1588     		addrstatus = i3c_bus_get_addr_slot_status(&master->bus,
> 1589     							  reg[0]);
> 1590     		if (addrstatus != I3C_ADDR_SLOT_FREE)
> 1591     			return -EINVAL;
> /drivers/i3c/master.c: 1591             in of_i3c_master_add_i3c_boardinfo()
> 1585     		if (reg[0] > I3C_MAX_ADDR)
> 1586     			return -EINVAL;
> 1587
> 1588     		addrstatus = i3c_bus_get_addr_slot_status(&master->bus,
> 1589     							  reg[0]);
> 1590     		if (addrstatus != I3C_ADDR_SLOT_FREE)
> >>>     CID 583811:           (RESOURCE_LEAK)
> >>>     Variable "boardinfo" going out of scope leaks the storage it points to.
> 1591     			return -EINVAL;
> 1592     	}
> 1593
> 1594     	boardinfo->static_addr = reg[0];
> 1595
> 1596     	if (!dev_read_u32(dev, "assigned-address", &init_dyn_addr)) {
> /drivers/i3c/master.c: 1598             in of_i3c_master_add_i3c_boardinfo()
> 1592     	}
> 1593
> 1594     	boardinfo->static_addr = reg[0];
> 1595
> 1596     	if (!dev_read_u32(dev, "assigned-address", &init_dyn_addr)) {
> 1597     		if (init_dyn_addr > I3C_MAX_ADDR)
> >>>     CID 583811:           (RESOURCE_LEAK)
> >>>     Variable "boardinfo" going out of scope leaks the storage it points to.
> 1598     			return -EINVAL;
> 1599
> 1600     		addrstatus = i3c_bus_get_addr_slot_status(&master->bus,
> 1601     							  init_dyn_addr);
> 1602     		if (addrstatus != I3C_ADDR_SLOT_FREE)
> 1603     			return -EINVAL;
> /drivers/i3c/master.c: 1603             in of_i3c_master_add_i3c_boardinfo()
> 1597     		if (init_dyn_addr > I3C_MAX_ADDR)
> 1598     			return -EINVAL;
> 1599
> 1600     		addrstatus = i3c_bus_get_addr_slot_status(&master->bus,
> 1601     							  init_dyn_addr);
> 1602     		if (addrstatus != I3C_ADDR_SLOT_FREE)
> >>>     CID 583811:           (RESOURCE_LEAK)
> >>>     Variable "boardinfo" going out of scope leaks the storage it points to.
> 1603     			return -EINVAL;
> 1604     	}
> 1605
> 1606     	boardinfo->pid = ((u64)reg[1] << 32) | reg[2];
> 1607
> 1608     	if ((boardinfo->pid & GENMASK_ULL(63, 48)) ||
> 
> ** CID 298388:       Integer handling issues  (SIGN_EXTENSION)
> /drivers/i3c/master/dw-i3c-master.c: 579           in dw_i3c_ccc_get()
> 
> 
> _________________________________________________________________
> ____________________________
> *** CID 298388:         Integer handling issues  (SIGN_EXTENSION)
> /drivers/i3c/master/dw-i3c-master.c: 579             in dw_i3c_ccc_get()
> 573     		return -ENOMEM;
> 574
> 575     	cmd = xfer->cmds;
> 576     	cmd->rx_buf = ccc->dests[0].payload.data;
> 577     	cmd->rx_len = ccc->dests[0].payload.len;
> 578
> >>>     CID 298388:         Integer handling issues  (SIGN_EXTENSION)
> >>>     Suspicious implicit sign extension: "ccc->dests[0].payload.len" with type
> "u16" (16 bits, unsigned) is promoted in "ccc->dests[0].payload.len << 16" to type
> "int" (32 bits, signed), then sign-extended to type "unsigned long" (64 bits,
> unsigned).  If "ccc->dests[0].payload.len << 16" is greater than 0x7FFFFFFF, the
> upper bits of the result will all be 1.
> 579     	cmd->cmd_hi = COMMAND_PORT_ARG_DATA_LEN(ccc-
> >dests[0].payload.len) |
> 580     		      COMMAND_PORT_TRANSFER_ARG;
> 581
> 582     	cmd->cmd_lo = COMMAND_PORT_READ_TRANSFER |
> 583     		      COMMAND_PORT_CP |
> 584     		      COMMAND_PORT_DEV_INDEX(pos) |
> 
> ** CID 298037:       Integer handling issues  (SIGN_EXTENSION)
> /drivers/i3c/master/dw-i3c-master.c: 375           in dw_i3c_clk_cfg()
> 
> 
> _________________________________________________________________
> ____________________________
> *** CID 298037:         Integer handling issues  (SIGN_EXTENSION)
> /drivers/i3c/master/dw-i3c-master.c: 375             in dw_i3c_clk_cfg()
> 369     	scl_timing = SCL_EXT_LCNT_1(lcnt);
> 370     	lcnt = DIV_ROUND_UP(core_rate, I3C_BUS_SDR2_SCL_RATE) - hcnt;
> 371     	scl_timing |= SCL_EXT_LCNT_2(lcnt);
> 372     	lcnt = DIV_ROUND_UP(core_rate, I3C_BUS_SDR3_SCL_RATE) - hcnt;
> 373     	scl_timing |= SCL_EXT_LCNT_3(lcnt);
> 374     	lcnt = DIV_ROUND_UP(core_rate, I3C_BUS_SDR4_SCL_RATE) - hcnt;
> >>>     CID 298037:         Integer handling issues  (SIGN_EXTENSION)
> >>>     Suspicious implicit sign extension: "lcnt" with type "u8" (8 bits, unsigned) is
> promoted in "lcnt << 24" to type "int" (32 bits, signed), then sign-extended to type
> "unsigned long" (64 bits, unsigned).  If "lcnt << 24" is greater than 0x7FFFFFFF, the
> upper bits of the result will all be 1.
> 375     	scl_timing |= SCL_EXT_LCNT_4(lcnt);
> 376     	writel(scl_timing, master->regs + SCL_EXT_LCNT_TIMING);
> 377
> 378     	return 0;
> 379     }
> 380
> 
> ** CID 296053:       Integer handling issues  (SIGN_EXTENSION)
> /drivers/i3c/master/dw-i3c-master.c: 535           in dw_i3c_ccc_set()
> 
> 
> _________________________________________________________________
> ____________________________
> *** CID 296053:         Integer handling issues  (SIGN_EXTENSION)
> /drivers/i3c/master/dw-i3c-master.c: 535             in dw_i3c_ccc_set()
> 529     		return -ENOMEM;
> 530
> 531     	cmd = xfer->cmds;
> 532     	cmd->tx_buf = ccc->dests[0].payload.data;
> 533     	cmd->tx_len = ccc->dests[0].payload.len;
> 534
> >>>     CID 296053:         Integer handling issues  (SIGN_EXTENSION)
> >>>     Suspicious implicit sign extension: "ccc->dests[0].payload.len" with type
> "u16" (16 bits, unsigned) is promoted in "ccc->dests[0].payload.len << 16" to type
> "int" (32 bits, signed), then sign-extended to type "unsigned long" (64 bits,
> unsigned).  If "ccc->dests[0].payload.len << 16" is greater than 0x7FFFFFFF, the
> upper bits of the result will all be 1.
> 535     	cmd->cmd_hi = COMMAND_PORT_ARG_DATA_LEN(ccc-
> >dests[0].payload.len) |
> 536     		      COMMAND_PORT_TRANSFER_ARG;
> 537
> 538     	cmd->cmd_lo = COMMAND_PORT_CP |
> 539     		      COMMAND_PORT_DEV_INDEX(pos) |
> 540     		      COMMAND_PORT_CMD(ccc->id) |
> 
> ** CID 295976:         (SIGN_EXTENSION)
> /drivers/i3c/master/dw-i3c-master.c: 395           in dw_i2c_clk_cfg()
> /drivers/i3c/master/dw-i3c-master.c: 401           in dw_i2c_clk_cfg()
> 
> 
> _________________________________________________________________
> ____________________________
> *** CID 295976:           (SIGN_EXTENSION)
> /drivers/i3c/master/dw-i3c-master.c: 395             in dw_i2c_clk_cfg()
> 389     		return -EINVAL;
> 390
> 391     	core_period = DIV_ROUND_UP(1000000000, core_rate);
> 392
> 393     	lcnt = DIV_ROUND_UP(I3C_BUS_I2C_FMP_TLOW_MIN_NS,
> core_period);
> 394     	hcnt = DIV_ROUND_UP(core_rate, I3C_BUS_I2C_FM_PLUS_SCL_RATE) -
> lcnt;
> >>>     CID 295976:           (SIGN_EXTENSION)
> >>>     Suspicious implicit sign extension: "hcnt" with type "u16" (16 bits,
> unsigned) is promoted in "hcnt << 16" to type "int" (32 bits, signed), then sign-
> extended to type "unsigned long" (64 bits, unsigned).  If "hcnt << 16" is greater
> than 0x7FFFFFFF, the upper bits of the result will all be 1.
> 395     	scl_timing = SCL_I2C_FMP_TIMING_HCNT(hcnt) |
> 396     		     SCL_I2C_FMP_TIMING_LCNT(lcnt);
> 397     	writel(scl_timing, master->regs + SCL_I2C_FMP_TIMING);
> 398
> 399     	lcnt = DIV_ROUND_UP(I3C_BUS_I2C_FM_TLOW_MIN_NS, core_period);
> 400     	hcnt = DIV_ROUND_UP(core_rate, I3C_BUS_I2C_FM_SCL_RATE) - lcnt;
> /drivers/i3c/master/dw-i3c-master.c: 401             in dw_i2c_clk_cfg()
> 395     	scl_timing = SCL_I2C_FMP_TIMING_HCNT(hcnt) |
> 396     		     SCL_I2C_FMP_TIMING_LCNT(lcnt);
> 397     	writel(scl_timing, master->regs + SCL_I2C_FMP_TIMING);
> 398
> 399     	lcnt = DIV_ROUND_UP(I3C_BUS_I2C_FM_TLOW_MIN_NS, core_period);
> 400     	hcnt = DIV_ROUND_UP(core_rate, I3C_BUS_I2C_FM_SCL_RATE) - lcnt;
> >>>     CID 295976:           (SIGN_EXTENSION)
> >>>     Suspicious implicit sign extension: "hcnt" with type "u16" (16 bits,
> unsigned) is promoted in "hcnt << 16" to type "int" (32 bits, signed), then sign-
> extended to type "unsigned long" (64 bits, unsigned).  If "hcnt << 16" is greater
> than 0x7FFFFFFF, the upper bits of the result will all be 1.
> 401     	scl_timing = SCL_I2C_FM_TIMING_HCNT(hcnt) |
> 402     		     SCL_I2C_FM_TIMING_LCNT(lcnt);
> 403     	writel(scl_timing, master->regs + SCL_I2C_FM_TIMING);
> 404
> 405     	writel(BUS_I3C_MST_FREE(lcnt), master->regs + BUS_FREE_TIMING);
> 406     	writel(readl(master->regs + DEVICE_CTRL) |
> DEV_CTRL_I2C_SLAVE_PRESENT,
> 
> ** CID 294913:       Integer handling issues  (SIGN_EXTENSION)
> /drivers/i3c/master/dw-i3c-master.c: 724           in dw_i3c_master_priv_xfers()
> 
> 
> _________________________________________________________________
> ____________________________
> *** CID 294913:         Integer handling issues  (SIGN_EXTENSION)
> /drivers/i3c/master/dw-i3c-master.c: 724             in
> dw_i3c_master_priv_xfers()
> 718     	if (!xfer)
> 719     		return -ENOMEM;
> 720
> 721     	for (i = 0; i < i3c_nxfers; i++) {
> 722     		struct dw_i3c_cmd *cmd = &xfer->cmds[i];
> 723
> >>>     CID 294913:         Integer handling issues  (SIGN_EXTENSION)
> >>>     Suspicious implicit sign extension: "i3c_xfers[i].len" with type "u16" (16
> bits, unsigned) is promoted in "i3c_xfers[i].len << 16" to type "int" (32 bits,
> signed), then sign-extended to type "unsigned long" (64 bits, unsigned).  If
> "i3c_xfers[i].len << 16" is greater than 0x7FFFFFFF, the upper bits of the result will
> all be 1.
> 724     		cmd->cmd_hi =
> COMMAND_PORT_ARG_DATA_LEN(i3c_xfers[i].len) |
> 725     			COMMAND_PORT_TRANSFER_ARG;
> 726
> 727     		if (i3c_xfers[i].rnw) {
> 728     			cmd->rx_buf = i3c_xfers[i].data.in;
> 729     			cmd->rx_len = i3c_xfers[i].len;
> 
> ** CID 294627:       Integer handling issues  (BAD_SHIFT)
> /drivers/i3c/master.c: 181           in i3c_bus_get_addr_slot_status()
> 
> 
> _________________________________________________________________
> ____________________________
> *** CID 294627:         Integer handling issues  (BAD_SHIFT)
> /drivers/i3c/master.c: 181             in i3c_bus_get_addr_slot_status()
> 175     	int status, bitpos = addr * 2;
> 176
> 177     	if (addr > I2C_MAX_ADDR)
> 178     		return I3C_ADDR_SLOT_RSVD;
> 179
> 180     	status = bus->addrslots[bitpos / BITS_PER_LONG];
> >>>     CID 294627:         Integer handling issues  (BAD_SHIFT)
> >>>     In expression "status >>= bitpos % 64", right shifting by more than 31 bits
> has undefined behavior.  The shift amount, "bitpos % 64", is as much as 63.
> 181     	status >>= bitpos % BITS_PER_LONG;
> 182
> 183     	return status & I3C_ADDR_SLOT_STATUS_MASK;
> 184     }
> 185
> 186     static void i3c_bus_set_addr_slot_status(struct i3c_bus *bus, u16 addr,
> 
> 
> 
> View Defects in Coverity Scan
> <https://scan.coverity.com/projects/das-u-boot?tab=overview>
> 
> Best regards,
> 
> The Coverity Scan Admin Team
> 
> ----- End forwarded message -----
> 
> --
> Tom

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New Defects reported by Coverity Scan for Das U-Boot
  2025-11-10 18:55 Fwd: " Tom Rini
@ 2025-11-12  8:53 ` Kory Maincent
  0 siblings, 0 replies; 20+ messages in thread
From: Kory Maincent @ 2025-11-12  8:53 UTC (permalink / raw)
  To: Tom Rini; +Cc: u-boot

On Mon, 10 Nov 2025 12:55:07 -0600
Tom Rini <trini@konsulko.com> wrote:

> Here's the latest report. Just 2 new issues, both from the extensions
> series. Can we please address these shortly? Thanks!

Ok I will take a look at these errors!

> 
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Nov 10, 2025 at 12:44 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:* 2
>    - 1 defect(s), reported by Coverity Scan earlier, were marked fixed in
>    the recent build analyzed by Coverity Scan.
>    - *Defects Shown:* Showing 2 of 2 defect(s)
> 
> Defect Details
> 
> ** CID 638558:       Memory - illegal accesses  (UNINIT)
> /boot/pxe_utils.c: 485           in label_boot_extension()
> 
> 
> _____________________________________________________________________________________________
> *** CID 638558:         Memory - illegal accesses  (UNINIT)
> /boot/pxe_utils.c: 485             in label_boot_extension()
> 479     			return;
> 480
> 481     		snprintf(overlay_dir, dir_len, "%s%s", label->fdtdir,
> 482     			 slash);
> 483     	} else {
> 484     		dir_len = 2;
> >>>     CID 638558:         Memory - illegal accesses  (UNINIT)
> >>>     Using uninitialized value "overlay_dir" when calling "snprintf".
> >>> [Note: The source code implementation of the function has been overridden
> >>> by a builtin model.]  
> 485     		snprintf(overlay_dir, dir_len, "/");
> 486     	}
> 487
> 488     	alist_for_each(extension, extension_list) {
> 489     		char *overlay_file;
> 490     		ulong size;
> 
> ** CID 638557:       Null pointer dereferences  (NULL_RETURNS)
> 
> 
> _____________________________________________________________________________________________
> *** CID 638557:         Null pointer dereferences  (NULL_RETURNS)
> /cmd/extension_board.c: 102             in do_extension_list()
> 96     {
> 97     	struct alist *extension_list;
> 98     	struct extension *extension;
> 99     	int i = 0;
> 100
> 101     	extension_list = extension_get_list();
> >>>     CID 638557:         Null pointer dereferences  (NULL_RETURNS)
> >>>     Dereferencing a pointer that might be "NULL" "extension_list" when
> >>> calling "alist_get_ptr".  
> 102     	if (!alist_get_ptr(extension_list, 0)) {
> 103     		printf("No extension registered - Please run
> \"extension scan\"\n"); 104     		return CMD_RET_SUCCESS;
> 105     	}
> 106
> 107     	alist_for_each(extension, extension_list) {
> 
> 
> 
> View Defects in Coverity Scan
> <https://scan.coverity.com/projects/das-u-boot?tab=overview>
> 
> Best regards,
> 
> The Coverity Scan Admin Team
> 
> ----- End forwarded message -----
> 



-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New Defects reported by Coverity Scan for Das U-Boot
  2025-12-08 19:38 Fwd: " Tom Rini
@ 2025-12-09 11:06 ` Adriana Nicolae
  2025-12-09 14:24   ` Tom Rini
  0 siblings, 1 reply; 20+ messages in thread
From: Adriana Nicolae @ 2025-12-09 11:06 UTC (permalink / raw)
  To: Tom Rini; +Cc: u-boot

Hello,

A possible fix for fdtdec.c tests would be to validate the fdt size
before using it.
All 3 tests in this file are using the same approach, if the previous tests were
acceptable the newly added one has some identical parts.

If there is a way to test and fix all these 3 errors, I've put some
changes which
might be enough for Coverity to assume that the size of the fdt is safe. Let me
know how I can test it or should I send an email to post it as a
separate change?

diff --git a/test/dm/fdtdec.c b/test/dm/fdtdec.c
index ea5a494612c..a3c90d38115 100644
--- a/test/dm/fdtdec.c
+++ b/test/dm/fdtdec.c
@@ -14,14 +14,21 @@

 DECLARE_GLOBAL_DATA_PTR;

+#define FDTDEC_MAX_SIZE  (2 * 1024 * 1024)
+
 static int dm_test_fdtdec_set_carveout(struct unit_test_state *uts)
 {
  struct fdt_memory resv;
  void *blob;
  const fdt32_t *prop;
- int blob_sz, len, offset;
+ int blob_sz, len, offset, fdt_sz;
+
+ fdt_sz = fdt_totalsize(gd->fdt_blob);
+ if (fdt_sz <= 0 || fdt_sz > FDTDEC_MAX_SIZE) {
+ return -EINVAL;
+ }

- blob_sz = fdt_totalsize(gd->fdt_blob) + 4096;
+ blob_sz = fdt_sz + 4096;
  blob = malloc(blob_sz);
  ut_assertnonnull(blob);

@@ -67,10 +74,15 @@ static int
dm_test_fdtdec_add_reserved_memory(struct unit_test_state *uts)
  fdt_size_t size;
  void *blob;
  unsigned long flags = FDTDEC_RESERVED_MEMORY_NO_MAP;
- int blob_sz, parent, subnode;
+ int blob_sz, parent, subnode, fdt_sz;
  uint32_t phandle, phandle1;

- blob_sz = fdt_totalsize(gd->fdt_blob) + 128;
+ fdt_sz = fdt_totalsize(gd->fdt_blob);
+ if (fdt_sz <= 0 || fdt_sz > FDTDEC_MAX_SIZE) {
+ return -EINVAL;
+ }
+
+ blob_sz = fdt_sz + 128;
  blob = malloc(blob_sz);
  ut_assertnonnull(blob);

@@ -138,14 +150,19 @@ static int dm_test_fdt_chosen_smbios(struct
unit_test_state *uts)
  void *blob;
  ulong val;
  struct smbios3_entry *entry;
- int chosen, blob_sz;
+ int chosen, blob_sz, fdt_sz;
  const fdt64_t *prop;

  if (!CONFIG_IS_ENABLED(GENERATE_SMBIOS_TABLE)) {
  return -EAGAIN;
  }

- blob_sz = fdt_totalsize(gd->fdt_blob) + 4096;
+ fdt_sz = fdt_totalsize(gd->fdt_blob);
+ if (fdt_sz <= 0 || fdt_sz > FDTDEC_MAX_SIZE) {
+ return -EINVAL;
+ }
+
+ blob_sz = fdt_sz + 4096;
  blob = memalign(8, blob_sz);
  ut_assertnonnull(blob);



On Mon, Dec 8, 2025 at 9:38 PM Tom Rini <trini@konsulko.com> wrote:
>
> Here's the latest Coverity scan report. I think the test/dm/clk_ccf.c
> report is just a "works as intended" but I'm not sure off-hand about the
> fdtdec.c test. Might be the case the previous test in the file also has
> this problem, and since it's just test code, might also be fine enough.
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Dec 8, 2025 at 1:23 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:* 2
>    - 1 defect(s), reported by Coverity Scan earlier, were marked fixed in
>    the recent build analyzed by Coverity Scan.
>    - *Defects Shown:* Showing 2 of 2 defect(s)
>
> Defect Details
>
> ** CID 639831:         (TAINTED_SCALAR)
>
>
> _____________________________________________________________________________________________
> *** CID 639831:           (TAINTED_SCALAR)
> /test/dm/fdtdec.c: 153             in dm_test_fdt_chosen_smbios()
> 147
> 148             blob_sz = fdt_totalsize(gd->fdt_blob) + 4096;
> 149             blob = memalign(8, blob_sz);
> 150             ut_assertnonnull(blob);
> 151
> 152             /* Make a writable copy of the fdt blob */
> >>>     CID 639831:           (TAINTED_SCALAR)
> >>>     Passing tainted expression "gd->fdt_blob->totalsize" to "fdt_open_into", which uses it as an offset.
> 153             ut_assertok(fdt_open_into(gd->fdt_blob, blob, blob_sz));
> 154
> 155             /* Mock SMBIOS table */
> 156             entry = map_sysmem(gd->arch.smbios_start, sizeof(struct
> smbios3_entry));
> 157             memcpy(entry->anchor, "_SM3_", 5);
> 158             entry->length = sizeof(struct smbios3_entry);
> /test/dm/fdtdec.c: 153             in dm_test_fdt_chosen_smbios()
> 147
> 148             blob_sz = fdt_totalsize(gd->fdt_blob) + 4096;
> 149             blob = memalign(8, blob_sz);
> 150             ut_assertnonnull(blob);
> 151
> 152             /* Make a writable copy of the fdt blob */
> >>>     CID 639831:           (TAINTED_SCALAR)
> >>>     Passing tainted expression "gd->fdt_blob->size_dt_strings" to "fdt_open_into", which uses it as an offset.
> 153             ut_assertok(fdt_open_into(gd->fdt_blob, blob, blob_sz));
> 154
> 155             /* Mock SMBIOS table */
> 156             entry = map_sysmem(gd->arch.smbios_start, sizeof(struct
> smbios3_entry));
> 157             memcpy(entry->anchor, "_SM3_", 5);
> 158             entry->length = sizeof(struct smbios3_entry);
> /test/dm/fdtdec.c: 153             in dm_test_fdt_chosen_smbios()
> 147
> 148             blob_sz = fdt_totalsize(gd->fdt_blob) + 4096;
> 149             blob = memalign(8, blob_sz);
> 150             ut_assertnonnull(blob);
> 151
> 152             /* Make a writable copy of the fdt blob */
> >>>     CID 639831:           (TAINTED_SCALAR)
> >>>     Passing tainted expression "gd->fdt_blob->size_dt_struct" to "fdt_open_into", which uses it as an offset.
> 153             ut_assertok(fdt_open_into(gd->fdt_blob, blob, blob_sz));
> 154
> 155             /* Mock SMBIOS table */
> 156             entry = map_sysmem(gd->arch.smbios_start, sizeof(struct
> smbios3_entry));
> 157             memcpy(entry->anchor, "_SM3_", 5);
> 158             entry->length = sizeof(struct smbios3_entry);
>
> ** CID 639830:       Integer handling issues  (INTEGER_OVERFLOW)
> /test/dm/clk_ccf.c: 68           in dm_test_clk_ccf()
>
>
> _____________________________________________________________________________________________
> *** CID 639830:         Integer handling issues  (INTEGER_OVERFLOW)
> /test/dm/clk_ccf.c: 68             in dm_test_clk_ccf()
> 62      ut_asserteq(CLK_SET_RATE_NO_REPARENT, clk->flags);
> 63
> 64      rate = clk_get_parent_rate(clk);
> 65      ut_asserteq(rate, 60000000);
> 66
> 67      rate = clk_set_rate(clk, 60000000);
> >>>     CID 639830:         Integer handling issues  (INTEGER_OVERFLOW)
> >>>     Expression "_val1", where "rate" is known to be equal to -38, overflows the type of "_val1", which is type "unsigned int".
> 68      ut_asserteq(rate, -ENOSYS);
> 69
> 70      rate = clk_get_rate(clk);
> 71      ut_asserteq(rate, 60000000);
> 72
> 73      ret = clk_get_by_id(CLK_ID(dev, SANDBOX_CLK_PLL3_80M), &pclk);
>
>
>
> View Defects in Coverity Scan
> <https://scan.coverity.com/projects/das-u-boot?tab=overview>
>
> Best regards,
>
> The Coverity Scan Admin Team
>
> ----- End forwarded message -----
>
> --
> Tom

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: New Defects reported by Coverity Scan for Das U-Boot
  2025-12-09 11:06 ` Adriana Nicolae
@ 2025-12-09 14:24   ` Tom Rini
  0 siblings, 0 replies; 20+ messages in thread
From: Tom Rini @ 2025-12-09 14:24 UTC (permalink / raw)
  To: Adriana Nicolae; +Cc: u-boot

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

On Tue, Dec 09, 2025 at 01:06:28PM +0200, Adriana Nicolae wrote:

> Hello,
> 
> A possible fix for fdtdec.c tests would be to validate the fdt size
> before using it.
> All 3 tests in this file are using the same approach, if the previous tests were
> acceptable the newly added one has some identical parts.
> 
> If there is a way to test and fix all these 3 errors, I've put some
> changes which
> might be enough for Coverity to assume that the size of the fdt is safe. Let me
> know how I can test it or should I send an email to post it as a
> separate change?

Thanks. A frustrating thing to me with Coverity is I've never seen a way
to test if a change fixes a bug (if you *pay* for Coverity then you can
do many more runs, and also run it on CI test branches and not pollute
your main results I believe, is why). So we need to do it as a regular
patch.

> diff --git a/test/dm/fdtdec.c b/test/dm/fdtdec.c
> index ea5a494612c..a3c90d38115 100644
> --- a/test/dm/fdtdec.c
> +++ b/test/dm/fdtdec.c
> @@ -14,14 +14,21 @@
> 
>  DECLARE_GLOBAL_DATA_PTR;
> 
> +#define FDTDEC_MAX_SIZE  (2 * 1024 * 1024)
> +
>  static int dm_test_fdtdec_set_carveout(struct unit_test_state *uts)
>  {
>   struct fdt_memory resv;
>   void *blob;
>   const fdt32_t *prop;
> - int blob_sz, len, offset;
> + int blob_sz, len, offset, fdt_sz;
> +
> + fdt_sz = fdt_totalsize(gd->fdt_blob);
> + if (fdt_sz <= 0 || fdt_sz > FDTDEC_MAX_SIZE) {
> + return -EINVAL;
> + }

Since these are tests, can we ut_assertsomething here instead? Thanks!

-- 
Tom

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: New Defects reported by Coverity Scan for Das U-Boot
  2026-04-06 19:12 Fwd: " Tom Rini
@ 2026-04-07 20:44 ` Raymond Mao
  0 siblings, 0 replies; 20+ messages in thread
From: Raymond Mao @ 2026-04-07 20:44 UTC (permalink / raw)
  To: Tom Rini
  Cc: u-boot, Kory Maincent, Dan Carpenter, Varadarajan Narayanan,
	Bo-Chen Chen, David Lechner, Ilias Apalodimas

Hi Tom,

On Mon, Apr 6, 2026 at 3:12 PM Tom Rini <trini@konsulko.com> wrote:
>
> Here's the latest report, now that I've merged next to master, locally
> at least.
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Mon, Apr 6, 2026 at 12:40 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:* 11
>    - 15 defect(s), reported by Coverity Scan earlier, were marked fixed in
>    the recent build analyzed by Coverity Scan.
>    - *Defects Shown:* Showing 11 of 11 defect(s)
>
> Defect Details
>
> ** CID 645496:         (USE_AFTER_FREE)
> /tools/fwumdata_src/fwumdata.c: 94           in parse_config()
> /tools/fwumdata_src/fwumdata.c: 101           in parse_config()
>
>
> _____________________________________________________________________________________________
> *** CID 645496:           (USE_AFTER_FREE)
> /tools/fwumdata_src/fwumdata.c: 94             in parse_config()
> 88                          &devname,
> 89                          &devices[i].devoff,
> 90                          &devices[i].mdata_size,
> 91                          &devices[i].erase_size);
> 92
> 93              if (rc < 3) {
> >>>     CID 645496:           (USE_AFTER_FREE)
> >>>     Calling "free" frees pointer "devname" which has already been freed.
> 94                      free(devname);
> 95                      continue;
> 96              }
> 97
> 98              if (rc < 4)
> 99                      devices[i].erase_size = devices[i].mdata_size;
> /tools/fwumdata_src/fwumdata.c: 101             in parse_config()
> 95                      continue;
> 96              }
> 97
> 98              if (rc < 4)
> 99                      devices[i].erase_size = devices[i].mdata_size;
> 100
> >>>     CID 645496:           (USE_AFTER_FREE)
> >>>     Using freed pointer "devname".
> 101                     devices[i].devname = devname;
> 102                     i++;
> 103             }
> 104
> 105             free(line);
> 106             fclose(fp);
>
> ** CID 645495:       Uninitialized variables  (UNINIT)
> /fs/fat/fat.c: 175           in disk_rw()
>
>
> _____________________________________________________________________________________________
> *** CID 645495:         Uninitialized variables  (UNINIT)
> /fs/fat/fat.c: 175             in disk_rw()
> 169                     }
> 170             }
> 171     exit:
> 172             if (block)
> 173                     free(block);
> 174
> >>>     CID 645495:         Uninitialized variables  (UNINIT)
> >>>     Using uninitialized value "ret".
> 175             return (ret == -1) ? -1 : nr_sect;
> 176     }
> 177
> 178     static int disk_read(__u32 sect, __u32 nr_sect, void *buf)
> 179     {
> 180             return disk_rw(sect, nr_sect, buf, true);
>
> ** CID 645494:       Integer handling issues  (BAD_SHIFT)
> /drivers/power/regulator/mt6359_regulator.c: 287           in
> mt6359_get_voltage_sel()
>
>
> _____________________________________________________________________________________________
> *** CID 645494:         Integer handling issues  (BAD_SHIFT)
> /drivers/power/regulator/mt6359_regulator.c: 287             in
> mt6359_get_voltage_sel()
> 281
> 282             selector = pmic_reg_read(dev->parent, info->desc.vsel_reg);
> 283             if (selector < 0)
> 284                     return selector;
> 285
> 286             selector &= info->desc.vsel_mask;
> >>>     CID 645494:         Integer handling issues  (BAD_SHIFT)
> >>>     In expression "selector >>= generic_ffs(info->desc.vsel_mask) - 1", shifting by a negative amount has undefined behavior.  The shift amount, "generic_ffs(info->desc.vsel_mask) - 1", is -1.
> 287             selector >>= ffs(info->desc.vsel_mask) - 1;
> 288
> 289             return selector;
> 290     }
> 291
> 292     static int mt6359p_vemc_get_voltage_sel(struct udevice *dev,
> struct mt6359_regulator_info *info)
>
> ** CID 645493:       Control flow issues  (DEADCODE)
> /drivers/firmware/scmi/pinctrl.c: 206           in
> scmi_pinctrl_settings_get_one()
>
>
> _____________________________________________________________________________________________
> *** CID 645493:         Control flow issues  (DEADCODE)
> /drivers/firmware/scmi/pinctrl.c: 206             in
> scmi_pinctrl_settings_get_one()
> 200
> 201             msg.out_msg = (u8 *)out;
> 202             msg.out_msg_sz = out_sz;
> 203             in.id = selector;
> 204             in.attr = 0;
> 205             if (config_type == SCMI_PINCTRL_CONFIG_SETTINGS_FUNCTION)
> >>>     CID 645493:         Control flow issues  (DEADCODE)
> >>>     Execution cannot reach the expression "in.attr" inside this statement: "in.attr = ({
>   ({
>     do  {...".
> 206                     in.attr = FIELD_PREP(GENMASK(19, 18), 2);
> 207             in.attr |= FIELD_PREP(GENMASK(17, 16), select_type);
> 208             if (config_type != SCMI_PINCTRL_CONFIG_SETTINGS_FUNCTION)
> 209                     in.attr |= FIELD_PREP(GENMASK(7, 0), config_type);
> 210
> 211             ret = devm_scmi_process_msg(dev, &msg);
>
> ** CID 645492:         (BUFFER_SIZE)
> /drivers/fwu-mdata/raw_mtd.c: 173           in get_fwu_mdata_dev()
> /drivers/fwu-mdata/raw_mtd.c: 183           in get_fwu_mdata_dev()
>
>
> _____________________________________________________________________________________________
> *** CID 645492:           (BUFFER_SIZE)
> /drivers/fwu-mdata/raw_mtd.c: 173             in get_fwu_mdata_dev()
> 167             }
> 168
> 169             /* Get the offset of primary and secondary mdata */
> 170             ret = ofnode_read_string_index(dev_ofnode(dev),
> "mdata-parts", 0, &label);
> 171             if (ret)
> 172                     return ret;
> >>>     CID 645492:           (BUFFER_SIZE)
> >>>     Calling "strncpy" with a maximum size argument of 50 bytes on destination array "mtd_priv->pri_label" of size 50 bytes might leave the destination string unterminated.
> 173             strncpy(mtd_priv->pri_label, label, 50);
> 174
> 175             ret = flash_partition_offset(mtd_dev, mtd_priv->pri_label, &offset);
> 176             if (ret <= 0)
> 177                     return ret;
> 178             mtd_priv->pri_offset = offset;
> /drivers/fwu-mdata/raw_mtd.c: 183             in get_fwu_mdata_dev()
> 177                     return ret;
> 178             mtd_priv->pri_offset = offset;
> 179
> 180             ret = ofnode_read_string_index(dev_ofnode(dev),
> "mdata-parts", 1, &label);
> 181             if (ret)
> 182                     return ret;
> >>>     CID 645492:           (BUFFER_SIZE)
> >>>     Calling "strncpy" with a maximum size argument of 50 bytes on destination array "mtd_priv->sec_label" of size 50 bytes might leave the destination string unterminated.
> 183             strncpy(mtd_priv->sec_label, label, 50);
> 184
> 185             ret = flash_partition_offset(mtd_dev, mtd_priv->sec_label, &offset);
> 186             if (ret <= 0)
> 187                     return ret;
> 188             mtd_priv->sec_offset = offset;
>
> ** CID 645491:       Security best practices violations  (STRING_OVERFLOW)
> /drivers/fwu-mdata/raw_mtd.c: 244           in fwu_mtd_image_info_populate()
>
>
> _____________________________________________________________________________________________
> *** CID 645491:         Security best practices violations  (STRING_OVERFLOW)
> /drivers/fwu-mdata/raw_mtd.c: 244             in fwu_mtd_image_info_populate()
> 238                             ofnode_read_u32(image, "size", &image_size);
> 239
> 240                             mtd_images[off_img].start = bank_offset + image_offset;
> 241                             mtd_images[off_img].size = image_size;
> 242                             mtd_images[off_img].bank_num = bank_num;
> 243                             mtd_images[off_img].image_num = image_num;
> >>>     CID 645491:         Security best practices violations  (STRING_OVERFLOW)
> >>>     You might overrun the 37-character fixed-size string "mtd_images[off_img].uuidbuf" by copying "uuid" without checking the length.
> 244                             strcpy(mtd_images[off_img].uuidbuf, uuid);
> 245                             log_debug("\tImage%d: %s @0x%x\n\n",
> 246                                       image_num, uuid, bank_offset + image_offset);
> 247                             off_img++;
> 248                     }
> 249             }
>
> ** CID 645490:       Integer handling issues  (BAD_SHIFT)
> /drivers/power/regulator/mt6359_regulator.c: 245           in
> mt6359p_vemc_set_voltage_sel()
>
>
> _____________________________________________________________________________________________
> *** CID 645490:         Integer handling issues  (BAD_SHIFT)
> /drivers/power/regulator/mt6359_regulator.c: 245             in
> mt6359p_vemc_set_voltage_sel()
> 239
> 240     static int mt6359p_vemc_set_voltage_sel(struct udevice *dev,
> 241                                             struct mt6359_regulator_info *info, unsigned int sel)
> 242     {
> 243             int ret;
> 244
> >>>     CID 645490:         Integer handling issues  (BAD_SHIFT)
> >>>     In expression "sel <<= generic_ffs(info->desc.vsel_mask) - 1", shifting by a negative amount has undefined behavior.  The shift amount, "generic_ffs(info->desc.vsel_mask) - 1", is -1.
> 245             sel <<= ffs(info->desc.vsel_mask) - 1;
> 246             ret = pmic_reg_write(dev->parent, MT6359P_TMA_KEY_ADDR,
> MT6359P_TMA_KEY);
> 247             if (ret)
> 248                     return ret;
> 249
> 250             ret = pmic_reg_read(dev->parent, MT6359P_VM_MODE_ADDR);
>
> ** CID 645489:       Integer handling issues  (BAD_SHIFT)
> /drivers/power/regulator/mt6359_regulator.c: 234           in
> mt6359_set_voltage_sel_regmap()
>
>
> _____________________________________________________________________________________________
> *** CID 645489:         Integer handling issues  (BAD_SHIFT)
> /drivers/power/regulator/mt6359_regulator.c: 234             in
> mt6359_set_voltage_sel_regmap()
> 228     };
> 229
> 230     static int mt6359_set_voltage_sel_regmap(struct udevice *dev,
> 231                                              struct mt6359_regulator_info *info,
> 232                                              unsigned int sel)
> 233     {
> >>>     CID 645489:         Integer handling issues  (BAD_SHIFT)
> >>>     In expression "sel <<= generic_ffs(info->desc.vsel_mask) - 1", shifting by a negative amount has undefined behavior.  The shift amount, "generic_ffs(info->desc.vsel_mask) - 1", is -1.
> 234             sel <<= ffs(info->desc.vsel_mask) - 1;
> 235
> 236             return pmic_clrsetbits(dev->parent, info->desc.vsel_reg,
> 237                                    info->desc.vsel_mask, sel);
> 238     }
> 239
>
> ** CID 645488:       Error handling issues  (CHECKED_RETURN)
> /tools/fwumdata_src/fwumdata.c: 189           in read_device()
>
>
> _____________________________________________________________________________________________
> *** CID 645488:         Error handling issues  (CHECKED_RETURN)
> /tools/fwumdata_src/fwumdata.c: 189             in read_device()
> 183     {
> 184             if (lseek(dev->fd, dev->devoff, SEEK_SET) < 0) {
> 185                     fprintf(stderr, "Seek failed: %s\n", strerror(errno));
> 186                     return -errno;
> 187             }
> 188
> >>>     CID 645488:         Error handling issues  (CHECKED_RETURN)
> >>>     "read(int, void *, size_t)" returns the number of bytes read, but it is ignored.
> 189             if (read(dev->fd, buf, count) < 0) {
> 190                     fprintf(stderr, "Read failed: %s\n", strerror(errno));
> 191                     return -errno;
> 192             }
> 193
> 194             return 0;
>
> ** CID 645487:       Insecure data handling  (TAINTED_SCALAR)
> /lib/smbios.c: 1099           in smbios_write_type9_1slot()
>
>
> _____________________________________________________________________________________________
> *** CID 645487:         Insecure data handling  (TAINTED_SCALAR)
> /lib/smbios.c: 1099             in smbios_write_type9_1slot()
> 1093             * TODO:
> 1094             * peer_groups = <peer_grouping_count> * SMBIOS_TYPE9_PGROUP_SIZE
> 1095             */
> 1096            len += pgroups_size;
> 1097
> 1098            t = map_sysmem(*current, len);
> >>>     CID 645487:         Insecure data handling  (TAINTED_SCALAR)
> >>>     Passing tainted expression "len" to "memset", which uses it as an offset. [Note: The source code implementation of the function has been overridden by a builtin model.]

Fixed with patch at:
https://lore.kernel.org/u-boot/20260407204113.3102785-1-raymondmaoca@gmail.com/T/#u

Regards,
Raymond

> 1099            memset(t, 0, len);
> 1100
> 1101            fill_smbios_header(t, SMBIOS_SYSTEM_SLOTS, len, handle);
> 1102
> 1103            /* eos is at the end of the structure */
> 1104            eos_addr = (u8 *)t + len - sizeof(t->eos);
>
> ** CID 645486:       Integer handling issues  (BAD_SHIFT)
> /drivers/power/regulator/mt6359_regulator.c: 312           in
> mt6359p_vemc_get_voltage_sel()
>
>
> _____________________________________________________________________________________________
> *** CID 645486:         Integer handling issues  (BAD_SHIFT)
> /drivers/power/regulator/mt6359_regulator.c: 312             in
> mt6359p_vemc_get_voltage_sel()
> 306                     return -EINVAL;
> 307             }
> 308             if (selector < 0)
> 309                     return selector;
> 310
> 311             selector &= info->desc.vsel_mask;
> >>>     CID 645486:         Integer handling issues  (BAD_SHIFT)
> >>>     In expression "selector >>= generic_ffs(info->desc.vsel_mask) - 1", shifting by a negative amount has undefined behavior.  The shift amount, "generic_ffs(info->desc.vsel_mask) - 1", is -1.
> 312             selector >>= ffs(info->desc.vsel_mask) - 1;
> 313
> 314             return selector;
> 315     }
> 316
> 317     static int mt6359_get_enable(struct udevice *dev)
>
>
>
> View Defects in Coverity Scan
> <https://scan.coverity.com/projects/das-u-boot?tab=overview>
>
> Best regards,
>
> The Coverity Scan Admin Team
>
> ----- End forwarded message -----
>
> --
> Tom

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2026-04-07 20:44 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-07 17:15 Fwd: New Defects reported by Coverity Scan for Das U-Boot Tom Rini
2024-10-07 18:17 ` Richard Weinberger
2024-10-07 20:01   ` Tom Rini
  -- strict thread matches above, loose matches on Subject: below --
2026-04-06 19:12 Fwd: " Tom Rini
2026-04-07 20:44 ` Raymond Mao
2025-12-08 19:38 Fwd: " Tom Rini
2025-12-09 11:06 ` Adriana Nicolae
2025-12-09 14:24   ` Tom Rini
2025-11-10 18:55 Fwd: " Tom Rini
2025-11-12  8:53 ` Kory Maincent
2025-08-06 18:35 Fwd: " Tom Rini
2025-08-07  1:50 ` Maniyam, Dinesh
2025-07-08 14:10 Fwd: " Tom Rini
2025-07-09  9:13 ` Sughosh Ganu
2025-02-10 22:26 Fwd: " Tom Rini
2025-02-11 22:24 ` Raymond Mao
2025-02-11 22:30   ` Tom Rini
2024-12-31 13:55 Fwd: " Tom Rini
2025-01-01 10:50 ` Abbarapu, Venkatesh
2025-01-02 16:59   ` Tom Rini
2024-10-16  3:47 Fwd: " Tom Rini
2024-10-16  6:12 ` Ilias Apalodimas
2024-10-16  8:20   ` Abbarapu, Venkatesh
2024-10-16 15:23 ` Raymond Mao
2024-04-22 21:48 Fwd: " Tom Rini
2024-04-23  6:19 ` Ilias Apalodimas
     [not found] <65a933ab652b3_da12cbd3e77f998728e5@prd-scan-dashboard-0.mail>
2024-01-19  8:47 ` Fwd: " Heinrich Schuchardt
2024-01-22  6:44   ` Masahisa Kojima
     [not found] <618040194ba5b_1e5e522b0f269199b041648@prd-scan-dashboard-0.mail>
2021-11-01 20:21 ` Heinrich Schuchardt
     [not found] <603e3e0f4e143_13ed2a2aea2fe5af5856843@prd-scan-dashboard-0.mail>
     [not found] ` <c246b8e9-daea-389d-4437-9ce785a007e7@gmx.de>
2021-03-24 22:00   ` Simon Glass

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox