qemu_set_nonblock() checks that the file descriptor can be used and, if
not, crashes QEMU. An assert() is used for that. The use of assert() is
used to detect programming error and the coredump will allow to debug
the problem.
But in the case of the tap device, this assert() can be triggered by
a misconfiguration by the user. At startup, it's not a real problem, but it
can also happen during the hot-plug of a new device, and here it's a
problem because we can crash a perfectly healthy system.
For instance:
# ip link add link virbr0 name macvtap0 type macvtap mode bridge
# ip link set macvtap0 up
# TAP=/dev/tap$(ip -o link show macvtap0 | cut -d: -f1)
# qemu-system-x86_64 -machine q35 -device pcie-root-port,id=pcie-root-port-0 -monitor stdio 9<> $TAP
(qemu) netdev_add type=tap,id=hostnet0,vhost=on,fd=9
(qemu) device_add driver=virtio-net-pci,netdev=hostnet0,id=net0,bus=pcie-root-port-0
(qemu) device_del net0
(qemu) netdev_del hostnet0
(qemu) netdev_add type=tap,id=hostnet1,vhost=on,fd=9
qemu-system-x86_64: .../util/oslib-posix.c:247: qemu_set_nonblock: Assertion `f != -1' failed.
Aborted (core dumped)
To avoid that, add a function, qemu_try_set_nonblock(), that allows to report the
problem without crashing.
In the same way, we also update the function for vhostfd in net_init_tap_one() and
for fd in net_init_socket() (both descriptors are provided by the user and can
be wrong).
Signed-off-by: Laurent Vivier <lvivier@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
This patch allow users to set the "max_queue_size" according
to their environment.
Signed-off-by: Zhang Chen <chen.zhang@intel.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
Added fix for checksum offload for IPv6 if a backend doesn't
have a virtual header.
This patch is a part of IPv6 fragmentation.
Signed-off-by: Andrew Melnychenko <andrew@daynix.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
If you have a networking device and its virtio failover device, and
you remove them in this order:
- virtio device
- the real device
You get qemu crash.
See bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1820120
Bug exist on qemu 4.2 and 5.0.
But in 5.0 don't shows because commit
77b06bba62
somehow papers over it.
CC: Jason Wang <jasowang@redhat.com>
CC: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Acked-by: Laurent Vivier <lvivier@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
- Reduce race conditions on QEMUMachine::shutdown()
1. Remove the "bare except" pattern in the existing shutdown code,
which can mask problems and make debugging difficult.
2. Ensure that post-shutdown cleanup is always performed, even when
graceful termination fails.
3. Unify cleanup paths such that no matter how the VM is terminated,
the same functions and steps are always taken to reset the object
state.
4. Rewrite shutdown() such that any error encountered when attempting
a graceful shutdown will be raised as an AbnormalShutdown exception.
The pythonic idiom is to allow the caller to decide if this is a
problem or not.
- Modify part of the python/qemu library to comply with:
. mypy --strict
. pylint
. flake8
- Script for the TCG Continuous Benchmarking project that uses
callgrind to dissect QEMU execution into three main phases:
. code generation
. JIT execution
. helpers execution
CI jobs results:
. https://cirrus-ci.com/build/5421349961203712
. https://gitlab.com/philmd/qemu/-/pipelines/166556001
. https://travis-ci.org/github/philmd/qemu/builds/708102347
-----BEGIN PGP SIGNATURE-----
iQIzBAABCAAdFiEE+qvnXhKRciHc/Wuy4+MsLN6twN4FAl8OGCUACgkQ4+MsLN6t
wN52ZQ//cB8pRhOeirMzPTamtAePAHhU9kRKSQ4fFzVM/AHxaO3qO3KrMKAv4JnR
1fwdyAbum0Dv+HrRAXbIVz/lgrlTb6tLqwk20YWlmNUR2CtMmno9fPQzg6LeFWBe
+qr2/anXWBqRZu5uHzQ3Ly3LxlVXE3qmStxlttNxed9wK9wdhsQ4ijOrExXPmMI5
IQi9tKijsGvOczcSpIwJET6Px0L/GIYHSFGXfHv4LiJCMQUX4gqY1akr+/2adqlr
ElcOHlvWG90GHftTC+hq+KbSNmpkwJEnXLHC+HwMaYmQuhH4+U6f9AcrnzYGHjoQ
84muuYt5CBvT/Q38qO3dUsr42oD8uiDZe2cpD5DmcAHj6U9sza4UwU2Nkd4cNvRD
OiYRWuHQDzQhwPGbzBym1GRxRlejmPXCpvZ4XqaYODkzhEHi8UVq5AOsNhpTGWVk
WoDZ3NtnZb9WbU93wDordhWTtHKHOLHNk4CuJLbdRvkW+sg8aFqUObSRvc6RqyU8
1DXWMMQr9UfCDfMMbp/OCymphn0VrAaixIRIjmYc6nL5mQlyugMwij55jzyugqBH
rN5bdcsqxAhBTD03lu0lbDn1BYIqbZFXABXeDnJNbxmY/6gD0E1HKWRvIKjxzH+t
L19QXsOWLoVUpUIjxlOgMlb9QJLPc9m/N3yVOanO0zVrMSBmOKQ=
=Jp6n
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/philmd-gitlab/tags/python-next-20200714' into staging
Python patches for 5.1
- Reduce race conditions on QEMUMachine::shutdown()
1. Remove the "bare except" pattern in the existing shutdown code,
which can mask problems and make debugging difficult.
2. Ensure that post-shutdown cleanup is always performed, even when
graceful termination fails.
3. Unify cleanup paths such that no matter how the VM is terminated,
the same functions and steps are always taken to reset the object
state.
4. Rewrite shutdown() such that any error encountered when attempting
a graceful shutdown will be raised as an AbnormalShutdown exception.
The pythonic idiom is to allow the caller to decide if this is a
problem or not.
- Modify part of the python/qemu library to comply with:
. mypy --strict
. pylint
. flake8
- Script for the TCG Continuous Benchmarking project that uses
callgrind to dissect QEMU execution into three main phases:
. code generation
. JIT execution
. helpers execution
CI jobs results:
. https://cirrus-ci.com/build/5421349961203712
. https://gitlab.com/philmd/qemu/-/pipelines/166556001
. https://travis-ci.org/github/philmd/qemu/builds/708102347
# gpg: Signature made Tue 14 Jul 2020 21:40:05 BST
# gpg: using RSA key FAABE75E12917221DCFD6BB2E3E32C2CDEADC0DE
# gpg: Good signature from "Philippe Mathieu-Daudé (F4BUG) <f4bug@amsat.org>" [full]
# Primary key fingerprint: FAAB E75E 1291 7221 DCFD 6BB2 E3E3 2C2C DEAD C0DE
* remotes/philmd-gitlab/tags/python-next-20200714:
python/qmp.py: add QMPProtocolError
python/qmp.py: add casts to JSON deserialization
python/qmp.py: Do not return None from cmd_obj
python/qmp.py: re-absorb MonitorResponseError
iotests.py: use qemu.qmp type aliases
python/qmp.py: Define common types
python/machine.py: change default wait timeout to 3 seconds
python/machine.py: re-add sigkill warning suppression
python/machine.py: split shutdown into hard and soft flavors
tests/acceptance: Don't test reboot on cubieboard
tests/acceptance: wait() instead of shutdown() where appropriate
python/machine.py: Make wait() call shutdown()
python/machine.py: Add a configurable timeout to shutdown()
python/machine.py: Prohibit multiple shutdown() calls
python/machine.py: Perform early cleanup for wait() calls, too
python/machine.py: Add _early_cleanup hook
python/machine.py: Close QMP socket in cleanup
python/machine.py: consolidate _post_shutdown()
scripts/performance: Add dissect.py script
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
By using invalidated address, guest can do out-of-bounds accesses.
These patches fix the issue by only allowing SD card image sizes
power of 2, and not switching to SEND_DATA state when the address
is invalid (out of range).
This issue was found using QEMU fuzzing mode (using --enable-fuzzing,
see docs/devel/fuzzing.txt) and reported by Alexander Bulekov.
Reproducer:
https://bugs.launchpad.net/qemu/+bug/1880822/comments/1
CI jobs results:
. https://cirrus-ci.com/build/5157142548185088
. https://gitlab.com/philmd/qemu/-/pipelines/166381731
. https://travis-ci.org/github/philmd/qemu/builds/707956535
-----BEGIN PGP SIGNATURE-----
iQIzBAABCAAdFiEE+qvnXhKRciHc/Wuy4+MsLN6twN4FAl8NuSQACgkQ4+MsLN6t
wN7MEg/+PER/n+CpmrC2lggQ3WJwNjvY09A4yfPfhKldjOi+25/amf/bQ2Zjmj7m
HoiiPFu7vz+FugOfGv5YFlTS2+VNmN1UZqGqZRwY/YJJKg9am6TJ8zA4UBf4iegi
OqNBJOPW/EYsAYdH3jUFmW15zAsRHEM6g2vZ1Z4WwVZqfYHsMB/y2khp9Fr+jGU0
6wDeG0cdap5QVsamIll4/BoxgBa5UdtBYjzo7QBENs+abvOf56jjUqZx0+AL/Ua/
IOpZ01mmPZJ4wJxPNT87gfEnHv0MRA7bSpJ7TAC80xVoQjeoK+V2Ohvy+rvYPaqm
5mR0l4M+GGhglCg44wV3uwNonmltCxvTgGqZrQPsa3WnXMFoXqwGZgwl6XrYdLzV
hVODJAu/Ivegk9AAbVrZGXg/shQtkB4gyoOaE3Qoraf1az9/XudECIo+zwocP4Ip
Z0ny8bwQKq2QGYrCU3NWlgWi30sj6PeW5e6Jgq/2b1sUeKuUgNuuBPcRmXQ6kaz5
vMX7qYsXAxvO7o1QlbASzdvSvOXGx+0J0CJctPnY4jAJ7qjvJTKOb0j+jwMNJy+D
XFAgB+D0go+UvnaPJn6teIHzaD4NqWE37MaamxsMY6RWjAnoy1+OOvZIZTnq+LnH
iLbgk2EsxlFyBd3aZ/51ukeTUxpNgu9J6iRcXB3yVNBS4vqlBDw=
=VZsF
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/philmd-gitlab/tags/sdcard-CVE-2020-13253-pull-request' into staging
Fix CVE-2020-13253
By using invalidated address, guest can do out-of-bounds accesses.
These patches fix the issue by only allowing SD card image sizes
power of 2, and not switching to SEND_DATA state when the address
is invalid (out of range).
This issue was found using QEMU fuzzing mode (using --enable-fuzzing,
see docs/devel/fuzzing.txt) and reported by Alexander Bulekov.
Reproducer:
https://bugs.launchpad.net/qemu/+bug/1880822/comments/1
CI jobs results:
. https://cirrus-ci.com/build/5157142548185088
. https://gitlab.com/philmd/qemu/-/pipelines/166381731
. https://travis-ci.org/github/philmd/qemu/builds/707956535
# gpg: Signature made Tue 14 Jul 2020 14:54:44 BST
# gpg: using RSA key FAABE75E12917221DCFD6BB2E3E32C2CDEADC0DE
# gpg: Good signature from "Philippe Mathieu-Daudé (F4BUG) <f4bug@amsat.org>" [full]
# Primary key fingerprint: FAAB E75E 1291 7221 DCFD 6BB2 E3E3 2C2C DEAD C0DE
* remotes/philmd-gitlab/tags/sdcard-CVE-2020-13253-pull-request:
hw/sd/sdcard: Do not switch to ReceivingData if address is invalid
hw/sd/sdcard: Update coding style to make checkpatch.pl happy
hw/sd/sdcard: Do not allow invalid SD card sizes
hw/sd/sdcard: Simplify realize() a bit
hw/sd/sdcard: Restrict Class 6 commands to SCSD cards
tests/acceptance/boot_linux: Expand SD card image to power of 2
tests/acceptance/boot_linux: Tag tests using a SD card with 'device:sd'
docs/orangepi: Add instructions for resizing SD image to power of two
MAINTAINERS: Cc qemu-block mailing list
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
In the case that we receive a reply but are unable to understand it,
use this exception name to indicate that case.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200710052220.3306-7-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
mypy and python type hints are not powerful enough to properly describe
JSON messages in Python 3.6. The best we can do, generally, is describe
them as Dict[str, Any].
Add casts to coerce this type for static analysis; but do NOT enforce
this type at runtime in any way.
Note: Python 3.8 adds a TypedDict construct which allows for the
description of more arbitrary Dictionary shapes. There is a third-party
module, "Pydantic", which is compatible with 3.6 that can be used
instead of the JSON library that parses JSON messages to fully-typed
Python objects, and may be preferable in some cases.
(That is well beyond the scope of this commit or series.)
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200710052220.3306-6-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
This makes typing the qmp library difficult, as it necessitates wrapping
Optional[] around the type for every return type up the stack. At some
point, it becomes difficult to discern or remember why it's None instead
of the expected object.
Use the python exception system to tell us exactly why we didn't get an
object. Remove this special-cased return.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200710052220.3306-5-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
When I initially split this out, I considered this more of a machine
error than a QMP protocol error, but I think that's misguided.
Move this back to qmp.py and name it QMPResponseError. Convert
qmp.command() to use this exception type.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200710052220.3306-4-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
iotests.py should use the type definitions from qmp.py instead of its
own.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200710052220.3306-3-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Define some common types that we'll need to annotate a lot of other
functions going forward.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Message-Id: <20200710052220.3306-2-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Machine.wait() does not appear to be used except in the acceptance tests,
and an infinite timeout by default in a test suite is not the most helpful.
Change it to 3 seconds, like the default shutdown timeout.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20200710050649.32434-13-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
If the user kills QEMU on purpose, we don't need to warn
them about that having happened: they know already.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20200710050649.32434-12-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
This is done primarily to avoid the 'bare except' pattern, which
suppresses all exceptions during shutdown and can obscure errors.
Replace this with a pattern that isolates the different kind of shutdown
paradigms (_hard_shutdown and _soft_shutdown), and a new fallback shutdown
handler (_do_shutdown) that gracefully attempts one before the other.
This split now also ensures that no matter what happens,
_post_shutdown() is always invoked.
shutdown() changes in behavior such that if it attempts to do a graceful
shutdown and is unable to, it will now always raise an exception to
indicate this. This can be avoided by the test writer in three ways:
1. If the VM is expected to have already exited or is in the process of
exiting, wait() can be used instead of shutdown() to clean up resources
instead. This helps avoid race conditions in shutdown.
2. If a test writer is expecting graceful shutdown to fail, shutdown
should be called in a try...except block.
3. If the test writer has no interest in performing a graceful shutdown
at all, kill() can be used instead.
Handling shutdown in this way makes it much more explicit which type of
shutdown we want and allows the library to report problems with this
process.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20200710050649.32434-11-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
cubieboard does not have a functioning reboot, it halts and QEMU does
not exit.
vm.shutdown() is modified in a forthcoming patch that makes it less tolerant
of race conditions on shutdown; tests should consciously decide to WAIT
or to SHUTDOWN qemu.
So long as this test is attempting to reboot, the correct choice would
be to WAIT for the VM to exit. However, since that's broken, we should
SHUTDOWN instead.
SHUTDOWN is indeed what already happens when the test performs teardown,
however, if anyone fixes cubieboard reboot in the future, this test will
develop a new race condition that might be hard to debug.
Therefore: remove the reboot test and make it obvious that the VM is
still running when the test concludes, where the test teardown will do
the right thing.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20200710050649.32434-10-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
When issuing 'reboot' to a VM with the no-reboot option, that VM will
exit. When then issuing a shutdown command, the cleanup may race.
Add calls to vm.wait() which will gracefully mark the VM as having
exited. Subsequent vm.shutdown() calls in generic tearDown code will not
race when called after completion of the call.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20200710050649.32434-9-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
At this point, shutdown(has_quit=True) and wait() do essentially the
same thing; they perform cleanup without actually instructing QEMU to
quit.
Define one in terms of the other.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20200710050649.32434-8-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Three seconds is hardcoded. Use it as a default parameter instead, and use that
value for both waits that may occur in the function.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20200710050649.32434-7-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
If the VM is not launched, don't try to shut it down. As a change,
_post_shutdown now unconditionally also calls _early_cleanup in order to
offer comprehensive object cleanup in failure cases.
As a courtesy, treat it as a NOP instead of rejecting it as an
error. This is slightly nicer for acceptance tests where vm.shutdown()
is issued unconditionally in tearDown callbacks.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200710050649.32434-6-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
This is primarily for consistency, and is a step towards wait() and
shutdown() sharing the same implementation so that the two cleanup paths
cannot diverge.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20200710050649.32434-5-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Some parts of cleanup need to occur prior to shutdown, otherwise
shutdown might break. Move this into a suitably named method/callback.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20200710050649.32434-4-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
It's not important to do this before waiting for the process to exit, so
it can be done during generic post-shutdown cleanup.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20200710050649.32434-3-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Move more cleanup actions into _post_shutdown. As a change, if QEMU
should so happen to be terminated during a call to wait(), that event
will now be logged.
This is not likely to occur during normative use.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200710050649.32434-2-jsnow@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
- file-posix: Mitigate file fragmentation with extent size hints
- Tighten qemu-img rules on missing backing format
- qemu-img map: Don't limit block status request size
- Fix crash with virtio-scsi and iothreads
-----BEGIN PGP SIGNATURE-----
iQJFBAABCAAvFiEE3D3rFZqa+V09dFb+fwmycsiPL9YFAl8NsgMRHGt3b2xmQHJl
ZGhhdC5jb20ACgkQfwmycsiPL9Z0tA//eqauxD7cTEpwrtLNrRtpiBtMG64BBpxz
QfkURzB38bMVahHlwq3Gt7Zcov8V4V7vxK66h688Z/fhw3vmqIeVe8+P6+Y5s9FL
jil8lewHuLTa6xELeugoV7SZXH8AAh1W2fQmiR7EPiOmpSE0wf7C5IShVlX8A04E
r0n09+61qGjRIe1hNTwTtldqQEfx6UGnxQWcQb81JUPA1lZhX3cnPg/j94Bofr+m
v/DbVTfsmUtTMjc0PdU7n4DKTWu8OS5B/X0unF21rTtO//cYBrhAeY3ax2jbFBWi
CIZK8HLI5m9/HFyltql1LOsd+B5TtfnXMfSdvDh2jaVUlto7wTeTnWU1fv4wxUB5
hk7XgJo/y203ebFNHpTmW8tvLfGTP8uqCVfOEFxzjy+JHGrarlbWkwL2LMOFFAZ2
s2WcwlfqiYGFTG4+OFdhPf9qPWKSqMr+jTdZJTse64/c6+YXWHk+pP9lfYEUOgSi
OYwdQUY9uiZ1K13q5Tif2TbFvs+c118xdTgVhAV7VtfPnWc3c647dX7iaq8Szknc
IT93670Iqf/PzEj+L7XUbbLIIsAcmxD0sr7QAQEt7bfiYIDRIQLiVPyzXplETFg2
SEkvtqBovm84ct7pWQzqA6lFvr3oIFDNquR40XFGozHNnlBeNi5s7pXQnqUBLElr
wDDuEi+z5QM=
=DB0q
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/kevin/tags/for-upstream' into staging
Block layer patches:
- file-posix: Mitigate file fragmentation with extent size hints
- Tighten qemu-img rules on missing backing format
- qemu-img map: Don't limit block status request size
- Fix crash with virtio-scsi and iothreads
# gpg: Signature made Tue 14 Jul 2020 14:24:19 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:
block: Avoid stale pointer dereference in blk_get_aio_context()
qemu-img: Deprecate use of -b without -F
block: Add support to warn on backing file change without format
iotests: Specify explicit backing format where sensible
qcow2: Deprecate use of qemu-img amend to change backing file
block: Error if backing file fails during creation without -u
qcow: Tolerate backing_fmt=
vmdk: Add trivial backing_fmt support
sheepdog: Add trivial backing_fmt support
block: Finish deprecation of 'qemu-img convert -n -o'
qemu-img: Flush stdout before before potential stderr messages
file-posix: Mitigate file fragmentation with extent size hints
iotests/059: Filter out disk size with more standard filter
qemu-img map: Don't limit block status request size
iotests: Simplify _filter_img_create() a bit
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
This includes some vector extensions fixes, a PMP bug fix, OpenTitan
UART bug fix and support for OpenSBI dynamic firmware.
-----BEGIN PGP SIGNATURE-----
iQEzBAABCAAdFiEE9sSsRtSTSGjTuM6PIeENKd+XcFQFAl8M/HgACgkQIeENKd+X
cFTFDAf7BiC0iVDUNEdK91MU3eLBf3C+VcVXeFQ1U4WtQutajhC681jWtk4gemRW
QnZ0HWuOkvvKrdrPqV18c6gKYg+qcgpQ/JMCtl2bFk41nfVLS2Amlza6ycooQAhK
dMrwFDm0yRGy3gjsZwNaduQKaKWJqtZJc142yELtfgfJvNsHJirYKMt1YXMC/pJO
62Z5kACbSVsUDAr02ZZnFw9PX09FQh75LZpfRC9haMpyqkyffARmsu6rAtZJpk1G
XhXhJNq9j3IpBP0nt9BV7KNVW5KrbKnGwEnK+I5UZfEYmGrz4RFb+UWq/rqMF2ui
fbe9tY2bJRwcnS+EbF0s97M6wEweSw==
=T+nM
-----END PGP SIGNATURE-----
Merge remote-tracking branch 'remotes/alistair/tags/pull-riscv-to-apply-20200713' into staging
This is a colection of bug fixes and small imrprovements for RISC-V.
This includes some vector extensions fixes, a PMP bug fix, OpenTitan
UART bug fix and support for OpenSBI dynamic firmware.
# gpg: Signature made Tue 14 Jul 2020 01:29:44 BST
# gpg: using RSA key F6C4AC46D4934868D3B8CE8F21E10D29DF977054
# gpg: Good signature from "Alistair Francis <alistair@alistair23.me>" [full]
# Primary key fingerprint: F6C4 AC46 D493 4868 D3B8 CE8F 21E1 0D29 DF97 7054
* remotes/alistair/tags/pull-riscv-to-apply-20200713:
target/riscv: Fix pmp NA4 implementation
tcg/riscv: Remove superfluous breaks
hw/char: Convert the Ibex UART to use the registerfields API
hw/char: Convert the Ibex UART to use the qdev Clock model
target/riscv: fix vill bit index in vtype register
target/riscv: fix return value of do_opivx_widen()
target/riscv: correct the gvec IR called in gen_vec_rsub16_i64()
target/riscv: fix rsub gvec tcg_assert_listed_vecop assertion
hw/riscv: Modify MROM size to end at 0x10000
RISC-V: Support 64 bit start address
riscv: Add opensbi firmware dynamic support
RISC-V: Copy the fdt in dram instead of ROM
riscv: Unify Qemu's reset vector code path
hw/riscv: virt: Sort the SoC memmap table entries
MAINTAINERS: Add an entry for OpenSBI firmware
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Only move the state machine to ReceivingData if there is no
pending error. This avoids later OOB access while processing
commands queued.
"SD Specifications Part 1 Physical Layer Simplified Spec. v3.01"
4.3.3 Data Read
Read command is rejected if BLOCK_LEN_ERROR or ADDRESS_ERROR
occurred and no data transfer is performed.
4.3.4 Data Write
Write command is rejected if BLOCK_LEN_ERROR or ADDRESS_ERROR
occurred and no data transfer is performed.
WP_VIOLATION errors are not modified: the error bit is set, we
stay in receive-data state, wait for a stop command. All further
data transfer is ignored. See the check on sd->card_status at the
beginning of sd_read_data() and sd_write_data().
Fixes: CVE-2020-13253
Cc: qemu-stable@nongnu.org
Reported-by: Alexander Bulekov <alxndr@bu.edu>
Buglink: https://bugs.launchpad.net/qemu/+bug/1880822
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20200630133912.9428-6-f4bug@amsat.org>
To make the next commit easier to review, clean this code first.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Alexander Bulekov <alxndr@bu.edu>
Message-Id: <20200630133912.9428-3-f4bug@amsat.org>
QEMU allows to create SD card with unrealistic sizes. This could
work, but some guests (at least Linux) consider sizes that are not
a power of 2 as a firmware bug and fix the card size to the next
power of 2.
While the possibility to use small SD card images has been seen as
a feature, it became a bug with CVE-2020-13253, where the guest is
able to do OOB read/write accesses past the image size end.
In a pair of commits we will fix CVE-2020-13253 as:
Read command is rejected if BLOCK_LEN_ERROR or ADDRESS_ERROR
occurred and no data transfer is performed.
Write command is rejected if BLOCK_LEN_ERROR or ADDRESS_ERROR
occurred and no data transfer is performed.
WP_VIOLATION errors are not modified: the error bit is set, we
stay in receive-data state, wait for a stop command. All further
data transfer is ignored. See the check on sd->card_status at the
beginning of sd_read_data() and sd_write_data().
While this is the correct behavior, in case QEMU create smaller SD
cards, guests still try to access past the image size end, and QEMU
considers this is an invalid address, thus "all further data transfer
is ignored". This is wrong and make the guest looping until
eventually timeouts.
Fix by not allowing invalid SD card sizes (suggesting the expected
size as a hint):
$ qemu-system-arm -M orangepi-pc -drive file=rootfs.ext2,if=sd,format=raw
qemu-system-arm: Invalid SD card size: 60 MiB
SD card size has to be a power of 2, e.g. 64 MiB.
You can resize disk images with 'qemu-img resize <imagefile> <new-size>'
(note that this will lose data if you make the image smaller than it currently is).
Cc: qemu-stable@nongnu.org
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20200713183209.26308-8-f4bug@amsat.org>
We don't need to check if sd->blk is set twice.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20200630133912.9428-18-f4bug@amsat.org>
Only SCSD cards support Class 6 (Block Oriented Write Protection)
commands.
"SD Specifications Part 1 Physical Layer Simplified Spec. v3.01"
4.3.14 Command Functional Difference in Card Capacity Types
* Write Protected Group
SDHC and SDXC do not support write-protected groups. Issuing
CMD28, CMD29 and CMD30 generates the ILLEGAL_COMMAND error.
Cc: qemu-stable@nongnu.org
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Message-Id: <20200630133912.9428-7-f4bug@amsat.org>
In few commits we won't allow SD card images with invalid size
(not aligned to a power of 2). Prepare the tests: add the
pow2ceil() and image_pow2ceil_expand() methods and resize the
images (expanding) of the tests using SD cards.
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20200713183209.26308-5-f4bug@amsat.org>
Avocado tags are handy to automatically select tests matching
the tags. Since these tests use a SD card, tag them.
We can run all the tests using a SD card at once with:
$ avocado --show=app run -t u-boot tests/acceptance/
$ AVOCADO_ALLOW_LARGE_STORAGE=ok \
avocado --show=app \
run -t device:sd tests/acceptance/
Fetching asset from tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_sd
Fetching asset from tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic
Fetching asset from tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9
(1/3) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_sd: PASS (19.56 s)
(2/3) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_bionic: PASS (49.97 s)
(3/3) tests/acceptance/boot_linux_console.py:BootLinuxConsole.test_arm_orangepi_uboot_netbsd9: PASS (20.06 s)
RESULTS : PASS 3 | ERROR 0 | FAIL 0 | SKIP 0 | WARN 0 | INTERRUPT 0 | CANCEL 0
JOB TIME : 90.02 s
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Reviewed-by: Cleber Rosa <crosa@redhat.com>
Tested-by: Cleber Rosa <crosa@redhat.com>
Message-Id: <20200713183209.26308-4-f4bug@amsat.org>
SD cards need to have a size of a power of two.
Update the Orange Pi machine documentation to include
instructions for resizing downloaded images using the
qemu-img command.
Signed-off-by: Niek Linnenbank <nieklinnenbank@gmail.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-Id: <20200712183708.15450-1-nieklinnenbank@gmail.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
We forgot to include the qemu-block mailing list while adding
this section in commit 076a0fc32a. Fix this.
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Message-Id: <20200630133912.9428-2-f4bug@amsat.org>
It is possible for blk_remove_bs() to race with blk_drain_all(), causing
the latter to dereference a stale blk->root pointer:
blk_remove_bs(blk)
bdrv_root_unref_child(blk->root)
child_bs = blk->root->bs
bdrv_detach_child(blk->root)
...
g_free(blk->root) <============== blk->root becomes stale
bdrv_unref(child_bs) <============ yield at some point
A blk_drain_all() can be triggered by some guest action in the
meantime, eg. on POWER, SLOF might disable bus mastering on
a virtio-scsi-pci device:
virtio_write_config()
virtio_pci_stop_ioeventfd()
virtio_bus_stop_ioeventfd()
virtio_scsi_dataplane_stop()
blk_drain_all()
blk_get_aio_context()
bs = blk->root ? blk->root->bs : NULL
^^^^^^^^^
stale
Then, depending on one's luck, QEMU either crashes with SEGV or
hits the assertion in blk_get_aio_context().
blk->root is set by blk_insert_bs() which calls bdrv_root_attach_child()
first. The blk_remove_bs() function should rollback the changes made
by blk_insert_bs() in the opposite order (or it should be documented
somewhere why this isn't the case). Clear blk->root before calling
bdrv_root_unref_child() in blk_remove_bs().
Signed-off-by: Greg Kurz <groug@kaod.org>
Message-Id: <159430264541.389456.11925072456012783045.stgit@bahia.lan>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Creating an image that requires format probing of the backing image is
potentially unsafe (we've had several CVEs over the years based on
probes leaking information to the guest on a subsequent boot, although
these days tools like libvirt are aware of the issue enough to prevent
the worst effects). For example, if our probing algorithm ever
changes, or if other tools like libvirt determine a different probe
result than we do, then subsequent use of that backing file under a
different format will present corrupted data to the guest.
Fortunately, the worst effects occur only when the backing image is
originally raw, and we at least prevent commit into a probed raw
backing file that would change its probed type.
Still, it is worth starting a deprecation clock so that future
qemu-img can refuse to create backing chains that would rely on
probing, to encourage clients to avoid unsafe practices. Most
warnings are intentionally emitted from bdrv_img_create() in the block
layer, but qemu-img convert uses bdrv_create() which cannot emit its
own warning without causing spurious warnings on other code paths. In
the end, all command-line image creation or backing file rewriting now
performs a check.
Furthermore, if we probe a backing file as non-raw, then it is safe to
explicitly record that result (rather than relying on future probes);
only where we probe a raw image do we care about further warnings to
the user when using such an image (for example, commits into a
probed-raw backing file are prevented), to help them improve their
tooling. But whether or not we make the probe results explicit, we
still warn the user to remind them to upgrade their workflow to supply
-F always.
iotest 114 specifically wants to create an unsafe image for later
amendment rather than defaulting to our new default of recording a
probed format, so it needs an update. While touching it, expand it to
cover all of the various warnings enabled by this patch. iotest 301
also shows a change to qcow messages.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200706203954.341758-11-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
For now, this is a mechanical addition; all callers pass false. But
the next patch will use it to improve 'qemu-img rebase -u' when
selecting a backing file with no format.
Signed-off-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Message-Id: <20200706203954.341758-10-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
There are many existing qcow2 images that specify a backing file but
no format. This has been the source of CVEs in the past, but has
become more prominent of a problem now that libvirt has switched to
-blockdev. With older -drive, at least the probing was always done by
qemu (so the only risk of a changed format between successive boots of
a guest was if qemu was upgraded and probed differently). But with
newer -blockdev, libvirt must specify a format; if libvirt guesses raw
where the image was formatted, this results in data corruption visible
to the guest; conversely, if libvirt guesses qcow2 where qemu was
using raw, this can result in potential security holes, so modern
libvirt instead refuses to use images without explicit backing format.
The change in libvirt to reject images without explicit backing format
has pointed out that a number of tools have been far too reliant on
probing in the past. It's time to set a better example in our own
iotests of properly setting this parameter.
iotest calls to create, rebase, and convert are all impacted to some
degree. It's a bit annoying that we are inconsistent on command line
- while all of those accept -o backing_file=...,backing_fmt=..., the
shortcuts are different: create and rebase have -b and -F, while
convert has -B but no -F. (amend has no shortcuts, but the previous
patch just deprecated the use of amend to change backing chains).
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200706203954.341758-9-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
The use of 'qemu-img amend' to change qcow2 backing files is not
tested very well. In particular, our implementation has a bug where
if a new backing file is provided without a format, then the prior
format is blindly reused, even if this results in data corruption, but
this is not caught by iotests.
There are also situations where amending other options needs access to
the original backing file (for example, on a downgrade to a v2 image,
knowing whether a v3 zero cluster must be allocated or may be left
unallocated depends on knowing whether the backing file already reads
as zero), but the command line does not have a nice way to tell us
both the backing file to use for opening the image as well as the
backing file to install after the operation is complete.
Even if we do allow changing the backing file, it is redundant with
the existing ability to change backing files via 'qemu-img rebase -u'.
It is time to deprecate this support (leaving the existing behavior
intact, even if it is buggy), and at a point in the future, require
the use of only 'qemu-img rebase' for adjusting backing chain
relations, saving 'qemu-img amend' for changes unrelated to the
backing chain.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200706203954.341758-8-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Back in commit 6e6e55f5 (Jul 2017, v2.10), we tweaked the code to warn
if the backing file could not be opened but the user gave a size,
unless the user also passes the -u option to bypass the open of the
backing file. As one common reason for failure to open the backing
file is when there is mismatch in the requested backing format in
relation to what the backing file actually contains, we actually want
to open the backing file and ensure that it has the right format in as
many cases as possible. iotest 301 for qcow demonstrates how
detecting explicit format mismatch is useful to prevent the creation
of an image that would probe differently than the user requested. Now
is the time to finally turn the warning an error, as promised.
Note that the original warning was added prior to our documentation of
an official deprecation policy (eb22aeca, also Jul 2017), and because
the warning didn't mention the word "deprecated", we never actually
remembered to document it as such. But the warning has been around
long enough that I don't see prolonging it another two releases.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200706203954.341758-7-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
qcow has no space in the metadata to store a backing format, and there
are existing qcow images backed both by raw or by other formats
(usually qcow) images, reliant on probing to tell the difference. On
the bright side, because we probe every time, raw files are marked as
probed and we thus forbid a commit action into the backing file where
guest-controlled contents could change the result of the probe next
time around (the iotest added here proves that).
Still, allowing the user to specify the backing format during
creation, even if we can't record it, is a good thing. This patch
blindly allows any value that resolves to a known driver, even if the
user's request is a mismatch from what probing finds; then the next
patch will further enhance things to verify that the user's request
matches what we actually probe. With this and the next patch in
place, we will finally be ready to deprecate the creation of images
where a backing format was not explicitly specified by the user.
Note that this is only for QemuOpts usage; there is no change to the
QAPI to allow a format through -blockdev.
Add a new iotest 301 just for qcow, to demonstrate the latest
behavior, and to make it easier to show the improvements made in the
next patch.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200706203954.341758-6-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
vmdk already requires that if backing_file is present, that it be
another vmdk image (see vmdk_co_do_create). Meanwhile, we want to
move towards always being explicit about the backing format for other
drivers where it matters. So for convenience, make qemu-img create -F
vmdk work, while rejecting all other explicit formats (note that this
is only for QemuOpts usage; there is no change to the QAPI to allow a
format through -blockdev).
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200706203954.341758-5-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Sheepdog already requires that if backing_file is present, that it be
another sheepdog image (see sd_co_create). Meanwhile, we want to move
towards always being explicit about the backing format for other
drivers where it matters. So for convenience, make qemu-img create -F
sheepdog work, while rejecting all other explicit formats (note that
this is only for QemuOpts usage; there is no change to the QAPI to
allow a format through -blockdev).
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200706203954.341758-4-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
It's been two releases since we started warning; time to make the
combination an error as promised. There was no iotest coverage, so
add some.
While touching the documentation, tweak another section heading for
consistent style.
Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20200706203954.341758-3-eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
