New Defects reported by Coverity Scan for coreboot

Hi,

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

7 new defect(s) introduced to coreboot 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 7 of 7 defect(s)

** CID 1349857:  Uninitialized variables  (UNINIT)
/src/soc/intel/skylake/smihandler.c: 355 in southbridge_smi_gpi()

________________________________________________________________________________________________________
*** CID 1349857:  Uninitialized variables  (UNINIT)
/src/soc/intel/skylake/smihandler.c: 355 in southbridge_smi_gpi()
349     
350     void __attribute__((weak))
351     mainboard_smi_gpi_handler(const struct gpi_status *sts) { }
352     
353     static void southbridge_smi_gpi(void)
354     {
> > > CID 1349857:  Uninitialized variables  (UNINIT)
> > > Declaring variable "smi_sts" without initializer.

355     	struct gpi_status smi_sts;
356     
357     	gpi_clear_get_smi_status(&smi_sts);
358     	mainboard_smi_gpi_handler(&smi_sts);
359     
360     	/* Clear again after mainboard handler */

** CID 1349856:  Uninitialized variables  (UNINIT)
/src/mainboard/google/cyan/spd/spd.c: 166 in set_dimm_info()

________________________________________________________________________________________________________
*** CID 1349856:  Uninitialized variables  (UNINIT)
/src/mainboard/google/cyan/spd/spd.c: 166 in set_dimm_info()
160     		break;
161     
162     	case 8:
163     		log2_chips = 3;
164     		break;
165     	}
> > > CID 1349856:  Uninitialized variables  (UNINIT)
> > > Using uninitialized value "log2_chips".

166     	dimm->bus_width = (uint8_t)(log2_chips + (spd[7] & 7) + 2 - 3);
167     }
168     
169     void mainboard_save_dimm_info(struct romstage_params *params)
170     {
171     	struct dimm_info *dimm;

** CID 1349855:  Control flow issues  (UNREACHABLE)
/src/drivers/intel/fsp1_1/fsp_util.c: 44 in find_fsp()

________________________________________________________________________________________________________
*** CID 1349855:  Control flow issues  (UNREACHABLE)
/src/drivers/intel/fsp1_1/fsp_util.c: 44 in find_fsp()
38     	} fsp_id = {
39     		.str_id = CONFIG_FSP_IMAGE_ID_STRING
40     	};
41     
42     	u32 *image_id;
43
> > > CID 1349855:  Control flow issues  (UNREACHABLE)
> > > Since the loop increment is unreachable, the loop body will never execute more than once.

44     	for (;;) {
45     		/* Get the FSP binary base address in CBFS */
46     		fsp_ptr.u32 = fsp_base_address;
47     
48     		/* Check the FV signature, _FVH */
49     		if (fsp_ptr.fvh->Signature != 0x4856465F) {

** CID 1349854:  Incorrect expression  (UNUSED_VALUE)
/src/drivers/intel/fsp1_1/fsp_util.c: 56 in find_fsp()

________________________________________________________________________________________________________
*** CID 1349854:  Incorrect expression  (UNUSED_VALUE)
/src/drivers/intel/fsp1_1/fsp_util.c: 56 in find_fsp()
50     			fsp_ptr.u8 = (u8 *)ERROR_NO_FV_SIG;
51     			break;
52     		}
53     
54     		/* Locate the file header which follows the FV header. */
55     		fsp_ptr.u8 += fsp_ptr.fvh->ExtHeaderOffset;
> > > CID 1349854:  Incorrect expression  (UNUSED_VALUE)
> > > Assigning value from "fsp_ptr.fveh->ExtHeaderSize" to "fsp_ptr.u8" here, but that stored value is overwritten before it can be used.

56     		fsp_ptr.u8 += fsp_ptr.fveh->ExtHeaderSize;
57     		fsp_ptr.u8 = (u8 *)ALIGN_UP(fsp_ptr.u32, 8);
58     
59     		/* Check the FFS GUID */
60     		if ((((u32 *)&fsp_ptr.ffh->Name)[0] != 0x912740BE)
61     			|| (((u32 *)&fsp_ptr.ffh->Name)[1] != 0x47342284)

** CID 1349860:  Memory - corruptions  (ARRAY_VS_SINGLETON)
/src/soc/intel/skylake/romstage/spi.c: 34 in early_spi_read_wpsr()

________________________________________________________________________________________________________
*** CID 1349860:  Memory - corruptions  (ARRAY_VS_SINGLETON)
/src/soc/intel/skylake/romstage/spi.c: 34 in early_spi_read_wpsr()
28     
29     	spi_init();
30     
31     	/* sending NULL for spiflash struct parameter since we are not
32     	 * calling HWSEQ read_status() call via Probe.
33     	 */
> > > CID 1349860:  Memory - corruptions  (ARRAY_VS_SINGLETON)
> > > Taking address with "&rdsr" yields a singleton pointer.

34     	ret = pch_hwseq_read_status(NULL, &rdsr);
35     	if (ret) {
36     		printk(BIOS_ERR, "SPI rdsr failed\n");
37     		return ret;
38     	}
39     	*sr = rdsr & WPSR_MASK_SRP0_BIT;
40     
41     	return 0;

** CID 1349859:  Control flow issues  (DEADCODE)
/src/soc/intel/braswell/spi.c: 258 in spi_regs()

________________________________________________________________________________________________________
*** CID 1349859:  Control flow issues  (DEADCODE)
/src/soc/intel/braswell/spi.c: 258 in spi_regs()
252     #if ENV_SMM
253     	dev = PCI_DEV(0, LPC_DEV, LPC_FUNC);
254     #else
255     	dev = dev_find_slot(0, PCI_DEVFN(LPC_DEV, LPC_FUNC));
256     #endif
257     	if (!dev) {
> > > CID 1349859:  Control flow issues  (DEADCODE)
> > > Execution cannot reach this statement: "printk(3, "%s: PCI device n...".

258     		printk(BIOS_ERR, "%s: PCI device not found", __func__);
259     		return NULL;
260     	}
261     
262     	pci_read_config_dword(dev, SBASE, &sbase);
263     	sbase &= ~0x1ff;

** CID 1349858:  Memory - illegal accesses  (OVERRUN)
/src/soc/mediatek/mt8173/spi.c: 85 in mtk_spi_init()

________________________________________________________________________________________________________
*** CID 1349858:  Memory - illegal accesses  (OVERRUN)
/src/soc/mediatek/mt8173/spi.c: 85 in mtk_spi_init()
79     		  unsigned int speed_hz)
80     {
81     	u32 div, sck_ticks, cs_ticks, reg_val;
82     	/* mtk spi HW just support bus 0 */
83     	assert(bus == 0);
84     	struct mtk_spi_bus *slave = &spi_bus[bus];
> > > CID 1349858:  Memory - illegal accesses  (OVERRUN)
> > > Overrunning array of 1 48-byte elements at element index 1 (byte offset 48) by dereferencing pointer "slave".

85     	struct mtk_spi_regs *regs = slave->regs;
86     
87     	if (speed_hz < SPI_HZ / 2)
88     		div = div_round_up(SPI_HZ, speed_hz);
89     	else
90     		div = 1;

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

To manage Coverity Scan email notifications for "[e-mail]", click https://scan.coverity.com/subscriptions/edit?email=coreboot%40coreboot.org&a...

[Julius Werner]

I'm curious about this part:
> *** CID 1349858:  Memory - illegal accesses  (OVERRUN)
> /src/soc/mediatek/mt8173/spi.c: 85 in mtk_spi_init()
> 79                unsigned int speed_hz)
> 80     {
> 81      u32 div, sck_ticks, cs_ticks, reg_val;
> 82      /* mtk spi HW just support bus 0 */
> 83      assert(bus == 0);
> 84      struct mtk_spi_bus *slave = &spi_bus[bus];
> > > > CID 1349858:  Memory - illegal accesses  (OVERRUN)
> > > > Overrunning array of 1 48-byte elements at element index 1 (byte offset 48) by dereferencing pointer "slave".
>
> 85      struct mtk_spi_regs *regs = slave->regs;
> 86
> 87      if (speed_hz < SPI_HZ / 2)
> 88              div = div_round_up(SPI_HZ, speed_hz);
> 89      else
> 90              div = 1;

Clearly this seems to be a false positive since the 'spi_bus' array
has one element (as coverity seems to acknowledge itself) and we even
assert that the index 'bus' will always be 0 (although this assertion
may or may not actually generate code based on Kconfigs). Does
coverity just generally flag any function where you can pass in an
index that is then used to index a static array? That sounds a little
overzealous considering how it is otherwise often quite accurate...