From 624533e5a53e5df347e46f06408d15b9df5254f0 Mon Sep 17 00:00:00 2001 From: John Snow <jsnow@redhat.com> Date: Wed, 25 Nov 2015 16:03:37 -0500 Subject: [PATCH 1/9] util/id: fully allocate names table Trivial: this array should be allocated to have ID_MAX entries always. Otherwise if someone were to forget to expand this table, the assertion in the id generator won't actually trigger; it will read junk data. Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Jeff Cody <jcody@redhat.com> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> --- util/id.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/util/id.c b/util/id.c index bcc64d836f..7883fbec79 100644 --- a/util/id.c +++ b/util/id.c @@ -29,7 +29,7 @@ bool id_wellformed(const char *id) #define ID_SPECIAL_CHAR '#' -static const char *const id_subsys_str[] = { +static const char *const id_subsys_str[ID_MAX] = { [ID_QDEV] = "qdev", [ID_BLOCK] = "block", }; @@ -53,7 +53,7 @@ char *id_generate(IdSubSystems id) static uint64_t id_counters[ID_MAX]; uint32_t rnd; - assert(id < ID_MAX); + assert(id < ARRAY_SIZE(id_subsys_str)); assert(id_subsys_str[id]); rnd = g_random_int_range(0, 100); From e0df8f18f755d8c976db9bca2faabb763ad98ff2 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini <pbonzini@redhat.com> Date: Fri, 27 Nov 2015 13:08:25 +0100 Subject: [PATCH 2/9] bt: avoid unintended sign extension In the case of a 4-byte length, shifting a value by 24 may cause an unintended sign extension when converting from int to size_t. Use a uint32_t variable instead. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> --- hw/bt/sdp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/bt/sdp.c b/hw/bt/sdp.c index b9bcdcc78d..04eaecae67 100644 --- a/hw/bt/sdp.c +++ b/hw/bt/sdp.c @@ -42,7 +42,7 @@ struct bt_l2cap_sdp_state_s { static ssize_t sdp_datalen(const uint8_t **element, ssize_t *left) { - size_t len = *(*element) ++ & SDP_DSIZE_MASK; + uint32_t len = *(*element) ++ & SDP_DSIZE_MASK; if (!*left) return -1; From 0ef74c7496fd3c526b2259f86326eca4b3a03b78 Mon Sep 17 00:00:00 2001 From: Peter Maydell <peter.maydell@linaro.org> Date: Tue, 24 Nov 2015 14:55:46 +0000 Subject: [PATCH 3/9] configure: Diagnose broken linkers directly Currently if the user's compiler works for creating .o files but their linker is broken such that compiling an executable from a C file does not work, we will report a misleading error message about the compiler not supporting __thread (since that happens to be the first test we run which requires a working linker). Explicitly check that compile_prog works as well as compile_object, so that people whose toolchain setup is broken get a more helpful error message. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> --- configure | 3 +++ 1 file changed, 3 insertions(+) diff --git a/configure b/configure index 67801b06bb..2e8a672aeb 100755 --- a/configure +++ b/configure @@ -1428,6 +1428,9 @@ if compile_object ; then else error_exit "\"$cc\" either does not exist or does not work" fi +if ! compile_prog ; then + error_exit "\"$cc\" cannot build an executable (is your linker broken?)" +fi # Check that the C++ compiler exists and works with the C compiler if has $cxx; then From 0e1d02452bf2c3486406dd48524a5b1de3c0eba8 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" <berrange@redhat.com> Date: Fri, 13 Nov 2015 17:45:27 +0000 Subject: [PATCH 4/9] crypto: avoid two coverity false positive error reports In qcrypto_tls_creds_get_path() coverity complains that we are checking '*creds' for NULL, despite having dereferenced it previously. This is harmless bug due to fact that the trace call was too early. Moving it after the cleanup gets the desired semantics. In qcrypto_tls_creds_check_cert_key_purpose() coverity complains that we're passing a pointer to a previously free'd buffer into gnutls_x509_crt_get_key_purpose_oid() This is harmless because we're passing a size == 0, so gnutls won't access the buffer, but rather just report what size it needs to be. We can avoid it though by explicitly setting the buffer to NULL after free'ing it. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> --- crypto/tlscreds.c | 4 ++-- crypto/tlscredsx509.c | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/crypto/tlscreds.c b/crypto/tlscreds.c index 5ec982c6ee..e7d9c1cfac 100644 --- a/crypto/tlscreds.c +++ b/crypto/tlscreds.c @@ -123,10 +123,10 @@ qcrypto_tls_creds_get_path(QCryptoTLSCreds *creds, goto cleanup; } - trace_qcrypto_tls_creds_get_path(creds, filename, - *cred ? *cred : "<none>"); ret = 0; cleanup: + trace_qcrypto_tls_creds_get_path(creds, filename, + *cred ? *cred : "<none>"); return ret; } diff --git a/crypto/tlscredsx509.c b/crypto/tlscredsx509.c index d080deb83e..26f18cbb4a 100644 --- a/crypto/tlscredsx509.c +++ b/crypto/tlscredsx509.c @@ -255,6 +255,7 @@ qcrypto_tls_creds_check_cert_key_purpose(QCryptoTLSCredsX509 *creds, } g_free(buffer); + buffer = NULL; } if (isServer) { From fccd35a04640a728f979e6d72b2c7d02c05549f0 Mon Sep 17 00:00:00 2001 From: Rodrigo Rebello <rprebello@gmail.com> Date: Thu, 12 Nov 2015 12:04:28 -0200 Subject: [PATCH 5/9] configure: use appropriate code fragment for -fstack-protector checks The check for stack-protector support consisted in compiling and linking the test program below (output by function write_c_skeleton()) with the compiler flag -fstack-protector-strong first and then with -fstack-protector-all if the first one failed to work: int main(void) { return 0; } This caused false positives when using certain toolchains in which the compiler accepted -fstack-protector-strong but no support was provided by the C library, since for this stack-protector variant the compiler emits canary code only for functions that meet specific conditions (local arrays, memory references to local variables, etc.) and the code fragment under test included none of them (hence no stack protection code generated, no link failure). This fix changes the test program used for -fstack-protector checks to include a function that meets conditions which cause the compiler to generate canary code in all variants. Signed-off-by: Rodrigo Rebello <rprebello@gmail.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> --- configure | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/configure b/configure index 2e8a672aeb..b9552fda6f 100755 --- a/configure +++ b/configure @@ -1491,6 +1491,16 @@ for flag in $gcc_flags; do done if test "$stack_protector" != "no"; then + cat > $TMPC << EOF +int main(int argc, char *argv[]) +{ + char arr[64], *p = arr, *c = argv[0]; + while (*c) { + *p++ = *c++; + } + return 0; +} +EOF gcc_flags="-fstack-protector-strong -fstack-protector-all" sp_on=0 for flag in $gcc_flags; do From 63fc7375d6b0a083aef56950119af3d1d2a96e51 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini <pbonzini@redhat.com> Date: Fri, 6 Nov 2015 16:34:06 +0100 Subject: [PATCH 6/9] gt64xxx: fix decoding of ISD register The GT64xxx's internal registers can be placed above the first 4 GiB in the address space, but not above the first 64 GiB. Correctly cast the register to a 64-bit integer, and mask away bits above bit 35. Datasheet at http://pdf.datasheetarchive.com/datasheetsmain/Datasheets-33/DSA-655889.pdf (bug reported by Coverity). Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> --- hw/mips/gt64xxx_pci.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/hw/mips/gt64xxx_pci.c b/hw/mips/gt64xxx_pci.c index 10fcca33f8..f76a9fd36b 100644 --- a/hw/mips/gt64xxx_pci.c +++ b/hw/mips/gt64xxx_pci.c @@ -275,7 +275,8 @@ static void check_reserved_space (hwaddr *start, static void gt64120_isd_mapping(GT64120State *s) { - hwaddr start = s->regs[GT_ISD] << 21; + /* Bits 14:0 of ISD map to bits 35:21 of the start address. */ + hwaddr start = ((hwaddr)s->regs[GT_ISD] << 21) & 0xFFFE00000ull; hwaddr length = 0x1000; if (s->ISD_length) { From 8ea9900330404eff0ea8812e2d96d1d5cbfb2c24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Herv=C3=A9=20Poussineau?= <hpoussin@reactos.org> Date: Thu, 12 Nov 2015 22:26:33 +0100 Subject: [PATCH 7/9] scsi: remove scsi_req_free prototype MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Function has been deleted in ad2d30f79d3b0812f02c741be2189796b788d6d7. Signed-off-by: Hervé Poussineau <hpoussin@reactos.org> Reviewed-by: Fam Zheng <famz@redhat.com> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> --- include/hw/scsi/scsi.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/hw/scsi/scsi.h b/include/hw/scsi/scsi.h index cdaf0f8eb7..1915a7342e 100644 --- a/include/hw/scsi/scsi.h +++ b/include/hw/scsi/scsi.h @@ -250,7 +250,6 @@ SCSIRequest *scsi_req_alloc(const SCSIReqOps *reqops, SCSIDevice *d, SCSIRequest *scsi_req_new(SCSIDevice *d, uint32_t tag, uint32_t lun, uint8_t *buf, void *hba_private); int32_t scsi_req_enqueue(SCSIRequest *req); -void scsi_req_free(SCSIRequest *req); SCSIRequest *scsi_req_ref(SCSIRequest *req); void scsi_req_unref(SCSIRequest *req); From 2988cbeaf94203b2cf31c0b3f589aa4ebc0cff34 Mon Sep 17 00:00:00 2001 From: Markus Armbruster <armbru@redhat.com> Date: Thu, 19 Nov 2015 13:29:28 +0100 Subject: [PATCH 8/9] typedefs: Put them back into alphabetical order "Please keep this list in alphabetical order" has been more honoured in the breach than in the observance. Clean up. While there, drop a redundant struct declaration. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> --- include/qemu/typedefs.h | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h index 6b1093dcfc..3eedcf4c8f 100644 --- a/include/qemu/typedefs.h +++ b/include/qemu/typedefs.h @@ -3,7 +3,6 @@ /* A load of opaque types so that device init declarations don't have to pull in all the real definitions. */ -struct Monitor; /* Please keep this list in alphabetical order */ typedef struct AdapterInfo AdapterInfo; @@ -19,8 +18,8 @@ typedef struct BusState BusState; typedef struct CharDriverState CharDriverState; typedef struct CompatProperty CompatProperty; typedef struct CPUAddressSpace CPUAddressSpace; -typedef struct DeviceState DeviceState; typedef struct DeviceListener DeviceListener; +typedef struct DeviceState DeviceState; typedef struct DisplayChangeListener DisplayChangeListener; typedef struct DisplayState DisplayState; typedef struct DisplaySurface DisplaySurface; @@ -46,6 +45,7 @@ typedef struct MigrationIncomingState MigrationIncomingState; typedef struct MigrationParams MigrationParams; typedef struct MigrationState MigrationState; typedef struct Monitor Monitor; +typedef struct MonitorDef MonitorDef; typedef struct MouseTransformInfo MouseTransformInfo; typedef struct MSIMessage MSIMessage; typedef struct NetClientState NetClientState; @@ -63,13 +63,13 @@ typedef struct PCIESlot PCIESlot; typedef struct PCIExpressDevice PCIExpressDevice; typedef struct PCIExpressHost PCIExpressHost; typedef struct PCIHostState PCIHostState; -typedef struct PCMachineState PCMachineState; typedef struct PCMachineClass PCMachineClass; +typedef struct PCMachineState PCMachineState; typedef struct PCMCIACardState PCMCIACardState; typedef struct PixelFormat PixelFormat; typedef struct PostcopyDiscardState PostcopyDiscardState; -typedef struct PropertyInfo PropertyInfo; typedef struct Property Property; +typedef struct PropertyInfo PropertyInfo; typedef struct QEMUBH QEMUBH; typedef struct QemuConsole QemuConsole; typedef struct QEMUFile QEMUFile; @@ -78,10 +78,10 @@ typedef struct QemuOpts QemuOpts; typedef struct QemuOptsList QemuOptsList; typedef struct QEMUSGList QEMUSGList; typedef struct QEMUSizedBuffer QEMUSizedBuffer; -typedef struct QEMUTimerListGroup QEMUTimerListGroup; typedef struct QEMUTimer QEMUTimer; -typedef struct Range Range; +typedef struct QEMUTimerListGroup QEMUTimerListGroup; typedef struct RAMBlock RAMBlock; +typedef struct Range Range; typedef struct SerialState SerialState; typedef struct SHPCDevice SHPCDevice; typedef struct SMBusDevice SMBusDevice; @@ -89,6 +89,5 @@ typedef struct SSIBus SSIBus; typedef struct uWireSlave uWireSlave; typedef struct VirtIODevice VirtIODevice; typedef struct Visitor Visitor; -typedef struct MonitorDef MonitorDef; #endif /* QEMU_TYPEDEFS_H */ From 98475746b357f6c048caf9e001998d8a0618b2e4 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini <pbonzini@redhat.com> Date: Mon, 30 Nov 2015 10:57:25 +0100 Subject: [PATCH 9/9] bt: check struct sizes See http://permalink.gmane.org/gmane.linux.bluez.kernel/36505. For historical reasons these do not use sizeof, and Coverity caught a mistake in EVT_ENCRYPT_CHANGE_SIZE. In addition: - remove status from create_conn_cancel_cp; the "status" field is only in rp structs. Note that this means that the OCF_CREATE_CONN_CANCEL could never have worked (it would have failed the LENGTH_CHECK), but I am keeping it anyway. - OCF_READ_LINK_QUALITY similarly could never have worked, but I am fixing read_link_quality_cp anyway. - fix inquiry_info which is shorter by one: the kernel has a struct that is 14 byte long, but not counting the initial num_responses byte which the kernel parses separately; - remove extended_inquiry_info altogether, since it's not used and unlike the other inquiry structs does not have the initial num_responses byte. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> --- include/hw/bt.h | 21 ++++----------------- 1 file changed, 4 insertions(+), 17 deletions(-) diff --git a/include/hw/bt.h b/include/hw/bt.h index cb2a7e6579..c7c7909a37 100644 --- a/include/hw/bt.h +++ b/include/hw/bt.h @@ -504,7 +504,6 @@ typedef struct { #define OCF_CREATE_CONN_CANCEL 0x0008 typedef struct { - uint8_t status; bdaddr_t bdaddr; } QEMU_PACKED create_conn_cancel_cp; #define CREATE_CONN_CANCEL_CP_SIZE 6 @@ -1266,13 +1265,13 @@ typedef struct { uint8_t status; uint16_t handle; } QEMU_PACKED reset_failed_contact_counter_rp; -#define RESET_FAILED_CONTACT_COUNTER_RP_SIZE 4 +#define RESET_FAILED_CONTACT_COUNTER_RP_SIZE 3 #define OCF_READ_LINK_QUALITY 0x0003 typedef struct { uint16_t handle; } QEMU_PACKED read_link_quality_cp; -#define READ_LINK_QUALITY_CP_SIZE 4 +#define READ_LINK_QUALITY_CP_SIZE 2 typedef struct { uint8_t status; @@ -1332,7 +1331,7 @@ typedef struct { uint8_t dev_class[3]; uint16_t clock_offset; } QEMU_PACKED inquiry_info; -#define INQUIRY_INFO_SIZE 14 +#define INQUIRY_INFO_SIZE 15 #define EVT_CONN_COMPLETE 0x03 typedef struct { @@ -1381,7 +1380,7 @@ typedef struct { uint16_t handle; uint8_t encrypt; } QEMU_PACKED evt_encrypt_change; -#define EVT_ENCRYPT_CHANGE_SIZE 5 +#define EVT_ENCRYPT_CHANGE_SIZE 4 #define EVT_CHANGE_CONN_LINK_KEY_COMPLETE 0x09 typedef struct { @@ -1629,18 +1628,6 @@ typedef struct { } QEMU_PACKED evt_sniff_subrate; #define EVT_SNIFF_SUBRATE_SIZE 11 -#define EVT_EXTENDED_INQUIRY_RESULT 0x2F -typedef struct { - bdaddr_t bdaddr; - uint8_t pscan_rep_mode; - uint8_t pscan_period_mode; - uint8_t dev_class[3]; - uint16_t clock_offset; - int8_t rssi; - uint8_t data[240]; -} QEMU_PACKED extended_inquiry_info; -#define EXTENDED_INQUIRY_INFO_SIZE 254 - #define EVT_TESTING 0xFE #define EVT_VENDOR 0xFF