[coreboot] New Defects reported by Coverity Scan for coreboot

scan-admin at coverity.com scan-admin at coverity.com
Fri Dec 2 13:20:16 CET 2016


Hi,

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

47 new defect(s) introduced to coreboot found with Coverity Scan.


New defect(s) Reported-by: Coverity Scan
Showing 20 of 47 defect(s)


** CID 1366756:  Control flow issues  (DEADCODE)
/src/lib/spd_bin.c: 165 in get_spd_smbus()


________________________________________________________________________________________________________
*** CID 1366756:  Control flow issues  (DEADCODE)
/src/lib/spd_bin.c: 165 in get_spd_smbus()
159     		get_spd(spd_data_ptr + i * CONFIG_DIMM_SPD_SIZE,
160     			0xA0 + (i << 1));
161     		blk->spd_array[i] = spd_data_ptr + i * CONFIG_DIMM_SPD_SIZE;
162     	}
163     
164     	for (j = i; j < CONFIG_DIMM_MAX; j++)
>>>     CID 1366756:  Control flow issues  (DEADCODE)
>>>     Execution cannot reach this statement: "blk->spd_array[j] = NULL;".
165     		blk->spd_array[j] = NULL;
166     
167     	update_spd_len(blk);

** CID 1366755:  Error handling issues  (CHECKED_RETURN)
/src/lib/spd_bin.c: 120 in get_spd_cbfs_rdev()


________________________________________________________________________________________________________
*** CID 1366755:  Error handling issues  (CHECKED_RETURN)
/src/lib/spd_bin.c: 120 in get_spd_cbfs_rdev()
114     int get_spd_cbfs_rdev(struct region_device *spd_rdev, u8 spd_index)
115     {
116     	struct cbfsf fh;
117     
118     	uint32_t cbfs_type = CBFS_TYPE_SPD;
119     
>>>     CID 1366755:  Error handling issues  (CHECKED_RETURN)
>>>     Calling "cbfs_boot_locate" without checking return value (as is done elsewhere 10 out of 11 times).
120     	cbfs_boot_locate(&fh, "spd.bin", &cbfs_type);
121     	cbfs_file_data(spd_rdev, &fh);
122     	return rdev_chain(spd_rdev, spd_rdev, spd_index * CONFIG_DIMM_SPD_SIZE,
123     							CONFIG_DIMM_SPD_SIZE);
124     }
125     

** CID 1361275:    (TAINTED_SCALAR)
/util/cbfstool/ifwitool.c: 838 in parse_subpart_dir()


________________________________________________________________________________________________________
*** CID 1361275:    (TAINTED_SCALAR)
/util/cbfstool/ifwitool.c: 831 in parse_subpart_dir()
825     	memcpy(hdr.name, data + offset, sizeof(hdr.name));
826     	offset += sizeof(hdr.name);
827     
828     	validate_subpart_dir_without_checksum((struct subpart_dir *)&hdr, name);
829     
830     	assert(size > subpart_dir_size(&hdr));
>>>     CID 1361275:    (TAINTED_SCALAR)
>>>     Passing tainted variable "subpart_dir_size(&hdr)" to a tainted sink.
831     	alloc_buffer(subpart_dir_buf, subpart_dir_size(&hdr), "Subpart Dir");
832     	memcpy(buffer_get(subpart_dir_buf), &hdr, SUBPART_DIR_HEADER_SIZE);
833     
834     	/* Read Subpart Dir entries. */
835     	struct subpart_dir *subpart_dir = buffer_get(subpart_dir_buf);
836     	struct subpart_dir_entry *e = &subpart_dir->e[0];
/util/cbfstool/ifwitool.c: 838 in parse_subpart_dir()
832     	memcpy(buffer_get(subpart_dir_buf), &hdr, SUBPART_DIR_HEADER_SIZE);
833     
834     	/* Read Subpart Dir entries. */
835     	struct subpart_dir *subpart_dir = buffer_get(subpart_dir_buf);
836     	struct subpart_dir_entry *e = &subpart_dir->e[0];
837     	uint32_t i;
>>>     CID 1361275:    (TAINTED_SCALAR)
>>>     Using tainted variable "hdr.num_entries" as a loop boundary.
838     	for (i = 0; i < hdr.num_entries; i++) {
839     		memcpy(e[i].name, data + offset, sizeof(e[i].name));
840     		offset += sizeof(e[i].name);
841     		offset = read_member(data, offset, sizeof(e[i].offset),
842     				     &e[i].offset);
843     		offset = read_member(data, offset, sizeof(e[i].length),

** CID 1361274:  Insecure data handling  (TAINTED_SCALAR)


________________________________________________________________________________________________________
*** CID 1361274:  Insecure data handling  (TAINTED_SCALAR)
/util/cbfstool/ifwitool.c: 717 in alloc_bpdt_buffer()
711     {
712     	struct bpdt_header bpdt_header;
713     	assert((offset + BPDT_HEADER_SIZE) < size);
714     	bpdt_read_header((uint8_t *)data + offset, &bpdt_header, name);
715     
716     	/* Buffer to read BPDT header and entries. */
>>>     CID 1361274:  Insecure data handling  (TAINTED_SCALAR)
>>>     Passing tainted variable "get_bpdt_size(&bpdt_header)" to a tainted sink.
717     	alloc_buffer(b, get_bpdt_size(&bpdt_header), name);
718     
719     	struct bpdt *bpdt = buffer_get(b);
720     	memcpy(&bpdt->h, &bpdt_header, BPDT_HEADER_SIZE);
721     
722     	/*

** CID 1361253:  Memory - illegal accesses  (BUFFER_SIZE_WARNING)
/util/cbfstool/ifwitool.c: 1300 in init_subpart_dir_entry()


________________________________________________________________________________________________________
*** CID 1361253:  Memory - illegal accesses  (BUFFER_SIZE_WARNING)
/util/cbfstool/ifwitool.c: 1300 in init_subpart_dir_entry()
1294     static size_t init_subpart_dir_entry(struct subpart_dir_entry *e,
1295     				     struct buffer *b, size_t offset)
1296     {
1297     	memset(e, 0, sizeof(*e));
1298     
1299     	assert(strlen(b->name) <= sizeof(e->name));
>>>     CID 1361253:  Memory - illegal accesses  (BUFFER_SIZE_WARNING)
>>>     Calling strncpy with a maximum size argument of 12 bytes on destination array "e->name" of size 12 bytes might leave the destination string unterminated.
1300     	strncpy((char *)e->name, (char *)b->name, sizeof(e->name));
1301     	e->offset = offset;
1302     	e->length = buffer_size(b);
1303     
1304     	return (offset + buffer_size(b));
1305     }

** 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: 341 in integrate_psp_firmwares()


________________________________________________________________________________________________________
*** CID 1353028:  Error handling issues  (NEGATIVE_RETURNS)
/util/amdfwtool/amdfwtool.c: 341 in integrate_psp_firmwares()
335     			pspdir[4+4*i+2] = 1;
336     			pspdir[4+4*i+3] = 0;
337     		} else if (fw_table[i].filename != NULL) {
338     			pspdir[4+4*i+0] = fw_table[i].type;
339     
340     			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.]
341     			fstat(fd, &fd_stat);
342     			pspdir[4+4*i+1] = (uint32_t)fd_stat.st_size;
343     
344     			pspdir[4+4*i+2] = pos + rom_base_address;
345     			pspdir[4+4*i+3] = 0;
346     

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


________________________________________________________________________________________________________
*** CID 1353027:  Error handling issues  (NEGATIVE_RETURNS)
/util/amdfwtool/amdfwtool.c: 284 in integrate_firmwares()
278     	int i;
279     	uint32_t rom_base_address = 0xFFFFFFFF - rom_size + 1;
280     
281     	for (i = 0; fw_table[i].type != AMD_FW_INVALID; i++) {
282     		if (fw_table[i].filename != NULL) {
283     			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.]
284     			fstat(fd, &fd_stat);
285     
286     			switch (fw_table[i].type) {
287     			case AMD_FW_IMC:
288     				pos = ALIGN(pos, 0x10000U);
289     				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 36 out of 45 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: 355 in integrate_psp_firmwares()


________________________________________________________________________________________________________
*** CID 1353021:  Error handling issues  (CHECKED_RETURN)
/util/amdfwtool/amdfwtool.c: 355 in integrate_psp_firmwares()
349     					" will not fit %s.  Exiting.\n",
350     					rom_size, fw_table[i].filename);
351     				free(base);
352     				exit(1);
353     			}
354     
>>>     CID 1353021:  Error handling issues  (CHECKED_RETURN)
>>>     "read(int, void *, size_t)" returns the number of bytes read, but it is ignored.
355     			read(fd, (void *)(base + pos), (size_t)fd_stat.st_size);
356     
357     			pos += fd_stat.st_size;
358     			close(fd);
359     			pos = ALIGN(pos, 0x100U);
360     		} else {

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


________________________________________________________________________________________________________
*** CID 1353020:  Error handling issues  (CHECKED_RETURN)
/util/amdfwtool/amdfwtool.c: 341 in integrate_psp_firmwares()
335     			pspdir[4+4*i+2] = 1;
336     			pspdir[4+4*i+3] = 0;
337     		} else if (fw_table[i].filename != NULL) {
338     			pspdir[4+4*i+0] = fw_table[i].type;
339     
340     			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.]
341     			fstat(fd, &fd_stat);
342     			pspdir[4+4*i+1] = (uint32_t)fd_stat.st_size;
343     
344     			pspdir[4+4*i+2] = pos + rom_base_address;
345     			pspdir[4+4*i+3] = 0;
346     

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


________________________________________________________________________________________________________
*** CID 1353019:  Error handling issues  (CHECKED_RETURN)
/util/amdfwtool/amdfwtool.c: 310 in integrate_firmwares()
304     					" will not fit %s.  Exiting.\n",
305     					rom_size, fw_table[i].filename);
306     				free(base);
307     				exit(1);
308     			}
309     
>>>     CID 1353019:  Error handling issues  (CHECKED_RETURN)
>>>     "read(int, void *, size_t)" returns the number of bytes read, but it is ignored.
310     			read(fd, (void *)(base + pos), (size_t)fd_stat.st_size);
311     
312     			pos += fd_stat.st_size;
313     			close(fd);
314     			pos = ALIGN(pos, 0x100U);
315     		}

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


________________________________________________________________________________________________________
*** CID 1353018:  Error handling issues  (CHECKED_RETURN)
/util/amdfwtool/amdfwtool.c: 284 in integrate_firmwares()
278     	int i;
279     	uint32_t rom_base_address = 0xFFFFFFFF - rom_size + 1;
280     
281     	for (i = 0; fw_table[i].type != AMD_FW_INVALID; i++) {
282     		if (fw_table[i].filename != NULL) {
283     			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.]
284     			fstat(fd, &fd_stat);
285     
286     			switch (fw_table[i].type) {
287     			case AMD_FW_IMC:
288     				pos = ALIGN(pos, 0x10000U);
289     				romsig[1] = pos + rom_base_address;

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


________________________________________________________________________________________________________
*** CID 1325840:  Memory - illegal accesses  (OVERRUN)
/util/cbfstool/cbfs_image.c: 1406 in cbfs_print_entry_info()
1400     	while ((hash = cbfs_file_get_next_hash(entry, hash)) != NULL) {
1401     		unsigned int hash_type = ntohl(hash->hash_type);
1402     		if (hash_type > CBFS_NUM_SUPPORTED_HASHES) {
1403     			fprintf(fp, "invalid hash type %d\n", hash_type);
1404     			break;
1405     		}
>>>     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).
1406     		size_t hash_len = widths_cbfs_hash[hash_type];
1407     		char *hash_str = bintohex(hash->hash_data, hash_len);
1408     		uint8_t local_hash[hash_len];
1409     		if (vb2_digest_buffer(CBFS_SUBHEADER(entry),
1410     			ntohl(entry->len), hash_type, local_hash,
1411     			hash_len) != VB2_SUCCESS) {

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


________________________________________________________________________________________________________
*** CID 1325836:  Resource leaks  (RESOURCE_LEAK)
/util/cbfstool/cbfs_image.c: 1413 in cbfs_print_entry_info()
1407     		char *hash_str = bintohex(hash->hash_data, hash_len);
1408     		uint8_t local_hash[hash_len];
1409     		if (vb2_digest_buffer(CBFS_SUBHEADER(entry),
1410     			ntohl(entry->len), hash_type, local_hash,
1411     			hash_len) != VB2_SUCCESS) {
1412     			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.
1413     			break;
1414     		}
1415     		int valid = memcmp(local_hash, hash->hash_data, hash_len) == 0;
1416     		const char *valid_str = valid ? "valid" : "invalid";
1417     
1418     		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://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V05UPxvVjWch-2Bd2MGckcRbLuoVetFLSjdonCi1EjfHRqWGQvojmmkYaBE-2BPJiTQvQ-3D-3D_q4bX76XMySz3BXBlWr5fXXJ4cvAsgEXEqC7dBPM7O5ZBKQPjLAs4fVKY6aHX2cyjJGGN2rKtDOoeFQFVv6St42gq8e-2Fc4KVFZnh9BtAalVwgbx4XC8NKNfMdE00-2BMF1gYJRpwjQdMWfBfeGls9ix6eL4VzQ3BHp9LwsJqhAOw6fHsmjC4ek-2FifR7pmyAWoGaELMES0wLiFBlBfNNQw6m2q3dD5p6tAQaivTLuIprVJs-3D

To manage Coverity Scan email notifications for "coreboot at coreboot.org", click https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V05UPxvVjWch-2Bd2MGckcRbVDbis712qZDP-2FA8y06Nq4e-2BpBzwOa5gzBZa9dWpDbzfofODnVj1enK2UkK0-2BgCCqyeem8IVKvTxSaOFkteZFcnohwvb2rnYNjswGryEWCURnUk6WHU42sbOmtOjD-2Bx5c-3D_q4bX76XMySz3BXBlWr5fXXJ4cvAsgEXEqC7dBPM7O5ZBKQPjLAs4fVKY6aHX2cyjx0Zwpcs2gfpOf8q-2F4Y4JEkv1GWwU6ypqizIOrsvzIH3yHeJqfYRfNC2anTiVXIlUWhWa0KbeOT0IJrGoczMAYgJ65uOKTGJzncvITKht1j3e79zb6ljoOLRx6b7ScygzfKA3QOMpQSukivMrNqIE1TXRHaSKVhSEKGRoBXOKn-2Fg-3D




More information about the coreboot mailing list