Hello,
We run Coverity on the Xen source code occasionally and it happens to include SeaBIOS. The following new warnings have appeared since I pulled in rel-1.7.5.
At least the MISSING_BREAK ones look likely to be valid to me. Not sure about the other two...
Ian.
-------- Forwarded Message -------- From: scan-admin@coverity.com Subject: New Defects reported by Coverity Scan for XenProject Date: Wed, 16 Jul 2014 07:16:19 -0700 Message-id: 53c68933a9a35_6cda4073389495a@209.249.196.67.mail
Hi,
Please find the latest report on new defect(s) introduced to XenProject found with Coverity Scan.
Defect(s) Reported-by: Coverity Scan Showing 5 of 5 defect(s)
** CID 1226281: Unchecked return value (CHECKED_RETURN) /tools/firmware/seabios-dir-remote/src/fw/smbios.c: 578 in smbios_legacy_setup()
** CID 1226282: Missing break in switch (MISSING_BREAK) /tools/firmware/seabios-dir-remote/src/hw/blockcmd.c: 49 in cdb_cmd_data()
** CID 1226283: Missing break in switch (MISSING_BREAK) /tools/firmware/seabios-dir-remote/src/hw/blockcmd.c: 52 in cdb_cmd_data()
** CID 1226284: Missing break in switch (MISSING_BREAK) /tools/firmware/seabios-dir-remote/src/hw/blockcmd.c: 55 in cdb_cmd_data()
** CID 1226285: Unintentional integer overflow (OVERFLOW_BEFORE_WIDEN) /tools/firmware/seabios-dir-remote/src/hw/usb-xhci.c: 698 in configure_xhci()
________________________________________________________________________________________________________ *** CID 1226281: Unchecked return value (CHECKED_RETURN) /tools/firmware/seabios-dir-remote/src/fw/smbios.c: 578 in smbios_legacy_setup() 572 } 573 } 574 575 add_struct(32, p); 576 /* Add any remaining provided entries before the end marker */ 577 for (i = 0; i < 256; i++)
CID 1226281: Unchecked return value (CHECKED_RETURN) Calling "get_external" without checking return value (as is done elsewhere 12 out of 13 times).
578 get_external(i, &p, &nr_structs, &max_struct_size, end); 579 add_struct(127, p); 580 581 #undef add_struct 582 583 smbios_entry_point_setup(max_struct_size, p - start, start, nr_structs); 584 free(start);
________________________________________________________________________________________________________ *** CID 1226282: Missing break in switch (MISSING_BREAK) /tools/firmware/seabios-dir-remote/src/hw/blockcmd.c: 49 in cdb_cmd_data() 43 return esp_scsi_cmd_data(op, cdbcmd, blocksize); 44 case DTYPE_MEGASAS: 45 return megasas_cmd_data(op, cdbcmd, blocksize); 46 case DTYPE_USB_32: 47 if (!MODESEGMENT) 48 return usb_cmd_data(op, cdbcmd, blocksize);
CID 1226282: Missing break in switch (MISSING_BREAK) The above case falls through to this one.
49 case DTYPE_UAS_32: 50 if (!MODESEGMENT) 51 return uas_cmd_data(op, cdbcmd, blocksize); 52 case DTYPE_PVSCSI: 53 if (!MODESEGMENT) 54 return pvscsi_cmd_data(op, cdbcmd, blocksize);
________________________________________________________________________________________________________ *** CID 1226283: Missing break in switch (MISSING_BREAK) /tools/firmware/seabios-dir-remote/src/hw/blockcmd.c: 52 in cdb_cmd_data() 46 case DTYPE_USB_32: 47 if (!MODESEGMENT) 48 return usb_cmd_data(op, cdbcmd, blocksize); 49 case DTYPE_UAS_32: 50 if (!MODESEGMENT) 51 return uas_cmd_data(op, cdbcmd, blocksize);
CID 1226283: Missing break in switch (MISSING_BREAK) The above case falls through to this one.
52 case DTYPE_PVSCSI: 53 if (!MODESEGMENT) 54 return pvscsi_cmd_data(op, cdbcmd, blocksize); 55 case DTYPE_AHCI_ATAPI: 56 if (!MODESEGMENT) 57 return ahci_cmd_data(op, cdbcmd, blocksize);
________________________________________________________________________________________________________ *** CID 1226284: Missing break in switch (MISSING_BREAK) /tools/firmware/seabios-dir-remote/src/hw/blockcmd.c: 55 in cdb_cmd_data() 49 case DTYPE_UAS_32: 50 if (!MODESEGMENT) 51 return uas_cmd_data(op, cdbcmd, blocksize); 52 case DTYPE_PVSCSI: 53 if (!MODESEGMENT) 54 return pvscsi_cmd_data(op, cdbcmd, blocksize);
CID 1226284: Missing break in switch (MISSING_BREAK) The above case falls through to this one.
55 case DTYPE_AHCI_ATAPI: 56 if (!MODESEGMENT) 57 return ahci_cmd_data(op, cdbcmd, blocksize); 58 default: 59 return DISK_RET_EPARAM; 60 }
________________________________________________________________________________________________________ *** CID 1226285: Unintentional integer overflow (OVERFLOW_BEFORE_WIDEN) /tools/firmware/seabios-dir-remote/src/hw/usb-xhci.c: 698 in configure_xhci() 692 free(spba); 693 free(pad); 694 goto fail; 695 } 696 int i; 697 for (i = 0; i < spb; i++)
CID 1226285: Unintentional integer overflow (OVERFLOW_BEFORE_WIDEN) Potentially overflowing expression "i * 4096" with type "int" (32 bits, signed) is evaluated using 32-bit arithmetic before being used in a context which expects an expression of type "u64" (64 bits, unsigned). To avoid overflow, cast either operand to "u64" before performing the multiplication.
698 spba[i] = (u32)pad + (i * PAGE_SIZE); 699 xhci->devs[0].ptr_low = (u32)spba; 700 xhci->devs[0].ptr_high = 0; 701 } 702 703 reg = readl(&xhci->op->usbcmd);
________________________________________________________________________________________________________ To view the defects in Coverity Scan visit, http://scan.coverity.com/projects/606?tab=overview
To unsubscribe from the email notification for new defects, http://scan5.coverity.com/cgi-bin/unsubscribe.py
On Wed, Jul 16, 2014 at 04:27:18PM +0100, Ian Campbell wrote:
Hello,
We run Coverity on the Xen source code occasionally and it happens to include SeaBIOS. The following new warnings have appeared since I pulled in rel-1.7.5.
Thanks. All five look like false positives to me. I'm happy to take patches if you want to rework the code to prevent the warnings.
At least the MISSING_BREAK ones look likely to be valid to me. Not sure about the other two...
It's a bit ugly, but it should be okay because all three cases start with "if (!MODESEGMENT)" which is a compile time constant. So, when compiled in 32bit mode the three cases will each return the results from their respective functions, and in 16bit mode all three will return DISK_RET_EPARAM.
-Kevin
On Wed, 2014-07-16 at 13:33 -0400, Kevin O'Connor wrote:
On Wed, Jul 16, 2014 at 04:27:18PM +0100, Ian Campbell wrote:
Hello,
We run Coverity on the Xen source code occasionally and it happens to include SeaBIOS. The following new warnings have appeared since I pulled in rel-1.7.5.
Thanks. All five look like false positives to me.
OK, we will tag them as such for our runs. Thanks.
I'm happy to take patches if you want to rework the code to prevent the warnings.
At least the MISSING_BREAK ones look likely to be valid to me. Not sure about the other two...
It's a bit ugly, but it should be okay because all three cases start with "if (!MODESEGMENT)" which is a compile time constant. So, when compiled in 32bit mode the three cases will each return the results from their respective functions, and in 16bit mode all three will return DISK_RET_EPARAM.
Subtle!
"Kevin O'Connor" kevin@koconnor.net writes:
On Wed, Jul 16, 2014 at 04:27:18PM +0100, Ian Campbell wrote:
Hello,
We run Coverity on the Xen source code occasionally and it happens to include SeaBIOS. The following new warnings have appeared since I pulled in rel-1.7.5.
Thanks. All five look like false positives to me. I'm happy to take patches if you want to rework the code to prevent the warnings.
At least the MISSING_BREAK ones look likely to be valid to me. Not sure about the other two...
It's a bit ugly, but it should be okay because all three cases start with "if (!MODESEGMENT)" which is a compile time constant. So, when compiled in 32bit mode the three cases will each return the results from their respective functions, and in 16bit mode all three will return DISK_RET_EPARAM.
A comment would make your intention clearn and shut up Coverity:
case DTYPE_USB_32: if (!MODESEGMENT) return usb_cmd_data(op, cdbcmd, blocksize); /* fall through */ case DTYPE_UAS_32: if (!MODESEGMENT) return uas_cmd_data(op, cdbcmd, blocksize);