Qcrypto secrets luks 1 #35
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The standard glib provided g_base64_decode doesn't provide any kind of sensible error checking on its input. Add a QEMU custom wrapper qbase64_decode which can be used with untrustworthy input that can contain invalid base64 characters, embedded NUL characters, or not be NUL terminated at all. Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Switch from using g_base64_decode over to qbase64_decode in order to get error checking of the base64 input data. Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Switch from using g_base64_decode over to qbase64_decode in order to get error checking of the base64 input data. Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Introduce a new QCryptoSecret object class which will be used
for providing passwords and keys to other objects which need
sensitive credentials.
The new object can provide secret values directly as properties,
or indirectly via a file. The latter includes support for file
descriptor passing syntax on UNIX platforms. Ordinarily passing
secret values directly as properties is insecure, since they
are visible in process listings, or in log files showing the
CLI args / QMP commands. It is possible to use AES-256-CBC to
encrypt the secret values though, in which case all that is
visible is the ciphertext. For ad hoc developer testing though,
it is fine to provide the secrets directly without encryption
so this is not explicitly forbidden.
The anticipated scenario is that libvirtd will create a random
master key per QEMU instance (eg /var/run/libvirt/qemu/$VMNAME.key)
and will use that key to encrypt all passwords it provides to
QEMU via '-object secret,....'. This avoids the need for libvirt
(or other mgmt apps) to worry about file descriptor passing.
It also makes life easier for people who are scripting the
management of QEMU, for whom FD passing is significantly more
complex.
Providing data inline (insecure, only for ad hoc dev testing)
$QEMU -object secret,id=sec0,data=letmein
Providing data indirectly in raw format
printf "letmein" > mypasswd.txt
$QEMU -object secret,id=sec0,file=mypasswd.txt
Providing data indirectly in base64 format
$QEMU -object secret,id=sec0,file=mykey.b64,format=base64
Providing data with encryption
$QEMU -object secret,id=master0,file=mykey.b64,format=base64 \
-object secret,id=sec0,data=[base64 ciphertext],\
keyid=master0,iv=[base64 IV],format=base64
Note that 'format' here refers to the format of the ciphertext
data. The decrypted data must always be in raw byte format.
More examples are shown in the updated docs.
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Make use of the QCryptoSecret object to support loading of
encrypted x509 keys. The optional 'passwordid' parameter
to the tls-creds-x509 object type, provides the ID of a
secret object instance that holds the decryption password
for the PEM file.
# printf "123456" > mypasswd.txt
# $QEMU \
-object secret,id=sec0,filename=mypasswd.txt \
-object tls-creds-x509,passwordid=sec0,id=creds0,\
dir=/home/berrange/.pki/qemu,endpoint=server \
-vnc :1,tls-creds=creds0
This requires QEMU to be linked to GNUTLS >= 3.1.11. If
GNUTLS is too old an error will be reported if an attempt
is made to pass a decryption password.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Add a 'keyid' parameter that refers to the ID of a
QCryptoSecret instance that provides the encryption key.
eg
$QEMU \
-object secret,id=sec0,filename=/home/berrange/encrypted.pw \
-drive file=/home/berrange/encrypted.qcow,keyid=sec0
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Add a 'keyid' parameter that refers to the ID of a
QCryptoSecret instance that provides the encryption key.
$QEMU \
-object secret,id=sec0,filename=/home/berrange/encrypted.pw \
-drive file=/home/berrange/encrypted.qcow2,keyid=sec0
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The QMP monitor code has two helper methods object_add and qmp_object_del that are called from several places in the code (QMP, HMP and main emulator startup). We soon need to use this code from qemu-img, qemu-io and qemu-nbd too, but don't want those to depend on the monitor. To avoid this, move object_add to user_creatable_add an qmp_object_del to user_creatable_del, in the object_interfaces.c file Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Allow creation of user creatable object types with qemu-img
via a --object command line arg. This will be used to supply
passwords and/or encryption keys to the various block driver
backends via the recently added 'secret' object type.
# echo -n letmein > mypasswd.txt
# qemu-img info --object secret,id=sec0,file=mypasswd.txt \
...other info args...
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Allow creation of user creatable object types with qemu-nbd
via a --object command line arg. This will be used to supply
passwords and/or encryption keys to the various block driver
backends via the recently added 'secret' object type.
# echo -n letmein > mypasswd.txt
# qemu-nbd --object secret,id=sec0,file=mypasswd.txt \
...other nbd args...
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Allow creation of user creatable object types with qemu-io
via a --object command line arg. This will be used to supply
passwords and/or encryption keys to the various block driver
backends via the recently added 'secret' object type.
# echo -n letmein > mypasswd.txt
# qemu-io --object secret,id=sec0,file=mypasswd.txt \
...other args...
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently qemu-io allows an image filename to be passed on the command line, but does not have a way to set any options except the format eg qemu-io https://127.0.0.1/images/centos7.iso qemu-io /home/berrange/demo.qcow2 This adds a --source arg (that is mutually exclusive with a positional filename arg and -f arg) that accepts a full option string, as well as the original syntax eg qemu-io --source driver=http,url=https://127.0.0.1/images,sslverify=off qemu-io --source https://127.0.0.1/images/centos7.iso qemu-io --source file=/home/berrange/demo.qcow2 qemu-io --source /home/berrange/demo.qcow2 Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently qemu-nbd allows an image filename to be passed on the command line, but does not have a way to set any options except the format eg qemu-nbd https://127.0.0.1/images/centos7.iso qemu-nbd /home/berrange/demo.qcow2 This adds a --source arg (that is mutually exclusive with a positional filename arg and -f arg) that accepts a full option string, as well as the original syntax eg qemu-nbd --source driver=http,url=https://127.0.0.1/images,sslverify=off qemu-nbd --source https://127.0.0.1/images/centos7.iso qemu-nbd --source file=/home/berrange/demo.qcow2 qemu-nbd --source /home/berrange/demo.qcow2 Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Currently qemu-img allows an image filename to be passed on the command line, but does not have a way to set any options except the format eg qemu-img info https://127.0.0.1/images/centos7.iso This adds a --source arg (that is mutually exclusive with a positional filename arg and -f arg) that accepts a full option string, as well as the original syntax eg qemu-img info driver=http,url=https://127.0.0.1/images,sslverify=off Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Now that qcow & qcow2 are wired up to get encryption keys via the QCryptoSecret object, all traces of code which had to deal with prompting for passwords can be ripped out. When the image is initially opened, the encryption key must be available immediately, or an error will be reported. $ qemu-system-x86_64 -drive file=secret.qcow2 qemu-system-x86_64: -drive file=secret.qcow2: Image is encrypted but no secret is provided Users must provide the secret with -object $ echo 123456 > mypasswd.txt qemu-system-x86_64 -drive file=secret.qcow2,keysecret=sec0 -object secret,file=mypasswd.txt,id=sec0 The BDRV_O_NO_IO flag allows this error to be skipped, for use when 'qemu-img info' wants to open the file to query the headers, but not perform any actual I/O operations. Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Now that all encryption keys must be provided upfront via the QCryptoSecret API and associated block driver properties there is no need for any explicit encryption handling APIs in the block layer. Encryption can be handled transparently within the block driver. We only retain an API for querying whether an image is encrypted or not, since that is a potentially useful piece of metadata to report to the user. Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Refuse to open a qcow/qcow2 image with encryption if write access has been requested. To enable historic data to be liberated support for reading images is retained, as it does not pose an unreasonable support burden now that the new key handling infrastructure is inplace. Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Instead of requiring separate input/output buffers for encrypting data, change qcow2_encrypt_sectors() to assume use of a single buffer, encrypting in place. The current callers all used the same buffer for input/output already. Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Adds new methods to allow querying the length of the cipher key, block size and initialization vectors. Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Add a qcrypto_hash_digest_len() method which allows querying of the raw digest size for a given hash algorithm. Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The QCryptoHashAlgorithm enum is defined in the crypto/hash.h header. In the future some QAPI types will want to reference the hash enums, so move the enum definition into QAPI too. Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The QCryptoCipherAlgorithm and QCryptoCipherMode enums are defined in the crypto/cipher.h header. In the future some QAPI types will want to reference the hash enums, so move the enum definition into QAPI too. Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The rebuild of qapi-types.c/h is not correctly triggered when qapi/crypto.json is changed because it was missing from the list of files in the qapi-modules variable. Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The LUKS data format includes use of PBKDF2 (Password-Based Key Derivation Function). The Nettle library can provide an implementation of this, but we don't want code directly depending on a specific crypto library backend. Introduce a include/crypto/pbkdf.h header which defines a QEMU API for invoking PBKDK2. The initial implementations are backed by nettle & gcrypt, which are commonly available with distros shipping GNUTLS. The test suite data is taken from the cryptsetup codebase under the LGPLv2.1+ license. This merely aims to verify that whatever backend we provide for this function in QEMU will comply with the spec. Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
There are a number of different algorithms that can be used to generate initialization vectors for disk encryption. This introduces a simple internal QCryptoBlockIV object to provide a consistent internal API to the different algorithms. The initially implemented algorithms are 'plain', 'plain64' and 'essiv', each matching the same named algorithm provided by the Linux kernel dm-crypt driver. Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The LUKS format specifies an anti-forensic split algorithm which is used to artificially expand the size of the key material on disk. This is an implementation of that algorithm. Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
When reporting an incorrect key length for a cipher, we mixed up the actual vs expected arguments. Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Add a generic framework for support different block encryption formats. Upon instantiating a QCryptoBlock object, it will read the encryption header and extract the encryption keys. It is then possible to call methods to encrypt/decrypt data buffers. There is also a mode whereby it will create/initialize a new encryption header on a previously unformatted volume. The initial framework comes with support for the legacy QCow AES based encryption. This enables code in the QCow driver to be consolidated later. Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Provide a block encryption implementation that follows the LUKS/dm-crypt specification. This supports all combinations of hash, cipher algorithm, cipher mode and iv generator that are implemented by the current crypto layer. The notable missing feature is support for the 'xts' cipher mode, which is commonly used for disk encryption instead of 'cbc'. This is because it is not provided by either nettle or libgcrypt. A suitable implementation will be identified & integrated later. There is support for opening existing volumes formatted by dm-crypt, and for formatting new volumes. In the latter case it will only use key slot 0. Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
Add a block driver that is capable of supporting any full disk
encryption format. This utilizes the previously added block
encryption code, and at this time supports the LUKS format.
The driver code is capable of supporting any format supported
by the QCryptoBlock module, so it registers one block driver
for each format.
At this time, the "luks" driver is registered. A new LUKS
compatible volume can be formatted using qemu-img
$ qemu-img create --object secret,data=123456,id=sec0 \
-f luks -o keyid=sec0,cipher_alg=aes-256,\
cipher_mode=cbc,ivgen_alg=plain64,hash_alg=sha256 \
demo.luks 10G
And query its size
$ qemu-img info --object secret,data=123456,id=sec0 --source demo.luks,driver=luks,keyid=sec0
image: json:{"keyid": "sec0", "driver": "luks", "file": {"driver": "file", "filename": "demo.luks"}}
file format: luks
virtual size: 10.0G (10737416192 bytes)
disk size: 132K
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The QCryptoBlock framework trivially supports the legecy QCow encryption format. Convert QCow2 to use QCryptoBlock, since this will unlock the ability to support LUKS in QCow2 without increasing the code burden for encryption in QCow2. Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
The QCow2 format currently has support for built-in AES encryption, however, this is fundamentally flawed from a cryptographic security POV, so its use is deprecated. The previously added generic full disk encryption driver could be used to encrypt QCow2 files by either laying it above or below the QCow2 driver in the QEMU BlockBackend tree. If it is layered above (FDE -> QCow2 -> File), then only the image payload will be encrypted. There is no safe way to auto-detect use of FDE for the image payload, as you cannot safely distinguish between a QCow2 image that has the FDE driver layered above in the host, from a QCow2 image where the guest is using LUKS over a partitionless drive. Layering above the image is a valid use case, but this auto-detection limitation makes it undesirable as a default approach for QCow2 encryption. If it is layered below (QCow2 -> FDE -> File), then both the image payload and QCow2 headers are encrypted. This makes it impossible to query the disk image to determine its logical disk size, or backing file requirements without first unlocking the decryption key. This is again a valid use case for scenarios where it is desirable to avoid any leakage of information about the underling disk format, but it is undesirable as a default approach for QCow2 encryption. Thus this patch takes a third approach of integrating LUKS support directly into the QCow2 file format. Only the image payload is encrypted, the QCow2 file header remainins in clear text. Thus makes it possible to probe info about the disk size, backing files, etc without needing decryption keys. Since use of LUKS is encoded in the QCow2 header, it is still possible to reliabily distinguish host side encryption from guest side encryption. First a new QCow2 encryption scheme is defined to represent the LUKS format, with a value '2' (0 == plain text, 1 == the legacy AES format). A corresponding new QCow2 header extension is defined to hold the LUKS partition header data. This stores the various encryption parameters and key slot metadata and the encrypted master keys. The payload of the QCow2 file does not change in structure. Sectors are simply processed via the QCryptoBlock object to apply/remove encryption when required. Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
0day-ci
pushed a commit
to 0day-ci/qemu
that referenced
this pull request
Aug 9, 2016
2016-08-09 08:31-0700, no-reply@ec2-52-6-146-230.compute-1.amazonaws.com: > Hi, > > Your series seems to have some coding style problems. See output below for > more information: > > Message-id: 20160809150333.9991-1-rkrcmar@redhat.com > Type: series > Subject: [Qemu-devel] [PATCH for-2.7 0/2] intel-iommu: restrict EIM to quirkless KVM > > === TEST SCRIPT BEGIN === > #!/bin/bash > > BASE=base > n=1 > total=$(git log --oneline $BASE.. | wc -l) > failed=0 > > commits="$(git log --format=%H --reverse $BASE..)" > for c in $commits; do > echo "Checking PATCH $n/$total: $(git show --no-patch --format=%s $c)..." > if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then > failed=1 > echo > fi > n=$((n+1)) > done > > exit $failed > === TEST SCRIPT END === > > Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384 > Switched to a new branch 'test' > e018fb0 intel-iommu: restrict EIM to quirkless KVM > 5ef6f2f linux-headers: update to v4.8-rc1 > > === OUTPUT BEGIN === > Checking PATCH 1/2: linux-headers: update to v4.8-rc1... > ERROR: code indent should never use tabs > qemu#32: FILE: linux-headers/linux/kvm.h:885: > +^Iunion {$ > > ERROR: code indent should never use tabs > qemu#33: FILE: linux-headers/linux/kvm.h:886: > +^I^I__u32 pad;$ > > ERROR: code indent should never use tabs > qemu#34: FILE: linux-headers/linux/kvm.h:887: > +^I^I__u32 devid;$ > > ERROR: code indent should never use tabs > qemu#35: FILE: linux-headers/linux/kvm.h:888: > +^I};$ > > ERROR: code indent should never use tabs > qemu#43: FILE: linux-headers/linux/kvm.h:1034: > +#define KVM_MSI_VALID_DEVID^I(1U << 0)$ > > ERROR: code indent should never use tabs > qemu#50: FILE: linux-headers/linux/kvm.h:1040: > +^I__u32 devid;$ > > ERROR: code indent should never use tabs > qemu#51: FILE: linux-headers/linux/kvm.h:1041: > +^I__u8 pad[12];$ > > ERROR: code indent should never use tabs > qemu#59: FILE: linux-headers/linux/kvm.h:1086: > +^IKVM_DEV_TYPE_ARM_VGIC_ITS,$ > > ERROR: code indent should never use tabs > qemu#60: FILE: linux-headers/linux/kvm.h:1087: > +#define KVM_DEV_TYPE_ARM_VGIC_ITS^IKVM_DEV_TYPE_ARM_VGIC_ITS$ > > total: 9 errors, 0 warnings, 51 lines checked > > Your patch has style problems, please review. If any of these errors > are false positives report them to the maintainer, see > CHECKPATCH in MAINTAINERS. These indentation errors are false positives. ---8<--- Make scripts/checkpatch.pl accept tabs in linux-headers/, instead of changing scripts/update-linux-headers.sh to expand tabs when importing. Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
brooksdavis
pushed a commit
to brooksdavis/qemu
that referenced
this pull request
Jun 7, 2017
rlim_t is an int64_t, not an abi_ulong. fixes getrlimit/setrlimit
qemu-deploy
pushed a commit
that referenced
this pull request
Jun 22, 2018
checkpatch reminds us that statics shouldn't be zero-initialized: ERROR: do not initialise statics to 0 or NULL #35: FILE: vl.c:157: +static int num_serial_hds = 0; ERROR: do not initialise statics to 0 or NULL #36: FILE: vl.c:158: +static Chardev **serial_hds = NULL; I forgot to fix this in 6af2692; do so now. Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Thomas Huth <thuth@redhat.com> Message-id: 20180426140253.3918-1-peter.maydell@linaro.org
famz
pushed a commit
to famz/qemu
that referenced
this pull request
Sep 28, 2018
Both virtio-blk and virtio-scsi use virtio_queue_empty() as the
loop condition in VQ handlers (virtio_blk_handle_vq,
virtio_scsi_handle_cmd_vq). When a device is marked broken in
virtqueue_pop, for example if a vIOMMU address translation failed, we
want to break out of the loop.
This fixes a hanging problem when booting a CentOS 3.10.0-862.el7.x86_64
kernel:
$ qemu-system-x86_64 \
... \
-device intel-iommu,intremap=on,caching-mode=on,eim=on,device-iotlb=on \
-device virtio-scsi-pci,iommu_platform=on,ats=on,id=scsi0,bus=pci.4,addr=0x0
The dead loop happens immediately when the kernel boots and initializes
the device, where virtio_scsi_data_plane_handle_cmd will not return:
> ...
> qemu#13 0x00005586602b7793 in virtio_scsi_handle_cmd_vq
> qemu#14 0x00005586602b8d66 in virtio_scsi_data_plane_handle_cmd
> qemu#15 0x00005586602ddab7 in virtio_queue_notify_aio_vq
> qemu#16 0x00005586602dfc9f in virtio_queue_host_notifier_aio_poll
> qemu#17 0x00005586607885da in run_poll_handlers_once
> qemu#18 0x000055866078880e in try_poll_mode
> qemu#19 0x00005586607888eb in aio_poll
> qemu#20 0x0000558660784561 in aio_wait_bh_oneshot
> qemu#21 0x00005586602b9582 in virtio_scsi_dataplane_stop
> qemu#22 0x00005586605a7110 in virtio_bus_stop_ioeventfd
> qemu#23 0x00005586605a9426 in virtio_pci_stop_ioeventfd
> qemu#24 0x00005586605ab808 in virtio_pci_common_write
> qemu#25 0x0000558660242396 in memory_region_write_accessor
> qemu#26 0x00005586602425ab in access_with_adjusted_size
> qemu#27 0x0000558660245281 in memory_region_dispatch_write
> qemu#28 0x00005586601e008e in flatview_write_continue
> qemu#29 0x00005586601e01d8 in flatview_write
> qemu#30 0x00005586601e04de in address_space_write
> qemu#31 0x00005586601e052f in address_space_rw
> qemu#32 0x00005586602607f2 in kvm_cpu_exec
> qemu#33 0x0000558660227148 in qemu_kvm_cpu_thread_fn
> qemu#34 0x000055866078bde7 in qemu_thread_start
> qemu#35 0x00007f5784906594 in start_thread
> qemu#36 0x00007f5784639e6f in clone
With this patch, virtio_queue_empty will now return 1 as soon as the
vdev is marked as broken, after a "virtio: zero sized buffers are not
allowed" error.
To be consistent, update virtio_queue_empty_rcu as well.
Signed-off-by: Fam Zheng <famz@redhat.com>
---
v2: - Drop ATS condition from the patch description since it is not
essential.
- Drop patch 2. [Paolo]
qemu-deploy
pushed a commit
that referenced
this pull request
Oct 3, 2018
Both virtio-blk and virtio-scsi use virtio_queue_empty() as the
loop condition in VQ handlers (virtio_blk_handle_vq,
virtio_scsi_handle_cmd_vq). When a device is marked broken in
virtqueue_pop, for example if a vIOMMU address translation failed, we
want to break out of the loop.
This fixes a hanging problem when booting a CentOS 3.10.0-862.el7.x86_64
kernel with ATS enabled:
$ qemu-system-x86_64 \
... \
-device intel-iommu,intremap=on,caching-mode=on,eim=on,device-iotlb=on \
-device virtio-scsi-pci,iommu_platform=on,ats=on,id=scsi0,bus=pci.4,addr=0x0
The dead loop happens immediately when the kernel boots and initializes
the device, where virtio_scsi_data_plane_handle_cmd will not return:
> ...
> #13 0x00005586602b7793 in virtio_scsi_handle_cmd_vq
> #14 0x00005586602b8d66 in virtio_scsi_data_plane_handle_cmd
> #15 0x00005586602ddab7 in virtio_queue_notify_aio_vq
> #16 0x00005586602dfc9f in virtio_queue_host_notifier_aio_poll
> #17 0x00005586607885da in run_poll_handlers_once
> #18 0x000055866078880e in try_poll_mode
> #19 0x00005586607888eb in aio_poll
> #20 0x0000558660784561 in aio_wait_bh_oneshot
> #21 0x00005586602b9582 in virtio_scsi_dataplane_stop
> #22 0x00005586605a7110 in virtio_bus_stop_ioeventfd
> #23 0x00005586605a9426 in virtio_pci_stop_ioeventfd
> #24 0x00005586605ab808 in virtio_pci_common_write
> #25 0x0000558660242396 in memory_region_write_accessor
> #26 0x00005586602425ab in access_with_adjusted_size
> #27 0x0000558660245281 in memory_region_dispatch_write
> #28 0x00005586601e008e in flatview_write_continue
> #29 0x00005586601e01d8 in flatview_write
> #30 0x00005586601e04de in address_space_write
> #31 0x00005586601e052f in address_space_rw
> #32 0x00005586602607f2 in kvm_cpu_exec
> #33 0x0000558660227148 in qemu_kvm_cpu_thread_fn
> #34 0x000055866078bde7 in qemu_thread_start
> #35 0x00007f5784906594 in start_thread
> #36 0x00007f5784639e6f in clone
With this patch, virtio_queue_empty will now return 1 as soon as the
vdev is marked as broken, after a "virtio: zero sized buffers are not
allowed" error.
To be consistent, update virtio_queue_empty_rcu as well.
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <20180910145616.8598-2-famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
pranith
pushed a commit
to pranith/qemu
that referenced
this pull request
Oct 8, 2018
Both virtio-blk and virtio-scsi use virtio_queue_empty() as the
loop condition in VQ handlers (virtio_blk_handle_vq,
virtio_scsi_handle_cmd_vq). When a device is marked broken in
virtqueue_pop, for example if a vIOMMU address translation failed, we
want to break out of the loop.
This fixes a hanging problem when booting a CentOS 3.10.0-862.el7.x86_64
kernel with ATS enabled:
$ qemu-system-x86_64 \
... \
-device intel-iommu,intremap=on,caching-mode=on,eim=on,device-iotlb=on \
-device virtio-scsi-pci,iommu_platform=on,ats=on,id=scsi0,bus=pci.4,addr=0x0
The dead loop happens immediately when the kernel boots and initializes
the device, where virtio_scsi_data_plane_handle_cmd will not return:
> ...
> qemu#13 0x00005586602b7793 in virtio_scsi_handle_cmd_vq
> qemu#14 0x00005586602b8d66 in virtio_scsi_data_plane_handle_cmd
> qemu#15 0x00005586602ddab7 in virtio_queue_notify_aio_vq
> qemu#16 0x00005586602dfc9f in virtio_queue_host_notifier_aio_poll
> qemu#17 0x00005586607885da in run_poll_handlers_once
> qemu#18 0x000055866078880e in try_poll_mode
> qemu#19 0x00005586607888eb in aio_poll
> qemu#20 0x0000558660784561 in aio_wait_bh_oneshot
> qemu#21 0x00005586602b9582 in virtio_scsi_dataplane_stop
> qemu#22 0x00005586605a7110 in virtio_bus_stop_ioeventfd
> qemu#23 0x00005586605a9426 in virtio_pci_stop_ioeventfd
> qemu#24 0x00005586605ab808 in virtio_pci_common_write
> qemu#25 0x0000558660242396 in memory_region_write_accessor
> qemu#26 0x00005586602425ab in access_with_adjusted_size
> qemu#27 0x0000558660245281 in memory_region_dispatch_write
> qemu#28 0x00005586601e008e in flatview_write_continue
> qemu#29 0x00005586601e01d8 in flatview_write
> qemu#30 0x00005586601e04de in address_space_write
> qemu#31 0x00005586601e052f in address_space_rw
> qemu#32 0x00005586602607f2 in kvm_cpu_exec
> qemu#33 0x0000558660227148 in qemu_kvm_cpu_thread_fn
> qemu#34 0x000055866078bde7 in qemu_thread_start
> qemu#35 0x00007f5784906594 in start_thread
> qemu#36 0x00007f5784639e6f in clone
With this patch, virtio_queue_empty will now return 1 as soon as the
vdev is marked as broken, after a "virtio: zero sized buffers are not
allowed" error.
To be consistent, update virtio_queue_empty_rcu as well.
Signed-off-by: Fam Zheng <famz@redhat.com>
Message-Id: <20180910145616.8598-2-famz@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
|
Thank you for your interest in the QEMU project. This repository is a read-only mirror of the project's master QEMU welcomes contributions of code (either fixing bugs or adding new |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.