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

[Guillaume La Roque <glaroque@baylibre.com>]

Hi Tom,


sorry for delay, i check defects please see my comments inline

Le 16/01/2026 à 20:43, Tom Rini a écrit :
> Hey all,
>
> Here's the latest report from Coverity scan. For the LZMA ones, the
> _pad_ stuff seems to be a false positive (the _pad_ byte is just for
> padding and not refernced) and the flow control one is how that's
> written for whatever reason the upstream author wanted it like that.
>
> ---------- Forwarded message ---------
> From: <scan-admin@coverity.com>
> Date: Fri, Jan 16, 2026 at 1: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.
>
>     - *New Defects Found:* 7
>     - 2 defect(s), reported by Coverity Scan earlier, were marked fixed in
>     the recent build analyzed by Coverity Scan.
>     - *Defects Shown:* Showing 7 of 7 defect(s)
>
> Defect Details
>
> ** CID 641431:         (TAINTED_SCALAR)
>
>
> _____________________________________________________________________________________________
> *** CID 641431:           (TAINTED_SCALAR)
> /boot/image-android.c: 434             in android_image_get_kernel()
> 428     		if (*newbootargs) /* If there is something in newbootargs, a
> space is needed */
> 429     			strcat(newbootargs, " ");
> 430     		strcat(newbootargs, img_data.kcmdline_extra);
> 431     	}
> 432
> 433     	env_set("bootargs", newbootargs);
>>>>      CID 641431:           (TAINTED_SCALAR)
>>>>      Passing tainted expression "*newbootargs" to "dlfree", which uses it as an offset.
> 434     	free(newbootargs);
> 435
> 436     	if (os_data) {
> 437     		if (image_get_magic(ihdr) == IH_MAGIC) {
> 438     			*os_data = image_get_data(ihdr);
> 439     		} else {
> /boot/image-android.c: 433             in android_image_get_kernel()
> 427     	if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
> 428     		if (*newbootargs) /* If there is something in newbootargs, a
> space is needed */
> 429     			strcat(newbootargs, " ");
> 430     		strcat(newbootargs, img_data.kcmdline_extra);
> 431     	}
> 432
>>>>      CID 641431:           (TAINTED_SCALAR)
>>>>      Passing tainted expression "newbootargs" to "env_set", which uses it as an offset.
> 433     	env_set("bootargs", newbootargs);
> 434     	free(newbootargs);
> 435
> 436     	if (os_data) {
> 437     		if (image_get_magic(ihdr) == IH_MAGIC) {
> 438     			*os_data = image_get_data(ihdr);
> /boot/image-android.c: 434             in android_image_get_kernel()
> 428     		if (*newbootargs) /* If there is something in newbootargs, a
> space is needed */
> 429     			strcat(newbootargs, " ");
> 430     		strcat(newbootargs, img_data.kcmdline_extra);
> 431     	}
> 432
> 433     	env_set("bootargs", newbootargs);
>>>>      CID 641431:           (TAINTED_SCALAR)
>>>>      Passing tainted expression "*newbootargs" to "dlfree", which uses it as an offset.
> 434     	free(newbootargs);
> 435
> 436     	if (os_data) {
> 437     		if (image_get_magic(ihdr) == IH_MAGIC) {
> 438     			*os_data = image_get_data(ihdr);
> 439     		} else {
> /boot/image-android.c: 433             in android_image_get_kernel()
> 427     	if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
> 428     		if (*newbootargs) /* If there is something in newbootargs, a
> space is needed */
> 429     			strcat(newbootargs, " ");
> 430     		strcat(newbootargs, img_data.kcmdline_extra);
> 431     	}
> 432
>>>>      CID 641431:           (TAINTED_SCALAR)
>>>>      Passing tainted expression "newbootargs" to "env_set", which uses it as an offset.
> 433     	env_set("bootargs", newbootargs);
> 434     	free(newbootargs);
> 435
> 436     	if (os_data) {
> 437     		if (image_get_magic(ihdr) == IH_MAGIC) {
> 438     			*os_data = image_get_data(ihdr);
> /boot/image-android.c: 433             in android_image_get_kernel()
> 427     	if (img_data.kcmdline_extra && *img_data.kcmdline_extra) {
> 428     		if (*newbootargs) /* If there is something in newbootargs, a
> space is needed */
> 429     			strcat(newbootargs, " ");
> 430     		strcat(newbootargs, img_data.kcmdline_extra);
> 431     	}
> 432
>>>>      CID 641431:           (TAINTED_SCALAR)
>>>>      Passing tainted expression "newbootargs" to "env_set", which uses it as an offset.
> 433     	env_set("bootargs", newbootargs);
> 434     	free(newbootargs);
> 435
> 436     	if (os_data) {
> 437     		if (image_get_magic(ihdr) == IH_MAGIC) {
> 438     			*os_data = image_get_data(ihdr);
> /boot/image-android.c: 434             in android_image_get_kernel()
> 428     		if (*newbootargs) /* If there is something in newbootargs, a
> space is needed */
> 429     			strcat(newbootargs, " ");
> 430     		strcat(newbootargs, img_data.kcmdline_extra);
> 431     	}
> 432
> 433     	env_set("bootargs", newbootargs);
>>>>      CID 641431:           (TAINTED_SCALAR)
>>>>      Passing tainted expression "*newbootargs" to "dlfree", which uses it as an offset.

For CID 641431 : for me it's a false positives defect, malloc was done 
with strlen return and free done on malloc pointer.

> 434     	free(newbootargs);
> 435
> 436     	if (os_data) {
> 437     		if (image_get_magic(ihdr) == IH_MAGIC) {
> 438     			*os_data = image_get_data(ihdr);
> 439     		} else {
>
> ** CID 641430:         (TAINTED_SCALAR)
>
>
> _____________________________________________________________________________________________
> *** CID 641430:           (TAINTED_SCALAR)
> /cmd/abootimg.c: 244             in abootimg_get_ramdisk()
> 238     				      &rd_data, &rd_len))
> 239     		return CMD_RET_FAILURE;
> 240
> 241     	if (argc == 0) {
> 242     		printf("%lx\n", rd_data);
> 243     	} else {
>>>>      CID 641430:           (TAINTED_SCALAR)
>>>>      Passing tainted expression "rd_data" to "env_set_hex", which uses it as an offset.
> 244     		env_set_hex(argv[0], rd_data);
> 245     		if (argc == 2)
> 246     			env_set_hex(argv[1], rd_len);
> 247     	}
> 248
> 249     	return CMD_RET_SUCCESS;
> /cmd/abootimg.c: 246             in abootimg_get_ramdisk()
> 240
> 241     	if (argc == 0) {
> 242     		printf("%lx\n", rd_data);
> 243     	} else {
> 244     		env_set_hex(argv[0], rd_data);
> 245     		if (argc == 2)
>>>>      CID 641430:           (TAINTED_SCALAR)
>>>>      Passing tainted expression "rd_len" to "env_set_hex", which uses it as an offset.

CID 641430: false positive too. env_set_hex convert value on an env variable , so convert rd_len and rd_data
  in variable.

> 246     			env_set_hex(argv[1], rd_len);
> 247     	}
> 248
> 249     	return CMD_RET_SUCCESS;
> 250     }
> 251
>
> ** CID 641429:       Insecure data handling  (TAINTED_SCALAR)
>
>
> _____________________________________________________________________________________________
> *** CID 641429:         Insecure data handling  (TAINTED_SCALAR)
> /boot/image-android.c: 307             in android_image_get_data()
> 301     			printf("Incorrect vendor boot image header\n");
> 302     			unmap_sysmem(vhdr);
> 303     			unmap_sysmem(bhdr);
> 304     			return false;
> 305     		}
> 306     		android_boot_image_v3_v4_parse_hdr((const struct
> andr_boot_img_hdr_v3 *)bhdr, data);
>>>>      CID 641429:         Insecure data handling  (TAINTED_SCALAR)
>>>>      Passing tainted expression "vhdr->bootconfig_size" to "android_vendor_boot_image_v3_v4_parse_hdr", which uses it as a loop boundary.

CID 641429:  False positive too. "vhdr->bootconfig_size" come from android image so external source , not possible to validate if value is good or not except when AVB feature was enabled

> 307     		android_vendor_boot_image_v3_v4_parse_hdr(vhdr, data);
> 308     		unmap_sysmem(vhdr);
> 309     	} else {
> 310     		android_boot_image_v0_v1_v2_parse_hdr(bhdr, data);
> 311     	}
> 312
>
> ** CID 641428:         (TAINTED_SCALAR)
>
>
> _____________________________________________________________________________________________
> *** CID 641428:           (TAINTED_SCALAR)
> /boot/image-android.c: 658             in android_image_set_bootconfig()
> 652     		total_size += params_len + BOOTCONFIG_TRAILER_SIZE;
> 653
> 654     	/* Map Dest */
> 655     	ramdisk_dest = map_sysmem(ramdisk_addr, total_size);
> 656
> 657     	/* Copy data */
>>>>      CID 641428:           (TAINTED_SCALAR)
>>>>      Passing tainted expression "img_data.vendor_ramdisk_size" to "android_boot_append_bootconfig", which uses it as an offset.
> 658     	ret = android_boot_append_bootconfig(&img_data, params, params_len,
> 659     					     ramdisk_dest);
> 660
> 661     	unmap_sysmem(ramdisk_dest);
> 662     	free(params);
> 663     	free(new_bootargs);
> /boot/image-android.c: 658             in android_image_set_bootconfig()
> 652     		total_size += params_len + BOOTCONFIG_TRAILER_SIZE;
> 653
> 654     	/* Map Dest */
> 655     	ramdisk_dest = map_sysmem(ramdisk_addr, total_size);
> 656
> 657     	/* Copy data */
>>>>      CID 641428:           (TAINTED_SCALAR)
>>>>      Passing tainted expression "img_data.bootconfig_size" to "android_boot_append_bootconfig", which uses it as an offset.
> 658     	ret = android_boot_append_bootconfig(&img_data, params, params_len,
> 659     					     ramdisk_dest);
> 660
> 661     	unmap_sysmem(ramdisk_dest);
> 662     	free(params);
> 663     	free(new_bootargs);
> /boot/image-android.c: 658             in android_image_set_bootconfig()
> 652     		total_size += params_len + BOOTCONFIG_TRAILER_SIZE;
> 653
> 654     	/* Map Dest */
> 655     	ramdisk_dest = map_sysmem(ramdisk_addr, total_size);
> 656
> 657     	/* Copy data */
>>>>      CID 641428:           (TAINTED_SCALAR)
>>>>      Passing tainted expression "img_data.boot_ramdisk_size" to "android_boot_append_bootconfig", which uses it as an offset.

  CID 641428: for me it's false positive too. img_data.boot_ramdisk_size and vendor_ramdisk_size come from android image, it could be corrupted if we corrupt android image but it's an external source so difficult to say if value is corrupted or not , it's why on real device we have AB features to check it.

> 658     	ret = android_boot_append_bootconfig(&img_data, params, params_len,
> 659     					     ramdisk_dest);
> 660
> 661     	unmap_sysmem(ramdisk_dest);
> 662     	free(params);
> 663     	free(new_bootargs);
>
> ** CID 332278:       Control flow issues  (UNREACHABLE)
> /lib/lzma/LzmaDec.c: 720           in LzmaDec_TryDummy()
>
>
> _____________________________________________________________________________________________
> *** CID 332278:         Control flow issues  (UNREACHABLE)
> /lib/lzma/LzmaDec.c: 720             in LzmaDec_TryDummy()
> 714       UInt32 code = p->code;
> 715       const Byte *bufLimit = *bufOut;
> 716       const CLzmaProb *probs = GET_PROBS;
> 717       unsigned state = (unsigned)p->state;
> 718       ELzmaDummy res;
> 719
>>>>      CID 332278:         Control flow issues  (UNREACHABLE)
>>>>      Since the loop increment is unreachable, the loop body will never execute more than once.
> 720       for (;;)
> 721       {
> 722         const CLzmaProb *prob;
> 723         UInt32 bound;
> 724         unsigned ttt;
> 725         unsigned posState = CALC_POS_STATE(p->processedPos,
> ((unsigned)1 << p->prop.pb) - 1);
>
> ** CID 252901:       Uninitialized variables  (UNINIT)
> /lib/lzma/LzmaDec.c: 1295           in LzmaDec_AllocateProbs()
>
>
> _____________________________________________________________________________________________
> *** CID 252901:         Uninitialized variables  (UNINIT)
> /lib/lzma/LzmaDec.c: 1295             in LzmaDec_AllocateProbs()
> 1289
> 1290     SRes LzmaDec_AllocateProbs(CLzmaDec *p, const Byte *props,
> unsigned propsSize, ISzAllocPtr alloc)
> 1291     {
> 1292       CLzmaProps propNew;
> 1293       RINOK(LzmaProps_Decode(&propNew, props, propsSize))
> 1294       RINOK(LzmaDec_AllocateProbs2(p, &propNew, alloc))
>>>>      CID 252901:         Uninitialized variables  (UNINIT)
>>>>      Using uninitialized value "propNew". Field "propNew._pad_" is uninitialized.
> 1295       p->prop = propNew;
> 1296       return SZ_OK;
> 1297     }
> 1298
> 1299     SRes LzmaDec_Allocate(CLzmaDec *p, const Byte *props,
> unsigned propsSize, ISzAllocPtr alloc)
> 1300     {
>
> ** CID 252579:       Uninitialized variables  (UNINIT)
> /lib/lzma/LzmaDec.c: 1327           in LzmaDec_Allocate()
>
>
> _____________________________________________________________________________________________
> *** CID 252579:         Uninitialized variables  (UNINIT)
> /lib/lzma/LzmaDec.c: 1327             in LzmaDec_Allocate()
> 1321         {
> 1322           LzmaDec_FreeProbs(p, alloc);
> 1323           return SZ_ERROR_MEM;
> 1324         }
> 1325       }
> 1326       p->dicBufSize = dicBufSize;
>>>>      CID 252579:         Uninitialized variables  (UNINIT)
>>>>      Using uninitialized value "propNew". Field "propNew._pad_" is uninitialized.
> 1327       p->prop = propNew;
> 1328       return SZ_OK;
> 1329     }
> 1330
> 1331     SRes LzmaDecode(Byte *dest, SizeT *destLen, const Byte *src,
> SizeT *srcLen,
> 1332         const Byte *propData, unsigned propSize, ELzmaFinishMode
> finishMode,
>
>
>
> 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 -----
>
Regards,
Guillaume