s390x/pci: pass the retaddr to all PCI instructions
Once we wire up TCG, we will need the retaddr to correctly inject
program interrupts. As we want to get rid of the function
program_interrupt(), convert PCI code too.
For KVM, we can simply use RA_IGNORED.
Convert program_interrupt() to s390_program_interrupt() directly, making
use of the passed address.
valgrind pointed out that we call KVM_S390_GET_IRQ_STATE with an
undefined value for flags. Kernels prior to 4.15 did not use that
field, and later kernels ignore it for compatibility reasons, but we
better play safe.
The same is true for SET_IRQ_STATE. We should make sure to not use the
flag field, either.
Peter Maydell [Thu, 14 Dec 2017 14:22:17 +0000 (14:22 +0000)]
Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20171213' into staging
target-arm queue:
* xilinx_spips: set reset values correctly
* MAINTAINERS: fix an email address
* hw/display/tc6393xb: limit irq handler index to TC6393XB_GPIOS
* nvic: Make systick banked for v8M
* refactor get_phys_addr() so we can return the right format PAR
for ATS operations
* implement v8M TT instruction
* fix some minor v8M bugs
* Implement reset for GICv3 ITS
* xlnx-zcu102: Add support for the ZynqMP QSPI
* remotes/pmaydell/tags/pull-target-arm-20171213: (43 commits)
xilinx_spips: Use memset instead of a for loop to zero registers
xilinx_spips: Set all of the reset values
xilinx_spips: Update the QSPI Mod ID reset value
MAINTAINERS: replace the unavailable email address
hw/display/tc6393xb: limit irq handler index to TC6393XB_GPIOS
nvic: Make systick banked
nvic: Make nvic_sysreg_ns_ops work with any MemoryRegion
target/arm: Extend PAR format determination
target/arm: Remove fsr argument from get_phys_addr() and arm_tlb_fill()
target/arm: Ignore fsr from get_phys_addr() in do_ats_write()
target/arm: Use ARMMMUFaultInfo in deliver_fault()
target/arm: Convert get_phys_addr_pmsav8() to not return FSC values
target/arm: Convert get_phys_addr_pmsav7() to not return FSC values
target/arm: Convert get_phys_addr_pmsav5() to not return FSC values
target/arm: Convert get_phys_addr_lpae() to not return FSC values
target/arm: Convert get_phys_addr_v6() to not return FSC values
target/arm: Convert get_phys_addr_v5() to not return FSC values
target/arm: Remove fsr argument from arm_ld*_ptw()
target/arm: Provide fault type enum and FSR conversion functions
target/arm: Implement TT instruction
...
Vadim Galitsyn [Mon, 23 Oct 2017 15:13:10 +0000 (17:13 +0200)]
tests: test-hmp: print command execution result
Provide HMP monitor command execution result as it would be seen
by user who established an HMP monitor session.
Currently many commands may silently fail without any sign of that.
This patch let this info to be printed once test is running in
verbose mode.
For the future it might be useful to fail the test if command has
failed, however it would require a bit of rework inside test
engine itself.
A simple example of silent failure without reporting it would to
add some non-existent HMP command into 'hmp_cmds' list. In this case
test will report it successfully passed without error.
Thomas Huth [Thu, 30 Nov 2017 20:19:00 +0000 (21:19 +0100)]
hmp-commands: Remove the deprecated usb_add and usb_del
It's easy to use device_add and device_del as replacement instead.
The usb_add and usb_del commands are deprecated since QEMU 2.10,
and nobody complained that they are still needed, so let's get rid
of them now to make the HMP interface a little bit less overloaded.
hw/display/tc6393xb: limit irq handler index to TC6393XB_GPIOS
The ctz32() routine could return a value greater than
TC6393XB_GPIOS=16, because the device has 24 GPIO level
bits but we only implement 16 outgoing lines. This could
lead to an OOB array access. Mask 'level' to avoid it.
Peter Maydell [Wed, 13 Dec 2017 17:59:26 +0000 (17:59 +0000)]
nvic: Make systick banked
For the v8M security extension, there should be two systick
devices, which use separate banked systick exceptions. The
register interface is banked in the same way as for other
banked registers, including the existence of an NS alias
region for secure code to access the nonsecure timer.
Peter Maydell [Wed, 13 Dec 2017 17:59:26 +0000 (17:59 +0000)]
nvic: Make nvic_sysreg_ns_ops work with any MemoryRegion
Generalize nvic_sysreg_ns_ops so that we can pass it an
arbitrary MemoryRegion which it will use as the underlying
register implementation to apply the NS-alias behaviour
to. We'll want this so we can do the same with systick.
Now that do_ats_write() is entirely in control of whether to
generate a 32-bit PAR or a 64-bit PAR, we can make it use the
correct (complicated) condition for doing so.
Signed-off-by: Edgar E. Iglesias <[email protected]> Reviewed-by: Richard Henderson <[email protected]> Reviewed-by: Edgar E. Iglesias <[email protected]> Tested-by: Stefano Stabellini <[email protected]> Signed-off-by: Peter Maydell <[email protected]>
Message-id: 1512503192[email protected]
[PMM: Rebased Edgar's patch on top of get_phys_addr() refactoring;
use arm_s1_regime_using_lpae_format() rather than
regime_using_lpae_format() because the latter will assert
if passed ARMMMUIdx_S12NSE0 or ARMMMUIdx_S12NSE1;
updated commit message appropriately] Signed-off-by: Peter Maydell <[email protected]>
Peter Maydell [Wed, 13 Dec 2017 17:59:25 +0000 (17:59 +0000)]
target/arm: Ignore fsr from get_phys_addr() in do_ats_write()
In do_ats_write(), rather than using the FSR value from get_phys_addr(),
construct the PAR values using the information in the ARMMMUFaultInfo
struct. This allows us to create a PAR of the correct format regardless
of what the translation table format is.
For the moment we leave the condition for "when should this be a
64 bit PAR" as it was previously; this will need to be fixed to
properly support AArch32 Hyp mode.
Peter Maydell [Wed, 13 Dec 2017 17:59:25 +0000 (17:59 +0000)]
target/arm: Use ARMMMUFaultInfo in deliver_fault()
Now that ARMMMUFaultInfo is guaranteed to have enough information
to construct a fault status code, we can pass it in to the
deliver_fault() function and let it generate the correct type
of FSR for the destination, rather than relying on the value
provided by get_phys_addr().
I don't think there are any cases the old code was getting
wrong, but this is more obviously correct.
Peter Maydell [Wed, 13 Dec 2017 17:59:25 +0000 (17:59 +0000)]
target/arm: Convert get_phys_addr_pmsav5() to not return FSC values
Make get_phys_addr_pmsav5() return a fault type in the ARMMMUFaultInfo
structure, which we convert to the FSC at the callsite.
Note that PMSAv5 does not define any guest-visible fault status
register, so the different "fsr" values we were previously
returning are entirely arbitrary. So we can just switch to using
the most appropriae fi->type values without worrying that we
need to special-case FaultInfo->FSC conversion for PMSAv5.
Peter Maydell [Wed, 13 Dec 2017 17:59:24 +0000 (17:59 +0000)]
target/arm: Remove fsr argument from arm_ld*_ptw()
All the callers of arm_ldq_ptw() and arm_ldl_ptw() ignore the value
that those functions store in the fsr argument on failure: if they
return failure to their callers they will always overwrite the fsr
value with something else.
Remove the argument from these functions and S1_ptw_translate().
This will simplify removing fsr from the calling functions.
Peter Maydell [Wed, 13 Dec 2017 17:59:24 +0000 (17:59 +0000)]
target/arm: Provide fault type enum and FSR conversion functions
Currently get_phys_addr() and its various subfunctions return
a hard-coded fault status register value for translation
failures. This is awkward because FSR values these days may
be either long-descriptor format or short-descriptor format.
Worse, the right FSR type to use doesn't depend only on the
translation table being walked -- some cases, like fault
info reported to AArch32 EL2 for some kinds of ATS operation,
must be in long-descriptor format even if the translation
table being walked was short format. We can't get those cases
right with our current approach.
Provide fields in the ARMMMUFaultInfo struct which allow
get_phys_addr() to provide sufficient information for a caller to
construct an FSR value themselves, and utility functions which do
this for both long and short format FSR values, as a first step in
switching get_phys_addr() and its children to only returning the
failure cause in the ARMMMUFaultInfo struct.
Peter Maydell [Wed, 13 Dec 2017 17:59:24 +0000 (17:59 +0000)]
target/arm: Factor MPU lookup code out of get_phys_addr_pmsav8()
For the TT instruction we're going to need to do an MPU lookup that
also tells us which MPU region the access hit. This requires us
to do the MPU lookup without first doing the SAU security access
check, so pull the MPU lookup parts of get_phys_addr_pmsav8()
out into their own function.
The TT instruction also needs to know the MPU region number which
the lookup hit, so provide this information to the caller of the
MPU lookup code, even though get_phys_addr_pmsav8() doesn't
need to know it.
Peter Maydell [Wed, 13 Dec 2017 17:59:23 +0000 (17:59 +0000)]
target/arm: Create new arm_v7m_mmu_idx_for_secstate_and_priv()
The TT instruction is going to need to look up the MMU index
for a specified security and privilege state. Refactor the
existing arm_v7m_mmu_idx_for_secstate() into a version that
lets you specify the privilege state and one that uses the
current state of the CPU.
Peter Maydell [Wed, 13 Dec 2017 17:59:23 +0000 (17:59 +0000)]
target/arm: Split M profile MNegPri mmu index into user and priv
For M profile, we currently have an mmu index MNegPri for
"requested execution priority negative". This fails to
distinguish "requested execution priority negative, privileged"
from "requested execution priority negative, usermode", but
the two can return different results for MPU lookups. Fix this
by splitting MNegPri into MNegPriPriv and MNegPriUser, and
similarly for the Secure equivalent MSNegPri.
This takes us from 6 M profile MMU modes to 8, which means
we need to bump NB_MMU_MODES; this is OK since the point
where we are forced to reduce TLB sizes is 9 MMU modes.
(It would in theory be possible to stick with 6 MMU indexes:
{mpu-disabled,user,privileged} x {secure,nonsecure} since
in the MPU-disabled case the result of an MPU lookup is
always the same for both user and privileged code. However
we would then need to rework the TB flags handling to put
user/priv into the TB flags separately from the mmuidx.
Adding an extra couple of mmu indexes is simpler.)
Peter Maydell [Wed, 13 Dec 2017 17:59:23 +0000 (17:59 +0000)]
target/arm: Add missing M profile case to regime_is_user()
When we added the ARMMMUIdx_MSUser MMU index we forgot to
add it to the case statement in regime_is_user(), so we
weren't treating it as unprivileged when doing MPU lookups.
Correct the omission.
Peter Maydell [Wed, 13 Dec 2017 17:59:23 +0000 (17:59 +0000)]
target/arm: Allow explicit writes to CONTROL.SPSEL in Handler mode
In ARMv7M the CPU ignores explicit writes to CONTROL.SPSEL
in Handler mode. In v8M the behaviour is slightly different:
writes to the bit are permitted but will have no effect.
We've already done the hard work to handle the value in
CONTROL.SPSEL being out of sync with what stack pointer is
actually in use, so all we need to do to fix this last loose
end is to update the condition we use to guard whether we
call write_v7m_control_spsel() on the register write.
Peter Maydell [Wed, 13 Dec 2017 17:59:23 +0000 (17:59 +0000)]
target/arm: Handle SPSEL and current stack being out of sync in MSP/PSP reads
For v8M it is possible for the CONTROL.SPSEL bit value and the
current stack to be out of sync. This means we need to update
the checks used in reads and writes of the PSP and MSP special
registers to use v7m_using_psp() rather than directly checking
the SPSEL bit in the control register.
Eric Auger [Wed, 13 Dec 2017 17:59:23 +0000 (17:59 +0000)]
hw/intc/arm_gicv3_its: Implement full reset
Voiding the ITS caches is not supposed to happen via
individual register writes. So we introduced a dedicated
ITS KVM device ioctl to perform a cold reset of the ITS:
KVM_DEV_ARM_VGIC_GRP_CTRL/KVM_DEV_ARM_ITS_CTRL_RESET. Let's
use this latter if the kernel supports it.
Eric Auger [Wed, 13 Dec 2017 17:59:22 +0000 (17:59 +0000)]
hw/intc/arm_gicv3_its: Implement a minimalist reset
At the moment the ITS is not properly reset and this causes
various bugs on save/restore. We implement a minimalist reset
through individual register writes but for kernel versions
before v4.15 this fails voiding the vITS cache. We cannot
claim we have a comprehensive reset (hence the error message)
but that's better than nothing.
Eric Auger [Wed, 13 Dec 2017 17:59:22 +0000 (17:59 +0000)]
hw/intc/arm_gicv3_its: Don't call post_load on reset
From the very beginning, post_load() was called from common
reset. This is not standard and obliged to discriminate the
reset case from the restore case using the iidr value.
xilinx_spips: Add support for RX discard and RX drain
Add support for the RX discard and RX drain functionality. Also transmit
one byte per dummy cycle (to the flash memories) with commands that require
these.
xilinx_spips: Update striping to be big-endian bit order
Update striping functionality to be big-endian bit order (as according to
the Zynq-7000 Technical Reference Manual). Output thereafter the even bits
into the flash memory connected to the lower QSPI bus and the odd bits into
the flash memory connected to the upper QSPI bus.
xilinx_spips: Move FlashCMD, XilinxQSPIPS and XilinxSPIPSClass
Move the FlashCMD enum, XilinxQSPIPS and XilinxSPIPSClass structures to the
header for consistency (struct XilinxSPIPS is found there). Also move out
a define and remove two double included headers (while touching the code).
Finally, add 4 byte address commands to the FlashCMD enum.
m25p80: Add support for continuous read out of RDSR and READ_FSR
Add support for continuous read out of the RDSR and READ_FSR status
registers until the chip select is deasserted. This feature is supported
by amongst others 1 or more flashtypes manufactured by Numonyx (Micron),
Windbond, SST, Gigadevice, Eon and Macronix.
The vfio_iommu_spapr_tce driver advertises kernel's support for
v1 and v2 IOMMU support, however it is not always possible to use
the requested IOMMU type. For example, a pseries host platform does not
support dynamic DMA windows so v2 cannot initialize and QEMU fails to
start.
This adds a fallback to the v1 IOMMU if v2 cannot be used.
Alex Williamson [Wed, 13 Dec 2017 17:19:32 +0000 (10:19 -0700)]
vfio: Fix vfio-kvm group registration
Commit 8c37faa475f3 ("vfio-pci, ppc64/spapr: Reorder group-to-container
attaching") moved registration of groups with the vfio-kvm device from
vfio_get_group() to vfio_connect_container(), but it missed the case
where a group is attached to an existing container and takes an early
exit. Perhaps this is a less common case on ppc64/spapr, but on x86
(without viommu) all groups are connected to the same container and
thus only the first group gets registered with the vfio-kvm device.
This becomes a problem if we then hot-unplug the devices associated
with that first group and we end up with KVM being misinformed about
any vfio connections that might remain. Fix by including the call to
vfio_kvm_device_add_group() in this early exit path.
Peter Maydell [Mon, 11 Dec 2017 15:42:44 +0000 (15:42 +0000)]
target/arm: Generate UNDEF for 32-bit Thumb2 insns
The refactoring of commit 296e5a0a6c3935 has a nasty bug:
it accidentally dropped the generation of code to raise
the UNDEF exception when disas_thumb2_insn() returns nonzero.
This means that 32-bit Thumb2 instruction patterns that
ought to UNDEF just act like nops instead. This is likely
to break any number of things, including the kernel's "disable
the FPU and use the UNDEF exception to identify when to turn
it back on again" trick.
Eric Farman [Fri, 1 Dec 2017 15:15:38 +0000 (16:15 +0100)]
vhost-scsi: add missing virtqueue_size parameter
Commit 5c0919d02066 ("virtio-scsi: Add virtqueue_size parameter allowing
virtqueue size to be set.") introduced a new parameter to virtio-scsi.
Later, commit 920036106044 ("vhost-user-scsi: add missing virtqueue_size
param") added that parameter to the new vhost-user-scsi interface but
neglected the existing vhost-scsi interface it was built on.
Apply the same change to vhost-scsi, so that we can boot a guest with
a device defined. This also avoids crashing a guest when hotplugging
a vhost-scsi device.
Peter Maydell [Tue, 5 Dec 2017 10:00:48 +0000 (10:00 +0000)]
Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.11-20171205' into staging
ppc patch queue 2017-12-05
Alas, this is yet another fix for ppc that I think it's worth
squeezing into 2.11. It's a really ugly fix for some pretty ugly
code, but it does seem to address a real problem. It's also a problem
that's appeared relatively recently, since it was either created by,
or made much easier to trigger by, by the merge of MTTCG.
The "00000004" is CPU_INTERRUPT_EXITTB yet the code calls
cpu_interrupt(cs, CPU_INTERRUPT_HARD) ("00000002") in this function
just before the log message. Something is causing the HARD bit setting
to get lost.
The knock on effect of losing that bit is the decrementer timer interrupts
don't get delivered which causes the guest to sit idle in its idle handler
and 'hang'.
The issue occurs due to races from code which sets CPU_INTERRUPT_EXITTB.
Rather than poking directly into cs->interrupt_request, that code needs to:
a) hold BQL
b) use the cpu_interrupt() helper
This patch fixes the call sites to do this, fixing the hang. The calls
are made from a variety of contexts so a helper function is added to handle
the necessary locking. This can likely be improved and optimised in the future
but it ensures the code is correct and doesn't lockup as it stands today.
Alberto Garcia [Wed, 29 Nov 2017 17:56:34 +0000 (19:56 +0200)]
blockjob: Make block_job_pause_all() keep a reference to the jobs
Starting from commit 40840e419be31e6a32e6ea24511c74b389d5e0e4 we are
pausing all block jobs during bdrv_reopen_multiple() to prevent any of
them from finishing and removing nodes from the graph while they are
being reopened.
It turns out that pausing a block job doesn't necessarily prevent it
from finishing: a paused block job can still run its exit function
from the main loop and call block_job_completed(). The mirror block
job in particular always goes to the main loop while it is paused (by
virtue of the bdrv_drained_begin() call in mirror_run()).
Destroying a paused block job during bdrv_reopen_multiple() has two
consequences:
1) The references to the nodes involved in the job are released,
possibly destroying some of them. If those nodes were in the
reopen queue this would trigger the problem originally described
in commit 40840e419be, crashing QEMU.
2) At the end of bdrv_reopen_multiple(), bdrv_drain_all_end() would
not be doing all necessary bdrv_parent_drained_end() calls.
I can reproduce problem 1) easily with iotest 030 by increasing
STREAM_BUFFER_SIZE from 512KB to 8MB in block/stream.c, or by tweaking
the iotest like in this example:
This patch keeps an additional reference to all block jobs between
block_job_pause_all() and block_job_resume_all(), guaranteeing that
they are kept alive.
Peter Maydell [Mon, 4 Dec 2017 13:08:13 +0000 (13:08 +0000)]
Merge remote-tracking branch 'remotes/mst/tags/for_upstream' into staging
pc, pci, virtio: fixes for rc3
A bunch of fixes all over the place.
Signed-off-by: Michael S. Tsirkin <[email protected]>
# gpg: Signature made Fri 01 Dec 2017 17:06:33 GMT
# gpg: using RSA key 0x281F0DB8D28D5469
# gpg: Good signature from "Michael S. Tsirkin <[email protected]>"
# gpg: aka "Michael S. Tsirkin <[email protected]>"
# 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:
pc: fix crash on attempted cpu unplug
virtio: check VirtQueue Vring object is set
vhost: fix error check in vhost_verify_ring_mappings()
dump-guest-memory.py: fix No symbol "vmcoreinfo_find"
vhost: restore avail index from vring used index on disconnection
virtio: Add queue interface to restore avail index from vring used index
i386/msi: Correct mask of destination ID in MSI address
Peter Maydell [Mon, 4 Dec 2017 11:27:53 +0000 (11:27 +0000)]
Merge remote-tracking branch 'remotes/dgibson/tags/ppc-for-2.11-20171204' into staging
ppc patch queue 2017-12-04
We are, alas, not yet to the bottom of ppc bugs. This pull request
fixes several more. I believe they're important enough to include in
2.11. despite the late date.
David Gibson [Fri, 1 Dec 2017 05:05:33 +0000 (16:05 +1100)]
spapr: Include "pre-plugged" DIMMS in ram size calculation at reset
At guest reset time, we allocate a hash page table (HPT) for the guest
based on the guest's RAM size. If dynamic HPT resizing is not available we
use the maximum RAM size, if it is we use the current RAM size.
But the "current RAM size" calculation is incorrect - we just use the
"base" ram_size from the machine structure. This doesn't include any
pluggable DIMMs that are already plugged at reset time.
This means that if you try to start a 'pseries' machine with a DIMM
specified on the command line that's much larger than the "base" RAM size,
then the guest will get a woefully inadequate HPT. This can lead to a
guest freeze during boot as it runs out of HPT space during initial MMU
setup.
which is caused by pcms->acpi_dev == NULL due to ACPI support
being disabled.
Considering that ACPI support is necessary for unplug to work,
check that it's enabled and fail unplug request gracefully
if no acpi device were found.
Greg Kurz [Thu, 30 Nov 2017 21:39:59 +0000 (22:39 +0100)]
vhost: fix error check in vhost_verify_ring_mappings()
Since commit f1f9e6c5 "vhost: adapt vhost_verify_ring_mappings() to
virtio 1 ring layout", we check the mapping of each part (descriptor
table, available ring and used ring) of each virtqueue separately.
The checking of a part is done by the vhost_verify_ring_part_mapping()
function: it returns either 0 on success or a negative errno if the
part cannot be mapped at the same place.
Unfortunately, the vhost_verify_ring_mappings() function checks its
return value the other way round. It means that we either:
- only verify the descriptor table of the first virtqueue, and if it
is valid we ignore all the other mappings
- or ignore all broken mappings until we reach a valid one
ie, we only raise an error if all mappings are broken, and we consider
all mappings are valid otherwise (false success), which is obviously
wrong.
This patch ensures that vhost_verify_ring_mappings() only returns
success if ALL mappings are okay.
Chao Gao [Fri, 17 Nov 2017 06:24:23 +0000 (14:24 +0800)]
i386/msi: Correct mask of destination ID in MSI address
According to SDM 10.11.1, only [19:12] bits of MSI address are
Destination ID, change the mask to avoid ambiguity for VT-d spec
has used the bit 4 to indicate a remappable interrupt request.
The msr invalidation code (commits 993eb and 2360b) inverts all
bits except MSR_TGPR and MSR_HVB. On non PowerPC 601 processors
this leads to incorrect change of excp_prefix in hreg_store_msr()
function. The problem is that new msr value get multiplied by msr_mask
and inverted msr does not, thus values of MSR_EP bit in new msr value
and inverted msr are distinct, so that excp_prefix changes but should
not.
Paolo Bonzini [Wed, 29 Nov 2017 10:25:13 +0000 (11:25 +0100)]
blockjob: reimplement block_job_sleep_ns to allow cancellation
This reverts the effects of commit 4afeffc857 ("blockjob: do not allow
coroutine double entry or entry-after-completion", 2017-11-21)
This fixed the symptom of a bug rather than the root cause. Canceling the
wait on a sleeping blockjob coroutine is generally fine, we just need to
make it work correctly across AioContexts. To do so, use a QEMUTimer
that calls block_job_enter. Use a mutex to ensure that block_job_enter
synchronizes correctly with block_job_sleep_ns.
Paolo Bonzini [Wed, 29 Nov 2017 10:25:12 +0000 (11:25 +0100)]
blockjob: introduce block_job_do_yield
Hide the clearing of job->busy in a single function, and set it
in block_job_enter. This lets block_job_do_yield verify that
qemu_coroutine_enter is not used while job->busy = false.
Paolo Bonzini [Wed, 29 Nov 2017 10:25:11 +0000 (11:25 +0100)]
blockjob: remove clock argument from block_job_sleep_ns
All callers are using QEMU_CLOCK_REALTIME, and it will not be possible to
support more than one clock when block_job_sleep_ns switches to a single
timer stored in the BlockJob struct.
Kevin Wolf [Wed, 29 Nov 2017 10:25:10 +0000 (11:25 +0100)]
block: Expect graph changes in bdrv_parent_drained_begin/end
The .drained_begin/end callbacks can (directly or indirectly via
aio_poll()) cause block nodes to be removed or the current BdrvChild to
point to a different child node.
Use QLIST_FOREACH_SAFE() to make sure we don't access invalid
BlockDriverStates or accidentally continue iterating the parents of the
new child node instead of the node we actually came from.
Alberto Garcia [Tue, 28 Nov 2017 14:53:27 +0000 (16:53 +0200)]
blockjob: Remove the job from the list earlier in block_job_unref()
When destroying a block job in block_job_unref() we should remove it
from the job list before calling block_job_remove_all_bdrv().
This is because removing the BDSs can trigger an aio_poll() and wake
up other jobs that might attempt to use the block job list. If that
happens the job we're currently destroying should not be in that list
anymore.
Peter Maydell [Tue, 28 Nov 2017 13:12:48 +0000 (13:12 +0000)]
Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2017-11-28' into staging
nbd patches for 2017-11-28
Eric Blake - 0/2 fix two NBD server CVEs
# gpg: Signature made Tue 28 Nov 2017 12:58:29 GMT
# gpg: using RSA key 0xA7A16B4A2527436A
# gpg: Good signature from "Eric Blake <[email protected]>"
# gpg: aka "Eric Blake (Free Software Programmer) <[email protected]>"
# gpg: aka "[jpeg image of size 6874]"
# Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2 F3AA A7A1 6B4A 2527 436A
* remotes/ericb/tags/pull-nbd-2017-11-28:
nbd/server: CVE-2017-15118 Stack smash on large export name
nbd/server: CVE-2017-15119 Reject options larger than 32M
Eric Blake [Wed, 22 Nov 2017 21:07:22 +0000 (15:07 -0600)]
nbd/server: CVE-2017-15118 Stack smash on large export name
Introduced in commit f37708f6b8 (2.10). The NBD spec says a client
can request export names up to 4096 bytes in length, even though
they should not expect success on names longer than 256. However,
qemu hard-codes the limit of 256, and fails to filter out a client
that probes for a longer name; the result is a stack smash that can
potentially give an attacker arbitrary control over the qemu
process.
The smash can be easily demonstrated with this client:
$ qemu-io f raw nbd://localhost:10809/$(printf %3000d 1 | tr ' ' a)
If the qemu NBD server binary (whether the standalone qemu-nbd, or
the builtin server of QMP nbd-server-start) was compiled with
-fstack-protector-strong, the ability to exploit the stack smash
into arbitrary execution is a lot more difficult (but still
theoretically possible to a determined attacker, perhaps in
combination with other CVEs). Still, crashing a running qemu (and
losing the VM) is bad enough, even if the attacker did not obtain
full execution control.
Eric Blake [Wed, 22 Nov 2017 22:25:16 +0000 (16:25 -0600)]
nbd/server: CVE-2017-15119 Reject options larger than 32M
The NBD spec gives us permission to abruptly disconnect on clients
that send outrageously large option requests, rather than having
to spend the time reading to the end of the option. No real
option request requires that much data anyways; and meanwhile, we
already have the practice of abruptly dropping the connection on
any client that sends NBD_CMD_WRITE with a payload larger than 32M.
For comparison, nbdkit drops the connection on any request with
more than 4096 bytes; however, that limit is probably too low
(as the NBD spec states an export name can theoretically be up
to 4096 bytes, which means a valid NBD_OPT_INFO could be even
longer) - even if qemu doesn't permit exports longer than 256
bytes.
It could be argued that a malicious client trying to get us to
read nearly 4G of data on a bad request is a form of denial of
service. In particular, if the server requires TLS, but a client
that does not know the TLS credentials sends any option (other
than NBD_OPT_STARTTLS or NBD_OPT_EXPORT_NAME) with a stated
payload of nearly 4G, then the server was keeping the connection
alive trying to read all the payload, tying up resources that it
would rather be spending on a client that can get past the TLS
handshake. Hence, this warranted a CVE.
Present since at least 2.5 when handling known options, and made
worse in 2.6 when fixing support for NBD_FLAG_C_FIXED_NEWSTYLE
to handle unknown options.
sockets: avoid crash when cleaning up sockets for an invalid FD
If socket_listen_cleanup is passed an invalid FD, then querying the socket
local address will fail. We must thus be prepared for the returned addr to
be NULL
Peter Maydell [Tue, 28 Nov 2017 10:03:26 +0000 (10:03 +0000)]
Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into staging
# gpg: Signature made Tue 28 Nov 2017 03:58:11 GMT
# gpg: using RSA key 0xEF04965B398D6211
# gpg: Good signature from "Jason Wang (Jason Wang on RedHat) <[email protected]>"
# gpg: WARNING: This key is not certified with sufficiently trusted signatures!
# gpg: It is not certain that the signature belongs to the owner.
# Primary key fingerprint: 215D 46F4 8246 689E C77F 3562 EF04 965B 398D 6211
* remotes/jasowang/tags/net-pull-request:
virtio-net: don't touch virtqueue if vm is stopped
Jason Wang [Wed, 22 Nov 2017 09:57:19 +0000 (17:57 +0800)]
virtio-net: don't touch virtqueue if vm is stopped
Guest state should not be touched if VM is stopped, unfortunately we
didn't check running state and tried to drain tx queue unconditionally
in virtio_net_set_status(). A crash was then noticed as a migration
destination when user type quit after virtqueue state is loaded but
before region cache is initialized. In this case,
virtio_net_drop_tx_queue_data() tries to access the uninitialized
region cache.
Fix this by only dropping tx queue data when vm is running.
projects / qemu.git / log