In previous commit:
commit 7dea29e4af
Author: Li Qiang <liq3ea@gmail.com>
Date: Fri Oct 19 03:50:36 2018 -0700
hw: ccid-card-emulated: cleanup resource when realize in error path
The emulated_realize method was changed so that it jumps to a cleanup
label to de-initialize state upon error. This change failed to ensure
the success path exited the method before this point though. So the
mutexes are always destroyed even in normal operation. The result is
as crashtastic as expected:
$ qemu-system-x86_64 -usb -device usb-ccid,id=ccid0 -device ccid-card-emulated,backend=nss-emulated,id=smartcard0,bus=ccid0.0
qemu-system-x86_64: util/qemu-thread-posix.c:64: qemu_mutex_lock_impl: Assertion `mutex->initialized' failed.
Aborted (core dumped)
Fixes: 7dea29e4af
Reported-by: Michael Tokarev <mjt@tls.msk.ru>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20181221134115.27973-1-berrange@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
The filename length in MTP metadata is specified by the guest. By
trusting it directly it'd theoretically be possible to get the host to
write memory parts outside the filename buffer into a filename. In
practice though there are usually NUL bytes stopping the string
operations.
Also use the opportunity to not assign the filename member twice.
Signed-off-by: Michael Hanselmann <public@hansmi.ch>
Message-id: ab70659d8d5c580bdf150a5f7d5cc60c8e374ffc.1544740018.git.public@hansmi.ch
[ kraxel: codestyle fix: break a long line ]
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Open files and directories with O_NOFOLLOW to avoid symlinks attacks.
While being at it also add O_CLOEXEC.
usb-mtp only handles regular files and directories and ignores
everything else, so users should not see a difference.
Because qemu ignores symlinks, carrying out a successful symlink attack
requires swapping an existing file or directory below rootdir for a
symlink and winning the race against the inotify notification to qemu.
Fixes: CVE-2018-16872
Cc: Prasad J Pandit <ppandit@redhat.com>
Cc: Bandan Das <bsd@redhat.com>
Reported-by: Michael Hanselmann <public@hansmi.ch>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Michael Hanselmann <public@hansmi.ch>
Message-id: 20181213122511.13853-1-kraxel@redhat.com
Use DeviceClass rather than SysBusDeviceClass in
tusb6010_class_init().
Cc: kraxel@redhat.com
Signed-off-by: Mao Zhongyi <maozhongyi@cmss.chinamobile.com>
Signed-off-by: Zhang Shengju <zhangshengju@cmss.chinamobile.com>
Message-id: 20181130093852.20739-20-maozhongyi@cmss.chinamobile.com
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
The token field contains the (guest-filled) state of the qtd, which
indicates whenever the other fields are valid or not. So make sure
we read the token first, otherwise we may end up with an stale next
pointer:
(1) ehci reads next
(2) guest writes next
(3) guest writes token
(4) ehci reads token
(5) ehci operates with stale next.
Typical effect is that qemu doesn't notice that the guest appends new
qtds to the end of the queue. Looks like the usb device stopped
responding. Linux can recover from that, but leaves a message in the
kernel log that it did reset the usb device in question.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20181126100836.8805-1-kraxel@redhat.com
we should perform these things as same as usb_host_close.
Signed-off-by: linzhecheng <linzhecheng@huawei.com>
Message-id: 20181130064700.5984-1-linzhecheng@huawei.com
[ kraxel: whitespace fixup ]
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Don't call xen_be_set_max_grant_refs() in usbback_alloc(), as the
gnttabdev pointer won't be initialised yet. The call can easily be
moved to usbback_connect().
Signed-off-by: Juergen Gross <jgross@suse.com>
Message-id: 20181206133923.30105-1-jgross@suse.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Slash is unix directory separator, so they are not allowed in filenames.
Note this also stops the classic escape via "../".
Fixes: CVE-2018-16867
Reported-by: Michael Hanselmann <public@hansmi.ch>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-id: 20181203101045.27976-3-kraxel@redhat.com
Make utf16_to_str return an allocated string. Remove the assumtion that
the number of string bytes equals the number of utf16 chars (which is
only true for ascii chars). Instead call wcstombs twice, once to figure
the storage size and once for the actual conversion (as suggested by the
wcstombs manpage).
FIXME: surrogate pairs are not working correctly. Pre-existing bug,
fixing that is left for another day.
Reported-by: Michael Hanselmann <public@hansmi.ch>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-id: 20181203101045.27976-2-kraxel@redhat.com
Because they are supposed to remain const.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20181114132931.22624-1-marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
If no kernel driver is active, we can already claim and perform I/O on
it without detaching it.
Signed-off-by: linzhecheng <linzhecheng@huawei.com>
Message-id: 20181120083419.17716-1-linzhecheng@huawei.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
This can avoid setting OCHIState.num_ports to a negative num.
Signed-off-by: Li Qiang <liq3ea@gmail.com>
Message-id: 1540263618-18344-1-git-send-email-liq3ea@gmail.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
From include/qapi/error.h:
* Pass an existing error to the caller with the message modified:
* error_propagate(errp, err);
* error_prepend(errp, "Could not frobnicate '%s': ", name);
Fei Li pointed out that doing error_propagate() first doesn't work
well when @errp is &error_fatal or &error_abort: the error_prepend()
is never reached.
Since I doubt fixing the documentation will stop people from getting
it wrong, introduce error_propagate_prepend(), in the hope that it
lures people away from using its constituents in the wrong order.
Update the instructions in error.h accordingly.
Convert existing error_prepend() next to error_propagate to
error_propagate_prepend(). If any of these get reached with
&error_fatal or &error_abort, the error messages improve. I didn't
check whether that's the case anywhere.
Cc: Fei Li <fli@suse.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-Id: <20181017082702.5581-2-armbru@redhat.com>
USB frame rate is slightly lower than 1kHz: ie. ~950Hz.
Thus usb-audio device is not able to perform a simple audio playback
without underruns on audio backend.
eg. "-device pci-ohci,id=ohci -device usb-audio,bus=ohci.0" vs PulseAudio
backend. more than 50 underruns are observed per second.
Update ohci_sof_time computation, using QEMU_CLOCK_VIRTUAL in
ohci_usb_start(), and increment by usb_frame_time in ohci_sof()
makes USB frame rate close to 1kHz.
This way, no audio underrun are observed during audio playback.
Signed-off-by: Miguel GAIO <mgaio35@gmail.com>
Message-Id: <20180927151936.3647-1-mgaio35@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Stale values in this field may result in qemu
expecting more data on the next operation
Signed-off-by: Bandan Das <bsd@redhat.com>
Message-id: 20180907220851.9658-4-bsd@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Return STORE_FULL if we can't write all the bytes but
return incomplete transfer if data received is less then
what was specified in the metadata. Also, use d->offset
as the file size which is valid for all file sizes.
Signed-off-by: Bandan <bsd@redhat.com>
Message-id: 20180907220851.9658-2-bsd@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
x-root was renamed as such owing to the experimental nature of the
property; the underlying filesystem semantics were undecided
Signed-off-by: Bandan Das <bsd@redhat.com>
Message-id: 20180720214020.22897-6-bsd@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
To support larger file transfers, rely on a short packet
to detect end of the data phase and rewrite d->length to
the size received
Signed-off-by: Bandan Das <bsd@redhat.com>
Message-id: 20180720214020.22897-5-bsd@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
For large buffers, write may not copy the full buffer. For example,
on Linux, write imposes a limit of 0x7ffff000. Note that this does
not fix >4G transfers but ~>2G files will transfer successfully.
Signed-off-by: Bandan Das <bsd@redhat.com>
Message-id: 20180720214020.22897-4-bsd@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
usb_mtp_realloc() was being incorrectly used when allocating
buffer for incoming data. Set d->length only after resizing
the buffer.
Signed-off-by: Bandan Das <bsd@redhat.com>
Message-id: 20180720214020.22897-3-bsd@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
The initiator can choose to cancel an ongoing request which
is specified by bRequest=0x64. If such a request arrives,
free up any pending state
Signed-off-by: Bandan Das <bsd@redhat.com>
Message-id: 20180720214020.22897-2-bsd@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
This is mandated by the ohci specification. It tells at 6.4.4 on page 104
that for transfer descriptors that are retired with an error the done
queue interrupt counter is cleared as if the interrupt delay field of the
descriptions were zero.
Before this change, error conditions were handled similarly to the
successful condition which is especially troublesome for control transfers.
Some drivers (e.g., the AmigaOS-one) as well as the example code in the
spec, set the setup stage with an interrupt delay of seven (which means no
interrupt). This is fine under normal conditions, because one usually
doesn't want to be notified about the completion of this stage. However, if
an error occurs in this stage, these drivers will not get notified with the
current implementation. The fix addresses this by following the spec more
closely. Also, otherwise, the ability to set interrupt delay to seven would
be useless.
Note that Linux drivers that I looked at don't seem to be affected as they
set six as the interrupt delay presumably for the reason that they won't
get notified otherwise.
Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info>
Message-id: 20180729191928.11254-1-mail@sebastianbauer.info
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
This reinstates commit b008326744,
which was temporarily reverted for the 3.0 release so that libvirt gets
some extra time to update their command lines.
The -drive option serial was deprecated in QEMU 2.10. It's time to
remove it.
Tests need to be updated to set the serial number with -global instead
of using the -drive option.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
This reverts commit b008326744.
Hold off removing this for one more QEMU release (current libvirt
release still uses it.)
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Set xhci into error state instead of throwing a core dump.
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 20180702162752.29233-1-kraxel@redhat.com
Fetching qtd with the NULL address most likely makes no sense so from now
on, we handle it this case similarly as if the terminate (T) bit is not
set, which is already an exception as according to section 3.6 of the EHCI
spec there is no T bit defined for the current_qtd field.
The spec is a bit vague on how an EHCI driver should initialize these
fields: "The general operational model is that the host controller can
detect whether the overlay area contains a description of an active
transfer" (p. 49). QEMU primarily uses the QTD_TOKEN_ACTIVE bit of the
queue header to infer the activity state but there are other ways
conceivable.
This change allows QEMU to boot further into AmigaOS. The public available
version of the EHCI driver recycles queue heads in some rare conditions but
only clears the current_qtd field but not the status field. This works with
many available EHCI PCI cards but e.g., not with the Freescale USB
controller's found on the P5040. On the emulated EHCI controller of QEMU
the consequence is that some garbage was read in, which resulted in a
reset of the controller. This change fixes the problem.
Signed-off-by: Sebastian Bauer <mail@sebastianbauer.info>
Tested-by: BALATON Zoltan <balaton@eik.bme.hu>
Message-id: 20180625222718.4488-1-mail@sebastianbauer.info
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
It eases code review, unit is explicit.
Patch generated using:
$ git grep -E '(1024|2048|4096|8192|(<<|>>).?(10|20|30))' hw/ include/hw/
and modified manually.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20180625124238.25339-36-f4bug@amsat.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
The error handling policy was traditionally set with -drive, but with
-blockdev it is no longer possible to set frontend options. scsi-disk
(and other block devices) have long supported qdev properties to
configure the error handling policy, so let's add these options to
usb-storage as well and just forward them to the internal scsi-disk
instance.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
The USB device don't hold the bus. There is no ASAN related reports
anymore.
This reverts commit cd7bc87868.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-id: 20180613172815.32738-3-marcandre.lureau@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
The bus is not owned by the device.
This reverts commit 410a096adf.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-id: 20180613172815.32738-2-marcandre.lureau@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
The -drive option serial was deprecated in QEMU 2.10. It's time to
remove it.
Tests need to be updated to set the serial number with -global instead
of using the -drive option.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Jeff Cody <jcody@redhat.com>
We banned use of certain g_assert_FOO() functions outside tests, and
made checkpatch.pl flag them (commit 6e9389563e). We neglected to
purge existing uses. Do that now.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20180608170231.27912-1-armbru@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: John Snow <jsnow@redhat.com>
CID 1390604
If the initiator sends a packet with TYPE_DATA set without
initiating a CMD_GET_OBJECT_INFO first, then usb_mtp_get_data
can trip on a null s->data_out.
Signed-off-by: Bandan Das <bsd@redhat.com>
Message-Id: <jpgr2m8ajfk.fsf_-_@linux.bootlegged.copy>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
qbus_create_inplace() creates a new reference in realize(), it must be
released in unrealize().
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-id: 20180531195119.22021-4-marcandre.lureau@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
USB Specification Revision 2.0, §5.5.3:
The Data stage of a control transfer from an endpoint to the host is complete when the endpoint does one of the following:
• Has transferred exactly the amount of data specified during the Setup stage
• Transfers a packet with a payload size less than wMaxPacketSize or transfers a zero-length packet"
hw/usb/redirect.c:802:9: warning: Declared variable-length array (VLA) has zero size
uint8_t buf[size];
^~~~~~~~~~~ ~~~~
Reported-by: Clang Static Analyzer
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20180604151421.23385-2-f4bug@amsat.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
vDPA support, fix to vhost blk RO bit handling, some include path
cleanups, NFIT ACPI table.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
-----BEGIN PGP SIGNATURE-----
iQEcBAABAgAGBQJbEXNvAAoJECgfDbjSjVRpc8gH/R8xrcFrV+k9wwbgYcOcGb6Y
LWjseE31pqJcxRV80vLOdzYEuLStZQKQQY7xBDMlA5vdyvZxIA6FLO2IsiJSbFAk
EK8pclwhpwQAahr8BfzenabohBv2UO7zu5+dqSvuJCiMWF3jGtPAIMxInfjXaOZY
odc1zY2D2EgsC7wZZ1hfraRbISBOiRaez9BoGDKPOyBY9G1ASEgxJgleFgoBLfsK
a1XU+fDM6hAVdxftfkTm0nibyf7PWPDyzqghLqjR9WXLvZP3Cqud4p8N29mY51pR
KSTjA4FYk6Z9EVMltyBHfdJs6RQzglKjxcNGdlrvacDfyFi79fGdiosVllrjfJM=
=3+V0
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging
acpi, vhost, misc: fixes, features
vDPA support, fix to vhost blk RO bit handling, some include path
cleanups, NFIT ACPI table.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
# gpg: Signature made Fri 01 Jun 2018 17:25:19 BST
# gpg: using RSA key 281F0DB8D28D5469
# gpg: Good signature from "Michael S. Tsirkin <mst@kernel.org>"
# gpg: aka "Michael S. Tsirkin <mst@redhat.com>"
# Primary key fingerprint: 0270 606B 6F3C DF3D 0B17 0970 C350 3912 AFBE 8E67
# Subkey fingerprint: 5D09 FD08 71C8 F85B 94CA 8A0D 281F 0DB8 D28D 5469
* remotes/mst/tags/for_upstream: (31 commits)
vhost-blk: turn on pre-defined RO feature bit
ACPI testing: test NFIT platform capabilities
nvdimm, acpi: support NFIT platform capabilities
tests/.gitignore: add entry for generated file
arch_init: sort architectures
ui: use local path for local headers
qga: use local path for local headers
colo: use local path for local headers
migration: use local path for local headers
usb: use local path for local headers
sd: fix up include
vhost-scsi: drop an unused include
ppc: use local path for local headers
rocker: drop an unused include
e1000e: use local path for local headers
ioapic: fix up includes
ide: use local path for local headers
display: use local path for local headers
trace: use local path for local headers
migration: drop an unused include
...
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
When pulling in headers that are in the same directory as the C file (as
opposed to one in include/), we should use its relative path, without a
directory.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Tested-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Remove those unneeded includes to speed up the compilation
process a little bit.
Code change produced with:
$ git grep '#include "sysemu/blockdev.h"' | \
cut -d: -f-1 | \
xargs egrep -L "(BlockInterfaceType|DriveInfo|drive_get|blk_legacy_dinfo|blockdev_mark_auto_del)" | \
xargs sed -i.bak '/#include "sysemu\/blockdev.h"/d'
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20180528232719.4721-15-f4bug@amsat.org>
Acked-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Now that helpers are available in xen_backend, use them throughout all
Xen PV backends.
Signed-off-by: Paul Durrant <paul.durrant@citrix.com>
Acked-by: Anthony Perard <anthony.perard@citrix.com>
Signed-off-by: Stefano Stabellini <sstabellini@kernel.org>
The current code was not correctly handling 64 B (Max USB 1.1 payload size)
packets and therefore preventing some of the messages from smart card to
pass through to the guest.
If the smart card in host responded with 34 B of data in APDU layer, the
CCID headers added up to 64 B. The packet was send, but not correctly
committed per USB specification (8.5.3.2 Variable-length Data Stage):
> When all of the data structure is returned to the host, the function
> should indicate that the Data stage is ended by returning a packet
> that is shorter than the MaxPacketSize for the pipe. If the data
> structure is an exact multiple of wMaxPacketSize for the pipe, the
> function will return a zero-length packet to indicate the end of the
> Data stage.
This lead the guest applications to timeout while waiting for the rest
of data (the emulation layer is answering with NAK until the timeout).
This patch is checking the current maximum packet size and if the
payload of this size is detected, the message buffer is not yet released.
With the next call, the empty buffer is sent and the message buffer
is finally released.
Signed-off-by: Jakub Jelen <jjelen@redhat.com>
Message-id: 20180516115544.3897-2-jjelen@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Since cc847bfd16, CCID card-passthru
fails to intialize, because it changed a debug line to an error,
probably by mistake. Change it back to a DPRINTF debug.
(solves Boxes creating VM with smartcard passthru failing to start)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20180515153039.27514-1-marcandre.lureau@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
