From 3daa41078aedf227ec98b0d1c9d56b77b6d20153 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 18 Dec 2015 09:54:53 +0100 Subject: scsi: revert change to scsi_req_cancel_async and add assertions Fam Zheng noticed that the change in commit 36896bf ("scsi: always call notifier on async cancellation", 2015-12-16) could cause a leak of the request; scsi_req_cancel_async now calls scsi_req_ref multiple times for multiple cancellations, but there is only one call to scsi_req_cancel_complete. So revert the patch and instead assert that the problematic case (a call to scsi_req_cancel_async after the aiocb has been completed) cannot happen. Signed-off-by: Paolo Bonzini --- hw/scsi/scsi-bus.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'hw') diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index 00bddc9..378bf4d 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -1759,6 +1759,15 @@ void scsi_req_cancel_async(SCSIRequest *req, Notifier *notifier) if (notifier) { notifier_list_add(&req->cancel_notifiers, notifier); } + if (req->io_canceled) { + /* A blk_aio_cancel_async is pending; when it finishes, + * scsi_req_cancel_complete will be called and will + * call the notifier we just added. Just wait for that. + */ + assert(req->aiocb); + return; + } + /* Dropped in scsi_req_cancel_complete. */ scsi_req_ref(req); scsi_req_dequeue(req); req->io_canceled = true; @@ -1775,6 +1784,8 @@ void scsi_req_cancel(SCSIRequest *req) if (!req->enqueued) { return; } + assert(!req->io_canceled); + /* Dropped in scsi_req_cancel_complete. */ scsi_req_ref(req); scsi_req_dequeue(req); req->io_canceled = true; -- cgit v1.1 From 4c1396cb576c9b14425558b73de1584c7a9735d7 Mon Sep 17 00:00:00 2001 From: P J P Date: Fri, 18 Dec 2015 11:35:07 +0530 Subject: i386: avoid null pointer dereference Hello, A null pointer dereference issue was reported by Mr Ling Liu, CC'd here. It occurs while doing I/O port write operations via hmp interface. In that, 'current_cpu' remains null as it is not called from cpu_exec loop, which results in the said issue. Below is a proposed (tested)patch to fix this issue; Does it look okay? === From ae88a4947fab9a148cd794f8ad2d812e7f5a1d0f Mon Sep 17 00:00:00 2001 From: Prasad J Pandit Date: Fri, 18 Dec 2015 11:16:07 +0530 Subject: [PATCH] i386: avoid null pointer dereference When I/O port write operation is called from hmp interface, 'current_cpu' remains null, as it is not called from cpu_exec() loop. This leads to a null pointer dereference in vapic_write routine. Add check to avoid it. Reported-by: Ling Liu Signed-off-by: Prasad J Pandit Message-Id: Signed-off-by: Paolo Bonzini Signed-off-by: P J P --- hw/i386/kvmvapic.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'hw') diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c index c6d34b2..f0922da 100644 --- a/hw/i386/kvmvapic.c +++ b/hw/i386/kvmvapic.c @@ -634,13 +634,18 @@ static int vapic_prepare(VAPICROMState *s) static void vapic_write(void *opaque, hwaddr addr, uint64_t data, unsigned int size) { - CPUState *cs = current_cpu; - X86CPU *cpu = X86_CPU(cs); - CPUX86State *env = &cpu->env; - hwaddr rom_paddr; VAPICROMState *s = opaque; + X86CPU *cpu; + CPUX86State *env; + hwaddr rom_paddr; - cpu_synchronize_state(cs); + if (!current_cpu) { + return; + } + + cpu_synchronize_state(current_cpu); + cpu = X86_CPU(current_cpu); + env = &cpu->env; /* * The VAPIC supports two PIO-based hypercalls, both via port 0x7E. -- cgit v1.1 From 36fef36b91f7ec0435215860f1458b5342ce2811 Mon Sep 17 00:00:00 2001 From: P J P Date: Mon, 21 Dec 2015 15:13:13 +0530 Subject: scsi: initialise info object with appropriate size While processing controller 'CTRL_GET_INFO' command, the routine 'megasas_ctrl_get_info' overflows the '&info' object size. Use its appropriate size to null initialise it. Reported-by: Qinghao Tang Signed-off-by: Prasad J Pandit Message-Id: Cc: qemu-stable@nongnu.org Signed-off-by: Paolo Bonzini Signed-off-by: P J P --- hw/scsi/megasas.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'hw') diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c index d7dc667..576f56c 100644 --- a/hw/scsi/megasas.c +++ b/hw/scsi/megasas.c @@ -718,7 +718,7 @@ static int megasas_ctrl_get_info(MegasasState *s, MegasasCmd *cmd) BusChild *kid; int num_pd_disks = 0; - memset(&info, 0x0, cmd->iov_size); + memset(&info, 0x0, dcmd_size); if (cmd->iov_size < dcmd_size) { trace_megasas_dcmd_invalid_xfer_len(cmd->index, cmd->iov_size, dcmd_size); -- cgit v1.1 From 46f296cd3a3ebc3d30e2dbc1da7c4882e3d35ce5 Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 23 Dec 2015 13:59:04 +0000 Subject: qemu-char: delete send_all/recv_all helper methods The qemu-char.c contains two helper methods send_all and recv_all. These are in fact declared in sockets.h so ought to have been in util/qemu-sockets.c. For added fun the impl of recv_all is completely missing on Win32. Fortunately there is only a single caller of these methods, the TPM passthrough code, which is only ever compiled on Linux. With only a single caller these helpers are not compelling enough to keep so inline them in the TPM code, avoiding the need to fix the missing recv_all on Win32. Signed-off-by: Daniel P. Berrange Message-Id: <1450879144-17111-1-git-send-email-berrange@redhat.com> Signed-off-by: Paolo Bonzini --- hw/tpm/tpm_passthrough.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-) (limited to 'hw') diff --git a/hw/tpm/tpm_passthrough.c b/hw/tpm/tpm_passthrough.c index be160c1..ab526db 100644 --- a/hw/tpm/tpm_passthrough.c +++ b/hw/tpm/tpm_passthrough.c @@ -83,12 +83,37 @@ static void tpm_passthrough_cancel_cmd(TPMBackend *tb); static int tpm_passthrough_unix_write(int fd, const uint8_t *buf, uint32_t len) { - return send_all(fd, buf, len); + int ret, remain; + + remain = len; + while (len > 0) { + ret = write(fd, buf, remain); + if (ret < 0) { + if (errno != EINTR && errno != EAGAIN) { + return -1; + } + } else if (ret == 0) { + break; + } else { + buf += ret; + remain -= ret; + } + } + return len - remain; } static int tpm_passthrough_unix_read(int fd, uint8_t *buf, uint32_t len) { - return recv_all(fd, buf, len, true); + int ret; + reread: + ret = read(fd, buf, len); + if (ret < 0) { + if (errno != EINTR && errno != EAGAIN) { + return -1; + } + goto reread; + } + return ret; } static uint32_t tpm_passthrough_get_size_from_buffer(const uint8_t *buf) -- cgit v1.1 From e1dc68155cafabfd6a065391f7826d5d0992b46e Mon Sep 17 00:00:00 2001 From: Cao jin Date: Wed, 6 Jan 2016 17:37:46 +0800 Subject: SCSI device: fix to incomplete QOMify Signed-off-by: Cao jin Acked-by: Michael S. Tsirkin Message-Id: <1452073066-28319-1-git-send-email-caoj.fnst@cn.fujitsu.com> Signed-off-by: Paolo Bonzini --- hw/scsi/megasas.c | 12 ++++++------ hw/scsi/scsi-bus.c | 4 ++-- hw/scsi/virtio-scsi.c | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) (limited to 'hw') diff --git a/hw/scsi/megasas.c b/hw/scsi/megasas.c index 576f56c..60c0e6c 100644 --- a/hw/scsi/megasas.c +++ b/hw/scsi/megasas.c @@ -744,7 +744,7 @@ static int megasas_ctrl_get_info(MegasasState *s, MegasasCmd *cmd) info.device.type = MFI_INFO_DEV_SAS3G; info.device.port_count = 8; QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) { - SCSIDevice *sdev = DO_UPCAST(SCSIDevice, qdev, kid->child); + SCSIDevice *sdev = SCSI_DEVICE(kid->child); uint16_t pd_id; if (num_pd_disks < 8) { @@ -960,7 +960,7 @@ static int megasas_dcmd_pd_get_list(MegasasState *s, MegasasCmd *cmd) max_pd_disks = MFI_MAX_SYS_PDS; } QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) { - SCSIDevice *sdev = DO_UPCAST(SCSIDevice, qdev, kid->child); + SCSIDevice *sdev = SCSI_DEVICE(kid->child); uint16_t pd_id; if (num_pd_disks >= max_pd_disks) @@ -1136,7 +1136,7 @@ static int megasas_dcmd_ld_get_list(MegasasState *s, MegasasCmd *cmd) max_ld_disks = MFI_MAX_LD; } QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) { - SCSIDevice *sdev = DO_UPCAST(SCSIDevice, qdev, kid->child); + SCSIDevice *sdev = SCSI_DEVICE(kid->child); if (num_ld_disks >= max_ld_disks) { break; @@ -1187,7 +1187,7 @@ static int megasas_dcmd_ld_list_query(MegasasState *s, MegasasCmd *cmd) max_ld_disks = MFI_MAX_LD; } QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) { - SCSIDevice *sdev = DO_UPCAST(SCSIDevice, qdev, kid->child); + SCSIDevice *sdev = SCSI_DEVICE(kid->child); if (num_ld_disks >= max_ld_disks) { break; @@ -1327,7 +1327,7 @@ static int megasas_dcmd_cfg_read(MegasasState *s, MegasasCmd *cmd) ld_offset = array_offset + sizeof(struct mfi_array) * num_pd_disks; QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) { - SCSIDevice *sdev = DO_UPCAST(SCSIDevice, qdev, kid->child); + SCSIDevice *sdev = SCSI_DEVICE(kid->child); uint16_t sdev_id = ((sdev->id & 0xFF) << 8) | (sdev->lun & 0xFF); struct mfi_array *array; struct mfi_ld_config *ld; @@ -2237,7 +2237,7 @@ static void megasas_soft_reset(MegasasState *s) * after the initial reset. */ QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) { - SCSIDevice *sdev = DO_UPCAST(SCSIDevice, qdev, kid->child); + SCSIDevice *sdev = SCSI_DEVICE(kid->child); sdev->unit_attention = SENSE_CODE(NO_SENSE); scsi_device_unit_attention_reported(sdev); diff --git a/hw/scsi/scsi-bus.c b/hw/scsi/scsi-bus.c index 378bf4d..164af87 100644 --- a/hw/scsi/scsi-bus.c +++ b/hw/scsi/scsi-bus.c @@ -1861,7 +1861,7 @@ void scsi_device_purge_requests(SCSIDevice *sdev, SCSISense sense) static char *scsibus_get_dev_path(DeviceState *dev) { - SCSIDevice *d = DO_UPCAST(SCSIDevice, qdev, dev); + SCSIDevice *d = SCSI_DEVICE(dev); DeviceState *hba = dev->parent_bus->parent; char *id; char *path; @@ -2034,7 +2034,7 @@ static void scsi_device_class_init(ObjectClass *klass, void *data) static void scsi_dev_instance_init(Object *obj) { DeviceState *dev = DEVICE(obj); - SCSIDevice *s = DO_UPCAST(SCSIDevice, qdev, dev); + SCSIDevice *s = SCSI_DEVICE(dev); device_add_bootindex_property(obj, &s->conf.bootindex, "bootindex", NULL, diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c index 3a4f520..607593c 100644 --- a/hw/scsi/virtio-scsi.c +++ b/hw/scsi/virtio-scsi.c @@ -352,7 +352,7 @@ static int virtio_scsi_do_tmf(VirtIOSCSI *s, VirtIOSCSIReq *req) target = req->req.tmf.lun[1]; s->resetting++; QTAILQ_FOREACH(kid, &s->bus.qbus.children, sibling) { - d = DO_UPCAST(SCSIDevice, qdev, kid->child); + d = SCSI_DEVICE(kid->child); if (d->channel == 0 && d->id == target) { qdev_reset_all(&d->qdev); } -- cgit v1.1