[coreboot] New Defects reported by Coverity Scan for coreboot

scan-admin at coverity.com scan-admin at coverity.com
Tue Apr 12 13:38:51 CEST 2016


Hi,

Please find the latest report on new defect(s) introduced to coreboot found with Coverity Scan.

78 new defect(s) introduced to coreboot found with Coverity Scan.
88 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 78 defect(s)


** CID 1354615:  Memory - illegal accesses  (OVERRUN)
/src/cpu/ti/am335x/gpio.c: 30 in gpio_regs_and_bit()


________________________________________________________________________________________________________
*** CID 1354615:  Memory - illegal accesses  (OVERRUN)
/src/cpu/ti/am335x/gpio.c: 30 in gpio_regs_and_bit()
24     
25     	if (bank > ARRAY_SIZE(am335x_gpio_banks)) {
26     		printk(BIOS_ERR, "Bad gpio index %d.\n", gpio);
27     		return NULL;
28     	}
29     	*bit = 1 << (gpio % 32);
>>>     CID 1354615:  Memory - illegal accesses  (OVERRUN)
>>>     Overrunning array "am335x_gpio_banks" of 4 4-byte elements at element index 4 (byte offset 16) using index "bank" (which evaluates to 4).
30     	return am335x_gpio_banks[bank];
31     }
32     
33     void am335x_disable_gpio_irqs(void)
34     {
35     	int i;

** CID 1353793:  Resource leaks  (RESOURCE_LEAK)
/util/nvidia/cbootimage/src/data_layout.c: 1096 in resign_bl()


________________________________________________________________________________________________________
*** CID 1353793:  Resource leaks  (RESOURCE_LEAK)
/util/nvidia/cbootimage/src/data_layout.c: 1096 in resign_bl()
1090     
1091     	if (read_from_image(context->input_image_filename,
1092     				offset, bl_length,
1093     				&image, &image_actual_size, file_type_bin)) {
1094     		printf("Error reading image file %s.\n",
1095     				context->input_image_filename);
>>>     CID 1353793:  Resource leaks  (RESOURCE_LEAK)
>>>     Variable "image" going out of scope leaks the storage it points to.
1096     		return -ENOMEM;
1097     	}
1098     
1099     	pages_in_image = ICEIL(image_actual_size, page_size);
1100     
1101     	/* Create a local copy of the bl */

** CID 1353781:  Control flow issues  (NO_EFFECT)
/util/nvidia/cbootimage/src/cbootimage.c: 242 in main()


________________________________________________________________________________________________________
*** CID 1353781:  Control flow issues  (NO_EFFECT)
/util/nvidia/cbootimage/src/cbootimage.c: 242 in main()
236     			context.input_image_filename);
237     			goto fail;
238     		}
239     
240     		/* Get BCT_SIZE from input image file  */
241     		bct_size = get_bct_size_from_image(&context);
>>>     CID 1353781:  Control flow issues  (NO_EFFECT)
>>>     This less-than-zero comparison of an unsigned value is never true. "bct_size < 0U".
242     		if (bct_size < 0) {
243     			printf("Error: Invalid input image file %s\n",
244     			context.input_image_filename);
245     			goto fail;
246     		}
247     

** CID 1353028:  Error handling issues  (NEGATIVE_RETURNS)
/util/amdfwtool/amdfwtool.c: 284 in integrate_psp_firmwares()


________________________________________________________________________________________________________
*** CID 1353028:  Error handling issues  (NEGATIVE_RETURNS)
/util/amdfwtool/amdfwtool.c: 284 in integrate_psp_firmwares()
278     			pspdir[4+4*i+2] = 1;
279     			pspdir[4+4*i+3] = 0;
280     		} else if (fw_table[i].filename != NULL) {
281     			pspdir[4+4*i+0] = fw_table[i].type;
282     
283     			fd = open (fw_table[i].filename, O_RDONLY);
>>>     CID 1353028:  Error handling issues  (NEGATIVE_RETURNS)
>>>     "fd" is passed to a parameter that cannot be negative. [Note: The source code implementation of the function has been overridden by a builtin model.]
284     			fstat(fd, &fd_stat);
285     			pspdir[4+4*i+1] = fd_stat.st_size;
286     
287     			pspdir[4+4*i+2] = pos + ROM_BASE_ADDRESS;
288     			pspdir[4+4*i+3] = 0;
289     

** CID 1353027:  Error handling issues  (NEGATIVE_RETURNS)
/util/amdfwtool/amdfwtool.c: 239 in integrate_firmwares()


________________________________________________________________________________________________________
*** CID 1353027:  Error handling issues  (NEGATIVE_RETURNS)
/util/amdfwtool/amdfwtool.c: 239 in integrate_firmwares()
233     	struct stat fd_stat;
234     	int i;
235     
236     	for (i = 0; fw_table[i].type != AMD_FW_INVALID; i ++) {
237     		if (fw_table[i].filename != NULL) {
238     			fd = open (fw_table[i].filename, O_RDONLY);
>>>     CID 1353027:  Error handling issues  (NEGATIVE_RETURNS)
>>>     "fd" is passed to a parameter that cannot be negative. [Note: The source code implementation of the function has been overridden by a builtin model.]
239     			fstat(fd, &fd_stat);
240     
241     			switch (fw_table[i].type) {
242     			case AMD_FW_IMC:
243     				pos = ALIGN(pos, 0x10000);
244     				romsig[1] = pos + ROM_BASE_ADDRESS;

** CID 1353022:  Error handling issues  (CHECKED_RETURN)
/util/nvidia/cbootimage/src/cbootimage.c: 297 in main()


________________________________________________________________________________________________________
*** CID 1353022:  Error handling issues  (CHECKED_RETURN)
/util/nvidia/cbootimage/src/cbootimage.c: 297 in main()
291     		begin_update(&context);
292     		/* Signing the bct. */
293     		e = sign_bct(&context, context.bct);
294     		if (e != 0) 
295     			printf("Signing BCT failed, error: %d.\n", e);
296     
>>>     CID 1353022:  Error handling issues  (CHECKED_RETURN)
>>>     Calling "fwrite" without checking return value (as is done elsewhere 33 out of 41 times).
297     		fwrite(context.bct, 1, context.bct_size,
298     			context.raw_file);
299     		printf("New BCT file %s has been successfully generated!\n",
300     			context.output_image_filename);
301     		goto fail;
302     	}

** CID 1353021:  Error handling issues  (CHECKED_RETURN)
/util/amdfwtool/amdfwtool.c: 290 in integrate_psp_firmwares()


________________________________________________________________________________________________________
*** CID 1353021:  Error handling issues  (CHECKED_RETURN)
/util/amdfwtool/amdfwtool.c: 290 in integrate_psp_firmwares()
284     			fstat(fd, &fd_stat);
285     			pspdir[4+4*i+1] = fd_stat.st_size;
286     
287     			pspdir[4+4*i+2] = pos + ROM_BASE_ADDRESS;
288     			pspdir[4+4*i+3] = 0;
289     
>>>     CID 1353021:  Error handling issues  (CHECKED_RETURN)
>>>     "read(int, void *, size_t)" returns the number of bytes read, but it is ignored.
290     			read (fd, base+pos, fd_stat.st_size);
291     
292     			pos += fd_stat.st_size;
293     			pos = ALIGN(pos, 0x100);
294     			close (fd);
295     		} else {

** CID 1353020:  Error handling issues  (CHECKED_RETURN)
/util/amdfwtool/amdfwtool.c: 284 in integrate_psp_firmwares()


________________________________________________________________________________________________________
*** CID 1353020:  Error handling issues  (CHECKED_RETURN)
/util/amdfwtool/amdfwtool.c: 284 in integrate_psp_firmwares()
278     			pspdir[4+4*i+2] = 1;
279     			pspdir[4+4*i+3] = 0;
280     		} else if (fw_table[i].filename != NULL) {
281     			pspdir[4+4*i+0] = fw_table[i].type;
282     
283     			fd = open (fw_table[i].filename, O_RDONLY);
>>>     CID 1353020:  Error handling issues  (CHECKED_RETURN)
>>>     Calling "fstat(fd, &fd_stat)" without checking return value. This library function may fail and return an error code. [Note: The source code implementation of the function has been overridden by a builtin model.]
284     			fstat(fd, &fd_stat);
285     			pspdir[4+4*i+1] = fd_stat.st_size;
286     
287     			pspdir[4+4*i+2] = pos + ROM_BASE_ADDRESS;
288     			pspdir[4+4*i+3] = 0;
289     

** CID 1353019:  Error handling issues  (CHECKED_RETURN)
/util/amdfwtool/amdfwtool.c: 257 in integrate_firmwares()


________________________________________________________________________________________________________
*** CID 1353019:  Error handling issues  (CHECKED_RETURN)
/util/amdfwtool/amdfwtool.c: 257 in integrate_firmwares()
251     				break;
252     			default:
253     				/* Error */
254     				break;
255     			}
256     
>>>     CID 1353019:  Error handling issues  (CHECKED_RETURN)
>>>     "read(int, void *, size_t)" returns the number of bytes read, but it is ignored.
257     			read (fd, base+pos, fd_stat.st_size);
258     
259     			pos += fd_stat.st_size;
260     			pos = ALIGN(pos, 0x100);
261     			close (fd);
262     		}

** CID 1353018:  Error handling issues  (CHECKED_RETURN)
/util/amdfwtool/amdfwtool.c: 239 in integrate_firmwares()


________________________________________________________________________________________________________
*** CID 1353018:  Error handling issues  (CHECKED_RETURN)
/util/amdfwtool/amdfwtool.c: 239 in integrate_firmwares()
233     	struct stat fd_stat;
234     	int i;
235     
236     	for (i = 0; fw_table[i].type != AMD_FW_INVALID; i ++) {
237     		if (fw_table[i].filename != NULL) {
238     			fd = open (fw_table[i].filename, O_RDONLY);
>>>     CID 1353018:  Error handling issues  (CHECKED_RETURN)
>>>     Calling "fstat(fd, &fd_stat)" without checking return value. This library function may fail and return an error code. [Note: The source code implementation of the function has been overridden by a builtin model.]
239     			fstat(fd, &fd_stat);
240     
241     			switch (fw_table[i].type) {
242     			case AMD_FW_IMC:
243     				pos = ALIGN(pos, 0x10000);
244     				romsig[1] = pos + ROM_BASE_ADDRESS;

** CID 1347367:  Control flow issues  (DEADCODE)
/util/cbfstool/elfheaders.c: 971 in write_phdrs()


________________________________________________________________________________________________________
*** CID 1347367:  Control flow issues  (DEADCODE)
/util/cbfstool/elfheaders.c: 971 in write_phdrs()
965     		if (!(sec->shdr.sh_flags & SHF_ALLOC))
966     			continue;
967     
968     		if (!section_consecutive(ew, i)) {
969     			/* Write out previously set phdr. */
970     			if (num_written != 0) {
>>>     CID 1347367:  Control flow issues  (DEADCODE)
>>>     Execution cannot reach this statement: "phdr_write(ew, phdrs, &phdr);".
971     				phdr_write(ew, phdrs, &phdr);
972     				num_written++;
973     			}
974     			phdr.p_type = PT_LOAD;
975     			phdr.p_offset = sec->shdr.sh_offset;
976     			phdr.p_vaddr = sec->shdr.sh_addr;

** CID 1347358:  Error handling issues  (NEGATIVE_RETURNS)
/util/amdfwtool/amdfwtool.c: 586 in main()


________________________________________________________________________________________________________
*** CID 1347358:  Error handling issues  (NEGATIVE_RETURNS)
/util/amdfwtool/amdfwtool.c: 586 in main()
580     		current = integrate_psp_firmwares(rom, current, psp2dir, amd_psp2_fw_table);
581     #endif
582     	}
583     #endif
584     
585     	targetfd = open(output, O_RDWR | O_CREAT | O_TRUNC, 0666);
>>>     CID 1347358:  Error handling issues  (NEGATIVE_RETURNS)
>>>     "targetfd" is passed to a parameter that cannot be negative.
586     	write(targetfd, amd_romsig, current - AMD_ROMSIG_OFFSET);
587     	close(targetfd);
588     	free(rom);
589     
590     	return 0;

** CID 1347335:    (UNINIT)
/util/cbfstool/elfheaders.c: 995 in write_phdrs()
/util/cbfstool/elfheaders.c: 992 in write_phdrs()
/util/cbfstool/elfheaders.c: 994 in write_phdrs()
/util/cbfstool/elfheaders.c: 996 in write_phdrs()


________________________________________________________________________________________________________
*** CID 1347335:    (UNINIT)
/util/cbfstool/elfheaders.c: 995 in write_phdrs()
989     			 * (sh_size == file size). This is standard in that
990     			 * an ELF section doesn't have a file size component. */
991     			if (sec->shdr.sh_flags & SHF_EXECINSTR)
992     				phdr.p_flags |= PF_X | PF_R;
993     			if (sec->shdr.sh_flags & SHF_WRITE)
994     				phdr.p_flags |= PF_W;
>>>     CID 1347335:    (UNINIT)
>>>     Using uninitialized value "phdr.p_filesz".
995     			phdr.p_filesz += buffer_size(&sec->content);
996     			phdr.p_memsz += sec->shdr.sh_size;
997     		}
998     	}
999     
1000     	/* Write out the last phdr. */
/util/cbfstool/elfheaders.c: 992 in write_phdrs()
986     		} else {
987     			/* Accumulate file size and memsize. The assumption
988     			 * is that each section is either NOBITS or full
989     			 * (sh_size == file size). This is standard in that
990     			 * an ELF section doesn't have a file size component. */
991     			if (sec->shdr.sh_flags & SHF_EXECINSTR)
>>>     CID 1347335:    (UNINIT)
>>>     Using uninitialized value "phdr.p_flags".
992     				phdr.p_flags |= PF_X | PF_R;
993     			if (sec->shdr.sh_flags & SHF_WRITE)
994     				phdr.p_flags |= PF_W;
995     			phdr.p_filesz += buffer_size(&sec->content);
996     			phdr.p_memsz += sec->shdr.sh_size;
997     		}
/util/cbfstool/elfheaders.c: 994 in write_phdrs()
988     			 * is that each section is either NOBITS or full
989     			 * (sh_size == file size). This is standard in that
990     			 * an ELF section doesn't have a file size component. */
991     			if (sec->shdr.sh_flags & SHF_EXECINSTR)
992     				phdr.p_flags |= PF_X | PF_R;
993     			if (sec->shdr.sh_flags & SHF_WRITE)
>>>     CID 1347335:    (UNINIT)
>>>     Using uninitialized value "phdr.p_flags".
994     				phdr.p_flags |= PF_W;
995     			phdr.p_filesz += buffer_size(&sec->content);
996     			phdr.p_memsz += sec->shdr.sh_size;
997     		}
998     	}
999     
/util/cbfstool/elfheaders.c: 996 in write_phdrs()
990     			 * an ELF section doesn't have a file size component. */
991     			if (sec->shdr.sh_flags & SHF_EXECINSTR)
992     				phdr.p_flags |= PF_X | PF_R;
993     			if (sec->shdr.sh_flags & SHF_WRITE)
994     				phdr.p_flags |= PF_W;
995     			phdr.p_filesz += buffer_size(&sec->content);
>>>     CID 1347335:    (UNINIT)
>>>     Using uninitialized value "phdr.p_memsz".
996     			phdr.p_memsz += sec->shdr.sh_size;
997     		}
998     	}
999     
1000     	/* Write out the last phdr. */
1001     	if (num_written != ew->ehdr.e_phnum)

** CID 1347333:  Memory - illegal accesses  (UNINIT)
/util/amdfwtool/amdfwtool.c: 585 in main()


________________________________________________________________________________________________________
*** CID 1347333:  Memory - illegal accesses  (UNINIT)
/util/amdfwtool/amdfwtool.c: 585 in main()
579     #else
580     		current = integrate_psp_firmwares(rom, current, psp2dir, amd_psp2_fw_table);
581     #endif
582     	}
583     #endif
584     
>>>     CID 1347333:  Memory - illegal accesses  (UNINIT)
>>>     Using uninitialized value "output" when calling "open".
585     	targetfd = open(output, O_RDWR | O_CREAT | O_TRUNC, 0666);
586     	write(targetfd, amd_romsig, current - AMD_ROMSIG_OFFSET);
587     	close(targetfd);
588     	free(rom);
589     
590     	return 0;

** CID 1325840:  Memory - illegal accesses  (OVERRUN)
/util/cbfstool/cbfs_image.c: 1155 in cbfs_print_entry_info()


________________________________________________________________________________________________________
*** CID 1325840:  Memory - illegal accesses  (OVERRUN)
/util/cbfstool/cbfs_image.c: 1155 in cbfs_print_entry_info()
1149     	while ((hash = cbfs_file_get_next_hash(entry, hash)) != NULL) {
1150     		unsigned int hash_type = ntohl(hash->hash_type);
1151     		if (hash_type > CBFS_NUM_SUPPORTED_HASHES) {
1152     			fprintf(fp, "invalid hash type %d\n", hash_type);
1153     			break;
1154     		}
>>>     CID 1325840:  Memory - illegal accesses  (OVERRUN)
>>>     Overrunning array "widths_cbfs_hash" of 4 8-byte elements at element index 4 (byte offset 32) using index "hash_type" (which evaluates to 4).
1155     		size_t hash_len = widths_cbfs_hash[hash_type];
1156     		char *hash_str = bintohex(hash->hash_data, hash_len);
1157     		uint8_t local_hash[hash_len];
1158     		if (vb2_digest_buffer(CBFS_SUBHEADER(entry),
1159     			ntohl(entry->len), hash_type, local_hash,
1160     			hash_len) != VB2_SUCCESS) {

** CID 1325836:  Resource leaks  (RESOURCE_LEAK)
/util/cbfstool/cbfs_image.c: 1162 in cbfs_print_entry_info()


________________________________________________________________________________________________________
*** CID 1325836:  Resource leaks  (RESOURCE_LEAK)
/util/cbfstool/cbfs_image.c: 1162 in cbfs_print_entry_info()
1156     		char *hash_str = bintohex(hash->hash_data, hash_len);
1157     		uint8_t local_hash[hash_len];
1158     		if (vb2_digest_buffer(CBFS_SUBHEADER(entry),
1159     			ntohl(entry->len), hash_type, local_hash,
1160     			hash_len) != VB2_SUCCESS) {
1161     			fprintf(fp, "failed to hash '%s'\n", name);
>>>     CID 1325836:  Resource leaks  (RESOURCE_LEAK)
>>>     Variable "hash_str" going out of scope leaks the storage it points to.
1162     			break;
1163     		}
1164     		int valid = memcmp(local_hash, hash->hash_data, hash_len) == 0;
1165     		const char *valid_str = valid ? "valid" : "invalid";
1166     
1167     		fprintf(fp, "    hash %s:%s %s\n",

** CID 1323515:  Error handling issues  (CHECKED_RETURN)
/util/broadcom/secimage/sbi.c: 112 in CreateSecureBootImage()


________________________________________________________________________________________________________
*** CID 1323515:  Error handling issues  (CHECKED_RETURN)
/util/broadcom/secimage/sbi.c: 112 in CreateSecureBootImage()
106     		} else {
107     			return SBIUsage();
108     		}
109     		--ac, ++av;
110     	}
111     
>>>     CID 1323515:  Error handling issues  (CHECKED_RETURN)
>>>     Calling "stat(bl, &file_stat)" without checking return value. This library function may fail and return an error code. [Note: The source code implementation of the function has been overridden by a builtin model.]
112     	stat(bl, &file_stat);
113     	filesize = file_stat.st_size + MIN_SIZE;
114     	buf = calloc(sizeof(uint8_t), filesize);
115     
116     	if (buf == NULL) {
117     		puts("Memory allocation error");

** CID 1323512:  Null pointer dereferences  (FORWARD_NULL)
/util/broadcom/secimage/sbi.c: 112 in CreateSecureBootImage()


________________________________________________________________________________________________________
*** CID 1323512:  Null pointer dereferences  (FORWARD_NULL)
/util/broadcom/secimage/sbi.c: 112 in CreateSecureBootImage()
106     		} else {
107     			return SBIUsage();
108     		}
109     		--ac, ++av;
110     	}
111     
>>>     CID 1323512:  Null pointer dereferences  (FORWARD_NULL)
>>>     Passing null pointer "bl" to "stat", which dereferences it. [Note: The source code implementation of the function has been overridden by a builtin model.]
112     	stat(bl, &file_stat);
113     	filesize = file_stat.st_size + MIN_SIZE;
114     	buf = calloc(sizeof(uint8_t), filesize);
115     
116     	if (buf == NULL) {
117     		puts("Memory allocation error");

** CID 1323511:  Null pointer dereferences  (FORWARD_NULL)
/util/broadcom/secimage/sbi.c: 80 in CreateSecureBootImage()


________________________________________________________________________________________________________
*** CID 1323511:  Null pointer dereferences  (FORWARD_NULL)
/util/broadcom/secimage/sbi.c: 80 in CreateSecureBootImage()
74      * Purpose :
75      * Input   : none
76      * Output  : none
77      *---------------------------------------------------------------------*/
78     int CreateSecureBootImage(int ac, char **av)
79     {
>>>     CID 1323511:  Null pointer dereferences  (FORWARD_NULL)
>>>     Assigning: "privkey" = "NULL".
80     	char *outfile, *configfile, *arg, *privkey = NULL, *bl = NULL;
81     	int status = 0;
82     	uint32_t sbiLen;
83     	struct stat file_stat;
84     	uint32_t add_header = 1;
85     	outfile = *av;

** CID 1323510:  Error handling issues  (NEGATIVE_RETURNS)
/util/broadcom/secimage/io.c: 81 in DataRead()


________________________________________________________________________________________________________
*** CID 1323510:  Error handling issues  (NEGATIVE_RETURNS)
/util/broadcom/secimage/io.c: 81 in DataRead()
75     	len = FileSizeGet(file);
76     	if (len < *length)
77     		*length = len;
78     	else
79     		/* Do not exceed the maximum length of the buffer */
80     		len = *length;
>>>     CID 1323510:  Error handling issues  (NEGATIVE_RETURNS)
>>>     "len" is passed to a parameter that cannot be negative.
81     	if (fread((uint8_t *)buf, 1, len, file) != len) {
82     		printf("Error reading data (%d bytes) from file: %s\n",
83     		       len, filename);
84     		return -1;
85     	}
86     	fclose(file);


________________________________________________________________________________________________________
To view the defects in Coverity Scan visit, https://scan.coverity.com/projects/coreboot?tab=overview

To manage Coverity Scan email notifications for "coreboot at coreboot.org", click https://scan.coverity.com/subscriptions/edit?email=coreboot%40coreboot.org&token=49533df725f93b78361afb7b89ccde93




More information about the coreboot mailing list