Anthony PERARD [Fri, 30 Apr 2021 16:37:42 +0000 (17:37 +0100)]
xen: Free xenforeignmemory_resource at exit
Because Coverity complains about it and this is one leak that Valgrind
reports.
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com> Acked-by: Paul Durrant <paul@xen.org>
Message-Id: <20210430163742.469739-1-anthony.perard@citrix.com> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
(cherry picked from commit f1e43b6026500690fc402828fa7cc735175b93b6)
Igor Druzhinin [Tue, 20 Apr 2021 03:35:02 +0000 (04:35 +0100)]
xen-mapcache: avoid a race on memory map while using MAP_FIXED
When we're replacing the existing mapping there is possibility of a race
on memory map with other threads doing mmap operations - the address being
unmapped/re-mapped could be occupied by another thread in between.
Linux mmap man page recommends keeping the existing mappings in place to
reserve the place and instead utilize the fact that the next mmap operation
with MAP_FIXED flag passed will implicitly destroy the existing mappings
behind the chosen address. This behavior is guaranteed by POSIX / BSD and
therefore is portable.
Note that it wouldn't make the replacement atomic for parallel accesses to
the replaced region - those might still fail with SIGBUS due to
xenforeignmemory_map not being atomic. So we're still not expecting those.
Tested-by: Anthony PERARD <anthony.perard@citrix.com> Signed-off-by: Igor Druzhinin <igor.druzhinin@citrix.com> Reviewed-by: Paul Durrant <paul@xen.org>
Message-Id: <1618889702-13104-1-git-send-email-igor.druzhinin@citrix.com> Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
(cherry picked from commit 3e81a71c9f3d23002b1e0dfff902c155d6c8d224)
Ian Jackson [Thu, 26 May 2016 15:21:56 +0000 (16:21 +0100)]
main loop: Big hammer to fix logfile disk DoS in Xen setups
Each time round the main loop, we now fstat stderr. If it is too big,
we dup2 /dev/null onto it. This is not a very pretty patch but it is
very simple, easy to see that it's correct, and has a low risk of
collateral damage.
There is no limit by default but can be adjusted by setting a new
environment variable.
This fixes CVE-2014-3672.
Signed-off-by: Ian Jackson <Ian.Jackson@eu.citrix.com> Tested-by: Ian Jackson <Ian.Jackson@eu.citrix.com>
Set the default to 0 so that it won't affect non-xen installation. The
limit will be set by Xen toolstack.
Klaus Jensen [Fri, 23 Apr 2021 05:21:26 +0000 (07:21 +0200)]
hw/block/nvme: fix invalid msix exclusive uninit
Commit 1901b4967c3f changed the nvme device from using a bar exclusive
for MSI-x to sharing it on bar0.
Unfortunately, the msix_uninit_exclusive_bar() call remains in
nvme_exit() which causes havoc when the device is removed with, say,
device_del. Fix this.
Additionally, a subregion is added but it is not removed on exit which
causes a reference to linger and the drive to never be unlocked.
Fixes: 1901b4967c3f ("hw/block/nvme: move msix table and pba to BAR 0") Signed-off-by: Klaus Jensen <k.jensen@samsung.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Alex Bennée [Thu, 22 Apr 2021 15:44:27 +0000 (16:44 +0100)]
target/s390x: fix s390_probe_access to check PAGE_WRITE_ORG for writeability
We can remove PAGE_WRITE when (internally) marking a page read-only
because it contains translated code. This can get confused when we are
executing signal return code on signal stacks.
Fixes: e56552cf07 ("target/s390x: Implement the MVPG condition-code-option bit") Found-by: Richard Henderson <richard.henderson@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: David Hildenbrand <david@redhat.com>
Message-id: 20210422154427.13038-1-alex.bennee@linaro.org Cc: Cornelia Huck <cohuck@redhat.com> Cc: Thomas Huth <thuth@redhat.com> Cc: David Hildenbrand <david@redhat.com> Cc: Laurent Vivier <laurent@vivier.eu> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Jason Wang [Fri, 23 Apr 2021 03:18:03 +0000 (11:18 +0800)]
net: check the existence of peer before trying to pad
There could be case that peer is NULL. This can happen when during
network device hot-add where net device needs to be added first. So
the patch check the existence of peer before trying to do the pad.
Fixes: 969e50b61a285 ("net: Pad short frames to minimum size before sending from SLiRP/TAP") Signed-off-by: Jason Wang <jasowang@redhat.com> Reviewed-by: Bin Meng <bmeng.cn@gmail.com> Reviewed-by: Stefan Weil <sw@weilnetz.de>
Message-id: 20210423031803.1479-1-jasowang@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Commit 54aa3de72 ("qapi: Use QAPI_LIST_PREPEND() where possible")
inadvertently removed the has_dependencies from the partition disk
info, resulting in empty list being returned.
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1950833 Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com>
Message-id: 20210420125831.233092-1-marcandre.lureau@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
target/mips/rel6_translate: Change license to GNU LGPL v2.1 (or later)
When adding this file and its new content in commit 3f7a927847a
("target/mips: LSA/DLSA R6 decodetree helpers") I did 2 mistakes:
1: Listed authors who haven't been involved in its development,
2: Used an incorrect GNU GPLv2 license text (using 'and' instead
of 'or').
Instead of correcting the GNU GPLv2 license text, replace the license
by the 'GNU LGPL v2.1 or later' one, to be coherent with the other
translation files in the target/mips/ folder.
Suggested-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 20210420100633.1752440-1-f4bug@amsat.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
migration: Deprecate redundant query-migrate result @blocked
Result @blocked is true when and only when result @blocked-reasons is
present. It's always non-empty when present. @blocked is redundant.
It was introduced in commit 3af8554bd0 "migration: Add blocker
information", and has not been released. This gives us a chance to
fix the interface with minimal fuss.
Unfortunately, we're already too close to the release to risk dropping
it. Deprecate it instead.
Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-id: 20210420051907.891470-1-armbru@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Michael Tokarev [Mon, 19 Apr 2021 13:42:47 +0000 (15:42 +0200)]
mptsas: Remove unused MPTSASState 'pending' field (CVE-2021-3392)
While processing SCSI i/o requests in mptsas_process_scsi_io_request(),
the Megaraid emulator appends new MPTSASRequest object 'req' to
the 's->pending' queue. In case of an error, this same object gets
dequeued in mptsas_free_request() only if SCSIRequest object
'req->sreq' is initialised. This may lead to a use-after-free issue.
Since s->pending is actually not used, simply remove it from
MPTSASState.
Cc: qemu-stable@nongnu.org Signed-off-by: Michael Tokarev <mjt@tls.msk.ru> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reported-by: Cheolwoo Myung <cwmyung@snu.ac.kr>
Message-id: 20210419134247.1467982-1-f4bug@amsat.org
Message-Id: <20210416102243.1293871-1-mjt@msgid.tls.msk.ru> Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Reported-by: Cheolwoo Myung <cwmyung@snu.ac.kr> BugLink: https://bugs.launchpad.net/qemu/+bug/1914236 Fixes: e351b826112 ("hw: Add support for LSI SAS1068 (mptsas) device")
[PMD: Reworded description, added more tags] Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Sat, 17 Apr 2021 19:47:32 +0000 (20:47 +0100)]
Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20210417' into staging
Fixes for rc4:
* Fix compile failures of C++ files with new glib headers
* mps3-an547: Use correct Cortex-M55 CPU and don't disable its FPU
* accel/tcg: Fix assertion failure executing from non-RAM with -icount
* remotes/pmaydell/tags/pull-target-arm-20210417:
accel/tcg: avoid re-translating one-shot instructions
target/arm: drop CF_LAST_IO/dc->condjump check
hw/arm/armsse: Make SSE-300 use Cortex-M55
hw/arm/armsse: Give SSE-300 its own Property array
include/qemu/osdep.h: Move system includes to top
osdep: protect qemu/osdep.h with extern "C"
osdep: include glib-compat.h before other QEMU headers
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
By definition a single instruction is capable of being an IO
instruction. This avoids a problem of triggering a cpu_io_recompile on
a non-recorded translation which then fails because it expects
tcg_tb_lookup() to succeed unconditionally. The normal use case
requires a TB to be able to resolve machine state.
The other users of tcg_tb_lookup() are able to tolerate a missing TB
if the machine state has been resolved by other means - which in the
single-shot case is always true because machine state is synced at the
start of a block.
Reported-by: Peter Maydell <peter.maydell@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20210415162454.22056-1-alex.bennee@linaro.org Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Alex Bennée [Fri, 16 Apr 2021 17:02:07 +0000 (18:02 +0100)]
target/arm: drop CF_LAST_IO/dc->condjump check
This is a left over erroneous check from the days front-ends handled
io start/end themselves. Regardless just because IO could be performed
on the last instruction doesn't obligate the front end to do so.
This fixes an abort faced by the aspeed execute-in-place support which
will necessarily trigger this state (even before the one-shot
CF_LAST_IO fix). The test still seems to hang once it attempts to boot
the Linux kernel but I suspect this is an unrelated issue with icount
and the timer handling code.
The original intention of the cpu_abort (added in commit 2e70f6efa8b9
when the icount stuff was first added) seems to have been to act as
an assert() to catch an unhandled corner case where the generated code
would be something like:
conditional branch to condlabel if its cc failed
implementation of the insn (a conditional branch or trap)
code emitted by gen_io_end()
condlabel:
gen_goto_tb or equivalent thing to go to next insn
At runtime the cc-failed case would skip over the code emitted by
gen_io_end(), leaving the can_do_io flag incorrectly set.
In commit ba3e7926691ed33 we switched to an implementation which
always clears can_do_io at the start of the following TB instead
of trying to clear it at the end of a TB that did IO. So the corner
case that this cpu_abort() was trying to flag is no longer possible,
because the gen_io_end() call has been deleted. We can therefore
safely remove the no-longer-valid assertion.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 20210416170207.12504-1-alex.bennee@linaro.org Cc: Cédric Le Goater <clg@kaod.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Fri, 16 Apr 2021 10:40:10 +0000 (11:40 +0100)]
hw/arm/armsse: Make SSE-300 use Cortex-M55
The SSE-300 has a Cortex-M55 (which was the whole reason for us
modelling it), but we forgot to actually update the code to let it
have a different CPU type from the IoTKit and SSE-200. Add CPU type
as a field for ARMSSEInfo instead of hardcoding it to always use a
Cortex-M33.
Buglink: https://bugs.launchpad.net/qemu/+bug/1923861 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20210416104010.13228-1-peter.maydell@linaro.org
Peter Maydell [Thu, 15 Apr 2021 18:23:53 +0000 (19:23 +0100)]
hw/arm/armsse: Give SSE-300 its own Property array
SSE-300 currently shares the SSE-200 Property array. This is
bad principally because the default values of the CPU0_FPU
and CPU0_DSP properties disable the FPU and DSP on the CPU.
That is correct for the SSE-200 but not the SSE-300.
Give the SSE-300 its own Property array with the correct
SSE-300 specific settings:
* SSE-300 has only one CPU, so no CPU1* properties
* SSE-300 CPU has FPU and DSP
Buglink: https://bugs.launchpad.net/qemu/+bug/1923861 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20210415182353.8173-1-peter.maydell@linaro.org
Peter Maydell [Fri, 16 Apr 2021 13:55:40 +0000 (14:55 +0100)]
include/qemu/osdep.h: Move system includes to top
Mostly osdep.h puts the system includes at the top of the file; but
there are a couple of exceptions where we include a system header
halfway through the file. Move these up to the top with the rest
so that all the system headers we include are included before
we include os-win32.h or os-posix.h.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20210416135543.20382-4-peter.maydell@linaro.org
Message-id: 20210414184343.26235-1-peter.maydell@linaro.org
Paolo Bonzini [Fri, 16 Apr 2021 13:55:39 +0000 (14:55 +0100)]
osdep: protect qemu/osdep.h with extern "C"
System headers may include templates if compiled with a C++ compiler,
which cause the compiler to complain if qemu/osdep.h is included
within a C++ source file's 'extern "C"' block. Add
an 'extern "C"' block directly to qemu/osdep.h, so that
system headers can be kept out of it.
There is a stray declaration early in qemu/osdep.h, which needs
to be special cased. Add a definition in qemu/compiler.h to
make it look nice.
config-host.h, CONFIG_TARGET, exec/poison.h and qemu/compiler.h
are included outside the 'extern "C"' block; that is not
an issue because they consist entirely of preprocessor directives.
This allows us to move the include of osdep.h in our two C++
source files outside the extern "C" block they were previously
using for it, which in turn means that they compile successfully
against newer versions of glib which insist that glib.h is
*not* inside an extern "C" block.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20210416135543.20382-3-peter.maydell@linaro.org
[PMM: Moved disas/arm-a64.cc osdep.h include out of its extern "C" block;
explained in commit message why we're doing this] Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Paolo Bonzini [Fri, 16 Apr 2021 13:55:38 +0000 (14:55 +0100)]
osdep: include glib-compat.h before other QEMU headers
glib-compat.h is sort of like a system header, and it needs to include
system headers (glib.h) that may dislike being included under
'extern "C"'. Move it right after all system headers and before
all other QEMU headers.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> Acked-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20210416135543.20382-2-peter.maydell@linaro.org
[PMM: Added comment about why glib-compat.h is special] Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Thomas Huth [Mon, 12 Apr 2021 16:07:09 +0000 (18:07 +0200)]
qapi/qom.json: Do not use CONFIG_VIRTIO_CRYPTO in common code
The ObjectType enum and ObjectOptions are included from qapi-types-qom.h
into common code. We should not use target-specific config switches like
CONFIG_VIRTIO_CRYPTO here, since this is not defined in common code and
thus the enum will look differently between common and target specific
code. For this case, it's hopefully enough to check for CONFIG_VHOST_CRYPTO
only (which is a host specific config switch, i.e. it's the same on all
targets).
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20210412160710.639800-1-thuth@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
* remotes/pmaydell/tags/pull-target-arm-20210413:
sphinx: qapidoc: Wrap "If" section body in a paragraph node
hw/arm/mps2-tz: Assert if more than one RAM is attached to an MPC
hw/arm/mps2-tz: Fix MPC setting for AN524 SRAM block
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Anton Kuchin [Thu, 8 Apr 2021 19:55:34 +0000 (22:55 +0300)]
vhost-user-fs: fix features handling
Make virtio-fs take into account server capabilities.
Just returning requested features assumes they all of then are implemented
by server and results in setting unsupported configuration if some of them
are absent.
Signed-off-by: Anton Kuchin <antonkuchin@yandex-team.ru> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
With changes suggested by Stefan Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
block/nbd: fix possible use after free of s->connect_thread
If on nbd_close() we detach the thread (in
nbd_co_establish_connection_cancel() thr->state becomes
CONNECT_THREAD_RUNNING_DETACHED), after that point we should not use
s->connect_thread (which is set to NULL), as running thread may free it
at any time.
Still nbd_co_establish_connection() does exactly this: it saves
s->connect_thread to local variable (just for better code style) and
use it even after yield point, when thread may be already detached.
Fix that. Also check thr to be non-NULL on
nbd_co_establish_connection() start for safety.
After this patch "case CONNECT_THREAD_RUNNING_DETACHED" becomes
impossible in the second switch in nbd_co_establish_connection().
Still, don't add extra abort() just before the release. If it somehow
possible to reach this "case:" it won't hurt. Anyway, good refactoring
of all this reconnect mess will come soon.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210406155114.1057355-1-vsementsov@virtuozzo.com> Reviewed-by: Roman Kagan <rvkagan@yandex-team.ru> Signed-off-by: Max Reitz <mreitz@redhat.com>
Peter Maydell [Tue, 13 Apr 2021 12:05:07 +0000 (13:05 +0100)]
Merge remote-tracking branch 'remotes/dg-gitlab/tags/ppc-for-6.0-20210412' into staging
ppc patch queue for 2021-04-21
Here's what I hope is the last ppc related pull request for qemu-6.0.
The 2 patches here revert a behavioural change that after further
discussion we concluded was a bad idea (adding a timeout for
possibly-failed hot unplug requests). Instead it implements a
different approach to the original problem: we again let unplug
requests the guest doesn't respond to remain pending indefinitely, but
no longer allow those to block attempts to retry the same unplug
again.
The change is a bit more complex than I'd like for this late in the
freeze. Nonetheless, I think it's important to merge this for 6.0, so
we don't allow a release which has the probably-a-bad-idea timeout
behaviour.
target/mips: Fix TCG temporary leak in gen_cache_operation()
Fix a TCG temporary leak when translating CACHE opcode.
Fixes: 0d74a222c27 ("make ITC Configuration Tags accessible to the CPU") Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20210406202857.1440744-1-f4bug@amsat.org>
When adding the Reset register in commit 5790b757cfb we
forgot to migrate it.
While it is possible a VM using the PIIX4 is migrated just
after requesting a system shutdown, it is very unlikely.
However when restoring a migrated VM, we might have the
RCR bit #4 set on the stack and when the VM resume it
directly shutdowns.
Add a post_load() migration handler and set the default
RCR value to 0 for earlier versions, assuming the VM was
not going to shutdown before migration.
Fixes: 5790b757cfb ("piix4: Add the Reset Control Register") Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20210324200334.729899-1-f4bug@amsat.org>
* remotes/mcayland/tags/qemu-sparc-20210412:
tests/qtest: add tests for am53c974 device
esp: ensure that do_cmd is set to zero before submitting an ESP select command
esp: don't reset async_len directly in esp_select() if cancelling request
esp: don't overflow cmdfifo if TC is larger than the cmdfifo size
esp: don't overflow cmdfifo in get_cmd()
esp: don't underflow cmdfifo in do_cmd()
esp: ensure cmdfifo is not empty and current_dev is non-NULL
esp: introduce esp_fifo_pop_buf() and use it instead of fifo8_pop_buf()
esp: consolidate esp_cmdfifo_pop() into esp_fifo_pop()
esp: consolidate esp_cmdfifo_push() into esp_fifo_push()
esp: rework write_response() to avoid using the FIFO for DMA transactions
esp: always check current_req is not NULL before use in DMA callbacks
esp: fix setting of ESPState mig_version_id when launching QEMU with -S option
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
.SS If
.sp
\fBdefined(CONFIG_REPLICATION)\fP
.SS \fBBlockdevOptionsReplication\fP (Object)
...
Reported-by: Markus Armbruster <armbru@redhat.com> Tested-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: John Snow <jsnow@redhat.com> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 20210406141909.1992225-2-jsnow@redhat.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Use the autogenerated fuzzer test cases as the basis for a set of am53c974
regression tests.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Tested-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20210407195801.685-13-mark.cave-ayland@ilande.co.uk>
esp: ensure that do_cmd is set to zero before submitting an ESP select command
When a CDB has been received and is about to be submitted to the SCSI layer
via one of the ESP select commands, ensure that do_cmd is set to zero before
executing the command.
Otherwise a guest executing 2 valid CDBs in quick sequence can invoke the SCSI
.transfer_data callback again before do_cmd is set to zero by the callback
function triggering an assert at the start of esp_transfer_data().
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20210407195801.685-12-mark.cave-ayland@ilande.co.uk>
esp: don't reset async_len directly in esp_select() if cancelling request
Instead let the SCSI layer invoke the .cancel callback itself to cancel and
reset the request state.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Tested-by: Alexander Bulekov <alxndr@bu.edu> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20210407195801.685-11-mark.cave-ayland@ilande.co.uk>
esp: don't overflow cmdfifo if TC is larger than the cmdfifo size
If a guest transfers the message out/command phase data using DMA with a TC
that is larger than the cmdfifo size then the cmdfifo overflows triggering
an assert. Limit the size of the transfer to the free space available in
cmdfifo.
Buglink: https://bugs.launchpad.net/qemu/+bug/1919036 Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Tested-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20210407195801.685-10-mark.cave-ayland@ilande.co.uk>
If the guest tries to read a CDB using DMA and cmdfifo is not empty then it is
possible to overflow cmdfifo.
Since this can only occur by issuing deliberately incorrect instruction
sequences, ensure that the maximum length of the CDB transferred to cmdfifo is
limited to the available free space within cmdfifo.
Buglink: https://bugs.launchpad.net/qemu/+bug/1909247 Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Tested-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20210407195801.685-9-mark.cave-ayland@ilande.co.uk>
If the guest tries to execute a CDB when cmdfifo is not empty before the start
of the message out phase then clearing the message out phase data will cause
cmdfifo to underflow due to cmdfifo_cdb_offset being larger than the amount of
data within.
Since this can only occur by issuing deliberately incorrect instruction
sequences, ensure that the maximum length of esp_fifo_pop_buf() is limited to
the size of the data within cmdfifo.
Buglink: https://bugs.launchpad.net/qemu/+bug/1909247 Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Tested-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20210407195801.685-8-mark.cave-ayland@ilande.co.uk>
esp: ensure cmdfifo is not empty and current_dev is non-NULL
When about to execute a SCSI command, ensure that cmdfifo is not empty and
current_dev is non-NULL. This can happen if the guest tries to execute a TI
(Transfer Information) command without issuing one of the select commands
first.
esp: introduce esp_fifo_pop_buf() and use it instead of fifo8_pop_buf()
The const pointer returned by fifo8_pop_buf() lies directly within the array used
to model the FIFO. Building with address sanitizers enabled shows that if the
caller expects a minimum number of bytes present then if the FIFO is nearly full,
the caller may unexpectedly access past the end of the array.
Introduce esp_fifo_pop_buf() which takes a destination buffer and performs a
memcpy() in it to guarantee that the caller cannot overwrite the FIFO array and
update all callers to use it. Similarly add underflow protection similar to
esp_fifo_push() and esp_fifo_pop() so that instead of triggering an assert()
the operation becomes a no-op.
Buglink: https://bugs.launchpad.net/qemu/+bug/1909247 Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Tested-by: Alexander Bulekov <alxndr@bu.edu> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20210407195801.685-6-mark.cave-ayland@ilande.co.uk>
esp: consolidate esp_cmdfifo_pop() into esp_fifo_pop()
Each FIFO currently has its own pop functions with the only difference being
the capacity check. The original reason for this was that the fifo8
implementation doesn't have a formal API for retrieving the FIFO capacity,
however there are multiple examples within QEMU where the capacity field is
accessed directly.
Change esp_fifo_pop() to access the FIFO capacity directly and then consolidate
esp_cmdfifo_pop() into esp_fifo_pop().
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Tested-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20210407195801.685-5-mark.cave-ayland@ilande.co.uk>
esp: consolidate esp_cmdfifo_push() into esp_fifo_push()
Each FIFO currently has its own push functions with the only difference being
the capacity check. The original reason for this was that the fifo8
implementation doesn't have a formal API for retrieving the FIFO capacity,
however there are multiple examples within QEMU where the capacity field is
accessed directly.
Change esp_fifo_push() to access the FIFO capacity directly and then consolidate
esp_cmdfifo_push() into esp_fifo_push().
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> Tested-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20210407195801.685-4-mark.cave-ayland@ilande.co.uk>
esp: rework write_response() to avoid using the FIFO for DMA transactions
The code for write_response() has always used the FIFO to store the data for
the status/message in phases, even for DMA transactions. Switch to using a
separate buffer that can be used directly for DMA transactions and restrict
the FIFO use to the non-DMA case.
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Tested-by: Alexander Bulekov <alxndr@bu.edu> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20210407195801.685-3-mark.cave-ayland@ilande.co.uk>
esp: always check current_req is not NULL before use in DMA callbacks
After issuing a SCSI command the SCSI layer can call the SCSIBusInfo .cancel
callback which resets both current_req and current_dev to NULL. If any data
is left in the transfer buffer (async_len != 0) then the next TI (Transfer
Information) command will attempt to reference the NULL pointer causing a
segfault.
esp: fix setting of ESPState mig_version_id when launching QEMU with -S option
If QEMU is launched with the -S option then the ESPState mig_version_id property
is left unset due to the ordering of the VMState fields in the VMStateDescription
for sysbusespscsi and pciespscsi. If the VM is migrated and restored in this
stopped state, the version tests in the vmstate_esp VMStateDescription and
esp_post_load() become confused causing the migration to fail.
Fix the ordering problem by moving the setting of mig_version_id to a common
esp_pre_save() function which is invoked first by both sysbusespscsi and
pciespscsi rather than at the point where ESPState is itself serialised into the
migration stream.
Buglink: https://bugs.launchpad.net/qemu/+bug/1922611 Fixes: 0bd005be78 ("esp: add vmstate_esp version to embedded ESPState") Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk> Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20210407124842.32695-1-mark.cave-ayland@ilande.co.uk>
Peter Maydell [Fri, 9 Apr 2021 15:05:27 +0000 (16:05 +0100)]
hw/arm/mps2-tz: Assert if more than one RAM is attached to an MPC
Each board in mps2-tz.c specifies a RAMInfo[] array providing
information about each RAM in the board. The .mpc field of the
RAMInfo struct specifies which MPC, if any, the RAM is attached to.
We already assert if the array doesn't have any entry for an MPC, but
we don't diagnose the error of using the same MPC number twice (which
is quite easy to do by accident if copy-and-pasting structure
entries).
Enhance find_raminfo_for_mpc() so that it detects multiple entries
for the MPC as well as missing entries.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20210409150527.15053-3-peter.maydell@linaro.org
Peter Maydell [Fri, 9 Apr 2021 15:05:26 +0000 (16:05 +0100)]
hw/arm/mps2-tz: Fix MPC setting for AN524 SRAM block
The AN524 has three MPCs: one for the BRAM, one for the QSPI flash,
and one for the DDR. We incorrectly set the .mpc field in the
RAMInfo struct for the SRAM block to 1, giving it the same MPC we are
using for the QSPI. The effect of this was that the QSPI didn't get
mapped into the system address space at all, via an MPC or otherwise,
and guest programs which tried to read from the QSPI would get a bus
error. Correct the SRAM RAMInfo to indicate that it does not have an
associated MPC.
Fixes: 25ff112a8cc ("hw/arm/mps2-tz: Add new mps3-an524 board") Signed-off-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20210409150527.15053-2-peter.maydell@linaro.org
Peter Maydell [Mon, 12 Apr 2021 11:12:09 +0000 (12:12 +0100)]
Merge remote-tracking branch 'remotes/pmaydell/tags/pull-target-arm-20210412' into staging
target-arm queue:
* hw/arm/virt-acpi-build: Fix GSIV values of the {GERR, Sync} interrupts
* hw/arm/smmuv3: Emulate CFGI_STE_RANGE for an aligned range of StreamIDs
* accel/tcg: Preserve PAGE_ANON when changing page permissions
* target/arm: Check PAGE_WRITE_ORG for MTE writeability
* exec: Fix overlap of PAGE_ANON and PAGE_TARGET_1
* remotes/pmaydell/tags/pull-target-arm-20210412:
exec: Fix overlap of PAGE_ANON and PAGE_TARGET_1
target/arm: Check PAGE_WRITE_ORG for MTE writeability
accel/tcg: Preserve PAGE_ANON when changing page permissions
hw/arm/smmuv3: Emulate CFGI_STE_RANGE for an aligned range of StreamIDs
hw/arm/virt-acpi-build: Fix GSIV values of the {GERR, Sync} interrupts
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Unfortuately, the elements of PAGE_* were not in numerical
order and so PAGE_ANON was added to an "unused" bit.
As an arbitrary choice, move PAGE_TARGET_{1,2} together.
target/arm: Check PAGE_WRITE_ORG for MTE writeability
We can remove PAGE_WRITE when (internally) marking a page
read-only because it contains translated code.
This can be triggered by tests/tcg/aarch64/bti-2, after
having serviced SIGILL trampolines on the stack.
Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
accel/tcg: Preserve PAGE_ANON when changing page permissions
Using mprotect() to change PROT_* does not change the MAP_ANON
previously set with mmap(). Our linux-user version of MTE only
works with MAP_ANON pages, so losing PAGE_ANON caused MTE to
stop working.
Reported-by: Stephen Long <steplong@quicinc.com> Signed-off-by: Richard Henderson <richard.henderson@linaro.org> Reviewed-by: Alex Bennée <alex.bennee@linaro.org> Tested-by: Alex Bennée <alex.bennee@linaro.org> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
hw/arm/smmuv3: Emulate CFGI_STE_RANGE for an aligned range of StreamIDs
In emulation of the CFGI_STE_RANGE command, we now take StreamID as the
start of the invalidation range, regardless of whatever the Range is,
whilst the spec clearly states that
- "Invalidation is performed for an *aligned* range of 2^(Range+1)
StreamIDs."
- "The bottom Range+1 bits of the StreamID parameter are IGNORED,
aligning the range to its size."
Take CFGI_ALL (where Range == 31) as an example, if there are some random
bits in the StreamID field, we'll fail to perform the full invalidation but
get a strange range (e.g., SMMUSIDRange={.start=1, .end=0}) instead. Rework
the emulation a bit to get rid of the discrepancy with the spec.
Signed-off-by: Zenghui Yu <yuzenghui@huawei.com> Acked-by: Eric Auger <eric.auger@redhat.com>
Message-id: 20210402100449.528-1-yuzenghui@huawei.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
hw/arm/virt-acpi-build: Fix GSIV values of the {GERR, Sync} interrupts
The GSIV values in SMMUv3 IORT node are not correct as they don't match
the SMMUIrq enumeration, which describes the IRQ<->PIN mapping used by
our emulated vSMMU.
Fixes: a703b4f6c1ee ("hw/arm/virt-acpi-build: Add smmuv3 node in IORT table") Signed-off-by: Zenghui Yu <yuzenghui@huawei.com> Acked-by: Eric Auger <eric.auger@redhat.com>
Message-id: 20210402084731.93-1-yuzenghui@huawei.com Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Klaus Jensen [Thu, 8 Apr 2021 10:44:05 +0000 (12:44 +0200)]
hw/block/nvme: drain namespaces on sq deletion
For most commands, when issuing an AIO, the BlockAIOCB is stored in the
NvmeRequest aiocb pointer when the AIO is issued. The main use of this
is cancelling AIOs when deleting submission queues (it is currently not
used for Abort).
However, some commands like Dataset Management Zone Management Send
(zone reset) may involve more than one AIO and here the AIOs are issued
without saving a reference to the BlockAIOCB. This is a problem since
nvme_del_sq() will attempt to cancel outstanding AIOs, potentially with
an invalid BlockAIOCB since the aiocb pointer is not NULL'ed when the
request structure is recycled.
Fix this by
1. making sure the aiocb pointer is NULL'ed when requests are recycled
2. only attempt to cancel the AIO if the aiocb is non-NULL
3. if any AIOs could not be cancelled, drain all aio as a last resort.
Fixes: dc04d25e2f3f ("hw/block/nvme: add support for the format nvm command") Fixes: c94973288cd9 ("hw/block/nvme: add broadcast nsid support flush command") Fixes: e4e430b3d6ba ("hw/block/nvme: add simple copy command") Fixes: 5f5dc4c6a942 ("hw/block/nvme: zero out zones on reset") Fixes: 2605257a26b8 ("hw/block/nvme: add the dataset management command") Cc: Gollu Appalanaidu <anaidu.gollu@samsung.com> Cc: Minwoo Im <minwoo.im@samsung.com> Signed-off-by: Klaus Jensen <k.jensen@samsung.com> Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>
hw/block/nvme: map prp fix if prp2 contains non-zero offset
nvme_map_prp needs to calculate the number of list entries based on the
offset value. For the subsequent PRP2 list, need to ensure the number of
entries is within the MAX number of PRP entries for a page.
Signed-off-by: Padmakar Kalghatgi <p.kalghatgi@samsung.com> Signed-off-by: Klaus Jensen <k.jensen@samsung.com>
spapr.c: always pulse guest IRQ in spapr_core_unplug_request()
Commit 47c8c915b162 fixed a problem where multiple spapr_drc_detach()
requests were breaking QEMU. The solution was to just spapr_drc_detach()
once, and use spapr_drc_unplug_requested() to filter whether we already
detached it or not. The commit also tied the hotplug request to the
guest in the same condition.
Turns out that there is a reliable way for a CPU hotunplug to fail. If a
guest with one CPU hotplugs a CPU1, then offline CPU0s via 'echo 0 >
/sys/devices/system/cpu/cpu0/online', then attempts to hotunplug CPU1,
the kernel will refuse it because it's the last online CPU of the
system. Given that we're pulsing the IRQ only in the first try, in a
failed attempt, all other CPU1 hotunplug attempts will fail, regardless
of the online state of CPU1 in the kernel, because we're simply not
letting the guest know that we want to hotunplug the device.
Let's move spapr_hotplug_req_remove_by_index() back out of the "if
(!spapr_drc_unplug_requested(drc))" conditional, allowing for multiple
'device_del' requests to the same CPU core to reach the guest, in case
the CPU core didn't fully hotunplugged previously.
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Message-Id: <20210401000437.131140-3-danielhb413@gmail.com> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
spapr: rollback 'unplug timeout' for CPU hotunplugs
The pseries machines introduced the concept of 'unplug timeout' for CPU
hotunplugs. The idea was to circunvent a deficiency in the pSeries
specification (PAPR), that currently does not define a proper way for
the hotunplug to fail. If the guest refuses to release the CPU (see [1]
for an example) there is no way for QEMU to detect the failure.
Further discussions about how to send a QAPI event to inform about the
hotunplug timeout [2] exposed problems that weren't predicted back when
the idea was developed. Other QEMU machines don't have any type of
hotunplug timeout mechanism for any device, e.g. ACPI based machines
have a way to make hotunplug errors visible to the hypervisor. This
would make this timeout mechanism exclusive to pSeries, which is not
ideal.
The real problem is that a QAPI event that reports hotunplug timeouts
puts the management layer (namely Libvirt) in a weird spot. We're not
telling that the hotunplug failed, because we can't be 100% sure of
that, and yet we're resetting the unplug state back, preventing any
DEVICE_DEL events to reach out in case the guest decides to release the
device. Libvirt would need to inspect the guest itself to see if the
device was released or not, otherwise the internal domain states will be
inconsistent. Moreover, Libvirt already has an 'unplug timeout'
concept, and a QEMU side timeout would need to be juggled together with
the existing Libvirt timeout.
All this considered, this solution ended up creating more trouble than
it solved. This patch reverts the 3 commits that introduced the timeout
mechanism for CPU hotplugs in pSeries machines.
CC: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
Message-Id: <20210401000437.131140-2-danielhb413@gmail.com> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Greg Kurz [Fri, 9 Apr 2021 16:03:39 +0000 (18:03 +0200)]
cpu/core: Fix "help" of CPU core device types
Calling qdev_get_machine() from a QOM instance_init function is
fragile because we can't be sure the machine object actually
exists. And this happens to break when passing ",help" on the
command line to get the list of properties for a CPU core
device types :
This used to work before QEMU 5.0, but commit 3df261b6676b
unwillingly introduced a subtle regression : the above command
line needs to create an instance but the instance_init function
of the base class calls qdev_get_machine() before
qemu_create_machine() has been called, which is a programming bug.
Use current_machine instead. It is okay to skip the setting of
nr_thread in this case since only its type is displayed.
Fixes: 3df261b6676b ("softmmu/vl.c: Handle '-cpu help' and '-device help' before 'no default machine'") Reported-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Greg Kurz <groug@kaod.org> Cc: peter.maydell@linaro.org
Message-Id: <20210409160339.500167-3-groug@kaod.org> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
Babu Moger [Wed, 3 Mar 2021 15:45:30 +0000 (09:45 -0600)]
i386: Add missing cpu feature bits in EPYC-Rome model
Found the following cpu feature bits missing from EPYC-Rome model.
ibrs : Indirect Branch Restricted Speculation
ssbd : Speculative Store Bypass Disable
These new features will be added in EPYC-Rome-v2. The -cpu help output
after the change.
x86 EPYC-Rome (alias configured by machine type)
x86 EPYC-Rome-v1 AMD EPYC-Rome Processor
x86 EPYC-Rome-v2 AMD EPYC-Rome Processor
# gpg: Signature made Fri 09 Apr 2021 17:13:11 BST
# gpg: using RSA key DC3DEB159A9AF95D3D7456FE7F09B272C88F2FD6
# gpg: issuer "kwolf@redhat.com"
# gpg: Good signature from "Kevin Wolf <kwolf@redhat.com>" [full]
# Primary key fingerprint: DC3D EB15 9A9A F95D 3D74 56FE 7F09 B272 C88F 2FD6
* remotes/kevin/tags/for-upstream:
test-blockjob: Test job_wait_unpaused()
job: Allow complete for jobs on standby
mirror: Do not enter a paused job on completion
mirror: Move open_backing_file to exit_common
hw/block/fdc: Fix 'fallback' property on sysbus floppy disk controllers
iotests: Test mirror-top filter permissions
iotests: add test for removing persistent bitmap from backing file
iotests/qsd-jobs: Filter events in the first test
block/rbd: fix memory leak in qemu_rbd_co_create_opts()
block/rbd: fix memory leak in qemu_rbd_connect()
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Max Reitz [Fri, 9 Apr 2021 12:04:19 +0000 (14:04 +0200)]
mirror: Do not enter a paused job on completion
Currently, it is impossible to complete jobs on standby (i.e. paused
ready jobs), but actually the only thing in mirror_complete() that does
not work quite well with a paused job is the job_enter() at the end.
If we make it conditional, this function works just fine even if the
mirror job is paused.
So technically this is a no-op, but obviously the intention is to accept
block-job-complete even for jobs on standby, which we need this patch
for first.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20210409120422.144040-3-mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
hw/block/fdc: Fix 'fallback' property on sysbus floppy disk controllers
Setting the 'fallback' property corrupts the QOM instance state
(FDCtrlSysBus) because it accesses an incorrect offset (it uses
the offset of the FDCtrlISABus state).
Cc: qemu-stable@nongnu.org Fixes: a73275dd6fc ("fdc: Add fallback option") Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20210407133742.1680424-1-f4bug@amsat.org> Reviewed-by: Markus Armbruster <armbru@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20210331122815.51491-1-mreitz@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Reviewed-by: Eric Blake <eblake@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
iotests: add test for removing persistent bitmap from backing file
Just demonstrate one of x-blockdev-reopen usecases. We can't simply
remove persistent bitmap from RO node (for example from backing file),
as we need to remove it from the image too. So, we should reopen the
node first.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20210401161522.8001-1-vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Max Reitz [Thu, 1 Apr 2021 13:28:39 +0000 (15:28 +0200)]
iotests/qsd-jobs: Filter events in the first test
The job may or may not be ready before the 'quit' is issued. Whether it
is is irrelevant; for the purpose of the test, it only needs to still be
there. Filter the job status change and READY events from the output so
it becomes reliable.
Reported-by: Peter Maydell <peter.maydell@linaro.org> Suggested-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-Id: <20210401132839.139939-1-mreitz@redhat.com> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
block/rbd: fix memory leak in qemu_rbd_co_create_opts()
When we allocate 'q_namespace', we forgot to set 'has_q_namespace'
to true. This can cause several issues, including a memory leak,
since qapi_free_BlockdevCreateOptions() does not deallocate that
memory, as reported by valgrind:
13 bytes in 1 blocks are definitely lost in loss record 7 of 96
at 0x4839809: malloc (vg_replace_malloc.c:307)
by 0x48CEBB8: g_malloc (in /usr/lib64/libglib-2.0.so.0.6600.8)
by 0x48E3FE3: g_strdup (in /usr/lib64/libglib-2.0.so.0.6600.8)
by 0x180010: qemu_rbd_co_create_opts (rbd.c:446)
by 0x1AE72C: bdrv_create_co_entry (block.c:492)
by 0x241902: coroutine_trampoline (coroutine-ucontext.c:173)
by 0x57530AF: ??? (in /usr/lib64/libc-2.32.so)
by 0x1FFEFFFA6F: ???
Fix setting 'has_q_namespace' to true when we allocate 'q_namespace'.
Fixes: 19ae9ae014 ("block/rbd: Add support for ceph namespaces") Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Message-Id: <20210329150129.121182-3-sgarzare@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
In qemu_rbd_connect(), 'mon_host' is allocated by qemu_rbd_mon_host()
using g_strjoinv(), but it's only freed in the error path, leaking
memory in the success path as reported by valgrind:
80 bytes in 4 blocks are definitely lost in loss record 5,028 of 6,516
at 0x4839809: malloc (vg_replace_malloc.c:307)
by 0x5315BB8: g_malloc (in /usr/lib64/libglib-2.0.so.0.6600.8)
by 0x532B6FF: g_strjoinv (in /usr/lib64/libglib-2.0.so.0.6600.8)
by 0x87D07E: qemu_rbd_mon_host (rbd.c:538)
by 0x87D07E: qemu_rbd_connect (rbd.c:562)
by 0x87E1CE: qemu_rbd_open (rbd.c:740)
by 0x840EB1: bdrv_open_driver (block.c:1528)
by 0x8453A9: bdrv_open_common (block.c:1802)
by 0x8453A9: bdrv_open_inherit (block.c:3444)
by 0x8464C2: bdrv_open (block.c:3537)
by 0x8108CD: qmp_blockdev_add (blockdev.c:3569)
by 0x8EA61B: qmp_marshal_blockdev_add (qapi-commands-block-core.c:1086)
by 0x90B528: do_qmp_dispatch_bh (qmp-dispatch.c:131)
by 0x907EA4: aio_bh_poll (async.c:164)
Fix freeing 'mon_host' also when qemu_rbd_connect() ends correctly.
Fixes: 0a55679b4a5061f4d74bdb1a0e81611ba3390b00 Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
Message-Id: <20210329150129.121182-2-sgarzare@redhat.com> Reviewed-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Max Reitz <mreitz@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Pierre Morel [Thu, 8 Apr 2021 16:32:09 +0000 (18:32 +0200)]
s390x: css: report errors from ccw_dstream_read/write
ccw_dstream_read/write functions returned values are sometime
not taking into account and reported back to the upper level
of interpretation of CCW instructions.
It follows that accessing an invalid address does not trigger
a subchannel status program check to the guest as it should.
Let's test the return values of ccw_dstream_write[_buf] and
ccw_dstream_read[_buf] and report it to the caller.
Cc: qemu-stable@nongnu.org Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> Acked-by: Halil Pasic <pasic@linux.ibm.com>
Message-Id: <1617899529-9329-2-git-send-email-pmorel@linux.ibm.com> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
Peter Maydell [Thu, 8 Apr 2021 15:45:31 +0000 (16:45 +0100)]
Merge remote-tracking branch 'remotes/jasowang/tags/net-pull-request' into staging
# gpg: Signature made Thu 08 Apr 2021 10:34:24 BST
# gpg: using RSA key EF04965B398D6211
# gpg: Good signature from "Jason Wang (Jason Wang on RedHat) <jasowang@redhat.com>" [marginal]
# 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:
tap-win32: correctly recycle buffers
Revert "qapi: net: Add query-netdev command"
Revert "tests: Add tests for query-netdev command"
Revert "net: Move NetClientState.info_str to dynamic allocations"
Revert "hmp: Use QAPI NetdevInfo in hmp_info_network"
Revert "net: Do not fill legacy info_str for backends"
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Peter Maydell [Thu, 8 Apr 2021 10:39:53 +0000 (11:39 +0100)]
Merge remote-tracking branch 'remotes/stsquad/tags/pull-6.0-rc2-fixes-060421-1' into staging
Testing updates:
- fix x86_64 cross compilers
- don't use registry for non-x86 containers
- add valid host types for given cross compile containers
- clean up i386 code16 test with explicit -no-pie
- relax sha1.py gdbstub test
- add more gdbstub documentation
- remove annoying warning on gitlab
- test dtrace backend in gitlab
# gpg: Signature made Tue 06 Apr 2021 15:55:53 BST
# gpg: using RSA key 6685AE99E75167BCAFC8DF35FBD0DB095A9E2A44
# gpg: Good signature from "Alex Bennée (Master Work Key) <alex.bennee@linaro.org>" [full]
# Primary key fingerprint: 6685 AE99 E751 67BC AFC8 DF35 FBD0 DB09 5A9E 2A44
* remotes/stsquad/tags/pull-6.0-rc2-fixes-060421-1:
gitlab-ci.yml: Test the dtrace backend in one of the jobs
gitlab-ci.yml: Fix the filtering for the git submodules
docs/system/gdb.rst: Document how to debug multicore machines
docs/system/gdb.rst: Add some more heading structure
tests/tcg: relax the next step precision of the gdb sha1 test
tests/tcg/i386: force -fno-pie for test-i386
tests/tcg/i386: expand .data sections for system tests
tests/tcg/configure.sh: make sure we pick up x86_64 cross compilers
tests/tcg: add concept of container_hosts
tests/docker: don't set DOCKER_REGISTRY on non-x86_64
tests/tcg: update the defaults for x86 compilers
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Jason Wang [Fri, 26 Mar 2021 08:46:43 +0000 (16:46 +0800)]
tap-win32: correctly recycle buffers
Commit 969e50b61a28 ("net: Pad short frames to minimum size before
sending from SLiRP/TAP") tries to pad frames but try to recyle the
local array that is used for padding to tap thread. This patch fixes
this by recyling the original buffer.
Fixes: 969e50b61a28 ("net: Pad short frames to minimum size before sending from SLiRP/TAP") Tested-by: Howard Spoelstra <hsp.cat7@gmail.com> Tested-by: Bin Meng <bmeng.cn@gmail.com> Reviewed-by: Bin Meng <bmeng.cn@gmail.com> Signed-off-by: Jason Wang <jasowang@redhat.com>
Jason Wang [Fri, 2 Apr 2021 03:03:33 +0000 (11:03 +0800)]
Revert "net: Move NetClientState.info_str to dynamic allocations"
Several issues has been reported for query-netdev info
series. Consider it's late in the rc, this reverts commit
commit 59b5437eb732d6b103a9bc279c3482c834d1eff9.
Hyman Huang(黄勇) [Tue, 23 Mar 2021 15:43:58 +0000 (23:43 +0800)]
tests/migration: fix parameter of auto-converge migration
when execute the following test command:
$ ./guestperf-batch.py --auto-converge \
--auto-converge-step {percent} ...
test aborts and error message be throwed as the following:
"Parameter 'x-cpu-throttle-increment' is unexpected"
The reason is that 'x-cpu-throttle-increment' has been
deprecated and 'cpu-throttle-increment' was introduced
Since v2.7. Use the new parameter instead.
Signed-off-by: Hyman Huang(黄勇) <huangy81@chinatelecom.cn>
Message-Id: <0195d34a317ce3cc417b3efd275e30cad35a7618.1616513998.git.huangy81@chinatelecom.cn> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
migration: Rename 'bs' to 'block' in background snapshot code
Rename 'bs' to commonly used 'block' in migration/ram.c background
snapshot code.
Signed-off-by: Andrey Gruzdev <andrey.gruzdev@virtuozzo.com> Reported-by: David Hildenbrand <david@redhat.com>
Message-Id: <20210401092226.102804-5-andrey.gruzdev@virtuozzo.com> Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
migration: Pre-fault memory before starting background snasphot
This commit solves the issue with userfault_fd WP feature that
background snapshot is based on. For any never poluated or discarded
memory page, the UFFDIO_WRITEPROTECT ioctl() would skip updating
PTE for that page, thereby loosing WP setting for it.
So we need to pre-fault pages for each RAM block to be protected
before making a userfault_fd wr-protect ioctl().
Fixes: 278e2f551a095b234de74dca9c214d5502a1f72c (migration: support
UFFD write fault processing in ram_save_iterate()) Signed-off-by: Andrey Gruzdev <andrey.gruzdev@virtuozzo.com> Reported-by: David Hildenbrand <david@redhat.com> Reviewed-by: David Hildenbrand <david@redhat.com>
Message-Id: <20210401092226.102804-4-andrey.gruzdev@virtuozzo.com> Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
dgilbert:
Bodged ifdef __linux__ on ram_write_tracking_prepare, should really
go in a stub
Klaus Jensen [Wed, 7 Apr 2021 05:16:14 +0000 (07:16 +0200)]
hw/block/nvme: fix out-of-bounds read in nvme_subsys_ctrl
nvme_subsys_ctrl() is used in contexts where the given controller
identifier is from an untrusted source. Like its friends nvme_ns() and
nvme_subsys_ns(), nvme_subsys_ctrl() should just return NULL if an
invalid identifier is given.
Fixes: 645ce1a70cb6 ("hw/block/nvme: support namespace attachment command") Cc: Minwoo Im <minwoo.im.dev@gmail.com> Signed-off-by: Klaus Jensen <k.jensen@samsung.com> Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>
Klaus Jensen [Wed, 7 Apr 2021 05:07:43 +0000 (07:07 +0200)]
hw/block/nvme: fix assert crash in nvme_subsys_ns
nvme_subsys_ns() is used in contexts where the namespace identifier is
taken from an untrusted source. Commit 3921756dee6d ("hw/block/nvme:
assert namespaces array indices") tried to guard against this by
introducing an assert on the namespace identifier.
This is wrong since it is perfectly valid to call the function with an
invalid namespace identifier and like nvme_ns(), nvme_subsys_ns() should
simply return NULL.
Fixes: 3921756dee6d ("hw/block/nvme: assert namespaces array indices") Fixes: 94d8d6d16781 ("hw/block/nvme: support allocated namespace type") Cc: Minwoo Im <minwoo.im.dev@gmail.com> Signed-off-by: Klaus Jensen <k.jensen@samsung.com> Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>
nvme_ns_attachment() does not verify the contents of the host-supplied
16 bit "Number of Identifiers" field in the command payload.
Make sure the value is capped at 2047 and fix the out-of-bounds read.
Fixes: 645ce1a70cb6 ("hw/block/nvme: support namespace attachment command") Cc: Minwoo Im <minwoo.im.dev@gmail.com> Signed-off-by: Klaus Jensen <k.jensen@samsung.com> Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>
Klaus Jensen [Tue, 23 Mar 2021 11:43:24 +0000 (12:43 +0100)]
hw/block/nvme: fix handling of private namespaces
Prior to this patch, if a private nvme-ns device (that is, a namespace
that is not linked to a subsystem) is wired up to an nvme-subsys linked
nvme controller device, the device fails to verify that the namespace id
is unique within the subsystem. NVM Express v1.4b, Section 6.1.6 ("NSID
and Namespace Usage") states that because the device supports Namespace
Management, "NSIDs *shall* be unique within the NVM subsystem".
Additionally, prior to this patch, private namespaces are not known to
the subsystem and the namespace is considered exclusive to the
controller with which it is initially wired up to. However, this is not
the definition of a private namespace; per Section 1.6.33 ("private
namespace"), a private namespace is just a namespace that does not
support multipath I/O or namespace sharing, which means "that it is only
able to be attached to one controller at a time".
Fix this by always allocating namespaces in the subsystem (if one is
linked to the controller), regardless of the shared/private status of
the namespace. Whether or not the namespace is shareable is controlled
by a new `shared` nvme-ns parameter.
Finally, this fix allows the nvme-ns `subsys` parameter to be removed,
since the `shared` parameter now serves the purpose of attaching the
namespace to all controllers in the subsystem upon device realization.
It is invalid to have an nvme-ns namespace device with a linked
subsystem without the parent nvme controller device also being linked to
one and since the nvme-ns devices will unconditionally be "attached" (in
QEMU terms that is) to an nvme controller device through an NvmeBus, the
nvme-ns namespace device can always get a reference to the subsystem of
the controller it is explicitly (using 'bus=' parameter) or implicitly
attaching to.
Fixes: e570768566b3 ("hw/block/nvme: support for shared namespace in subsystem") Cc: Minwoo Im <minwoo.im.dev@gmail.com> Signed-off-by: Klaus Jensen <k.jensen@samsung.com> Reviewed-by: Gollu Appalanaidu <anaidu.gollu@samsung.com> Reviewed-by: Keith Busch <kbusch@kernel.org> Reviewed-by: Minwoo Im <minwoo.im.dev@gmail.com>
projects / qemu-xen.git / log