The uninitialized memory allocated for the command FIFO of the
floppy controller during the VM hardware initialization incurs
many unwanted reports by Valgrind when VM state is being saved.
That verbosity hardens a search for the real memory issues when
the iotests run. Particularly, the patch eliminates 20 unnecessary
reports of the Valgrind tool in the iotest #169.
Signed-off-by: Andrey Shinkevich <andrey.shinkevich@virtuozzo.com>
Message-id: 1559154027-282547-1-git-send-email-andrey.shinkevich@virtuozzo.com
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Max Reitz <mreitz@redhat.com>
This adds a new parameter to blk_new() which requires its callers to
declare from which AioContext this BlockBackend is going to be used (or
the locks of which AioContext need to be taken anyway).
The given context is only stored and kept up to date when changing
AioContexts. Actually applying the stored AioContext to the root node
is saved for another commit.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Commit c8a35f1cf0 "fdc: use IsaDma interface instead of global DMA_*
functions" accidentally introduced a segfault in fdctrl_stop_transfer() for
non-DMA transfers.
If fdctrl->dma_chann has not been configured then the fdctrl->dma interface
reference isn't initialised during isabus_fdc_realize(). Unfortunately
fdctrl_stop_transfer() unconditionally references the DMA interface when
finishing the transfer causing a NULL pointer dereference.
Fix the issue by adding a check in fdctrl_stop_transfer() so that the DMA
interface reference and release method is only invoked if fdctrl->dma_chann
has been set.
(This issue was discovered by Martin testing a recent change in the NetBSD
installer under qemu-system-sparc)
Cc: qemu-stable@nongnu.org
Reported-by: Martin Husemann <martin@duskware.de>
Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Reviewed-by: Hervé Poussineau <hpoussin@reactos.org>
Reviewed-by: John Snow <jsnow@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Use assert() instead of error_setg(&error_abort),
as suggested by the "qapi/error.h" documentation:
Please don't error_setg(&error_fatal, ...), use error_report() and
exit(), because that's more obvious.
Likewise, don't error_setg(&error_abort, ...), use assert().
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Acked-by: John Snow <jsnow@redhat.com>
Message-id: 20180625165749.3910-2-f4bug@amsat.org
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
A "powernv" machine type defines an ISA bus but it does not add any DMA
controller to it so it is possible to hit assert(fdctrl->dma) by
adding "-machine powernv -device isa-fdc".
This replaces assert() with an error message.
Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru>
[thuth: Slightly adjusted error message and updated scripts/device-crash-test]
Signed-off-by: Thomas Huth <thuth@redhat.com>
Message-Id: <1521193892-15552-3-git-send-email-thuth@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
[Drop virtio_blk_data_plane_create() change that misinterprets return
value when the virtio transport does not support dataplane.
--Stefan]
Cc: John Snow <jsnow@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: Keith Busch <keith.busch@intel.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-id: e77848d3735ba590f23ffbf8094379c646c33d79.1511317952.git.maozy.fnst@cn.fujitsu.com
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Modify the pre_save method on VMStateDescription to return an int
rather than void so that it potentially can fail.
Changed zillions of devices to make them return 0; the only
case I've made it return non-0 is hw/intc/s390_flic_kvm.c that already
had an error_report/return case.
Note: If you add an error exit in your pre_save you must emit
an error_report to say why.
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Message-Id: <20170925112917.21340-2-dgilbert@redhat.com>
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Convert floppy_drive_init() to realize and rename it to
floppy_drive_realize().
Cc: John Snow <jsnow@redhat.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Max Reitz <mreitz@redhat.com>
Cc: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Mao Zhongyi <maozy.fnst@cn.fujitsu.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 87119b34f32e2acf7166165fb5d8e6fca787b3bc.1505737465.git.maozy.fnst@cn.fujitsu.com
Signed-off-by: John Snow <jsnow@redhat.com>
In some cases a failing VMSTATE_*_EQUAL does not mean we detected a bug,
but it's actually the best we can do. Especially in these cases a verbose
error message is required.
Let's introduce infrastructure for specifying a error hint to be used if
equal check fails. Let's do this by adding a parameter to the _EQUAL
macros called _err_hint. Also change all current users to pass NULL as
last parameter so nothing changes for them.
Signed-off-by: Halil Pasic <pasic@linux.vnet.ibm.com>
Message-Id: <20170623144823.42936-1-pasic@linux.vnet.ibm.com>
Reviewed-by: Juan Quintela <quintela@redhat.com>
Signed-off-by: Juan Quintela <quintela@redhat.com>
The rename prepares for the patch after next's DEFINE_PROP_UNSIGNED().
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-Id: <20170607163635.17635-16-marcandre.lureau@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
[Commit message tweaked]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
This makes all device emulations with a qdev drive property request
permissions on their BlockBackend. The only thing we block at this point
is resizing images for some devices that can't support it.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Acked-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Some devices allow a media change between read-only and read-write
media. They need to adapt the permissions in their .change_media_cb()
implementation, which can fail. So add an Error parameter to the
function.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Acked-by: Fam Zheng <famz@redhat.com>
We want every user to be specific about the permissions it needs, so
we'll pass the initial permissions as parameters to blk_new(). A user
only needs to call blk_set_perm() if it wants to change the permissions
after the fact.
The permissions are stored in the BlockBackend and applied whenever a
BlockDriverState should be attached in blk_insert_bs().
This does not include actually choosing the right set of permissions
everywhere yet. Instead, the usual FIXME comment is added to each place
and will be addressed in individual patches.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Acked-by: Fam Zheng <famz@redhat.com>
This makes the FloppyDrive qdev object actually useful: Now that it has
all properties that don't belong to the controller, you can actually
use '-device floppy' and get a working result.
Command line semantics is consistent with CD-ROM drives: By default you
get a single empty floppy drive. You can override it with -drive and
using the same index, but if you use -drive to add a floppy to a
different index, you get both of them. However, as soon as you use any
'-device floppy', even to a different slot, the default drive is
disabled.
Using '-device floppy' without specifying the unit will choose the first
free slot on the controller.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Message-id: 1477386868-21826-4-git-send-email-kwolf@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
Floppy controllers automatically create two floppy drive devices in qdev
now. (They always created two drives, but managed them only internally.)
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1477386868-21826-3-git-send-email-kwolf@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
This adds a qbus to the floppy controller that should contain the floppy
drives eventually. At the moment it just exists and is empty.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1477386868-21826-2-git-send-email-kwolf@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
The isa_register_portio_list() function allocates ioports
data/state. Let's keep the reference to this data on some owner. This
isn't enough to fix leaks, but at least, ASAN stops complaining of
direct leaks. Further cleanup would require calling
portio_list_del/destroy().
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Sector-based blk_write() should die; switch to byte-based
blk_pwrite() instead. Likewise for blk_read().
Signed-off-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
This patch replaces get_ticks_per_sec() calls with the macro
NANOSECONDS_PER_SECOND. Also, as there are no callers, get_ticks_per_sec()
is then removed. This replacement improves the readability and
understandability of code.
For example,
timer_mod(fdctrl->result_timer,
qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL) + (get_ticks_per_sec() / 50));
NANOSECONDS_PER_SECOND makes it obvious that qemu_clock_get_ns
matches the unit of the expression on the right side of the plus.
Signed-off-by: Rutuja Shah <rutu.shah.26@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Commit 57cb38b included qapi/error.h into qemu/osdep.h to get the
Error typedef. Since then, we've moved to include qemu/osdep.h
everywhere. Its file comment explains: "To avoid getting into
possible circular include dependencies, this file should not include
any other QEMU headers, with the exceptions of config-host.h,
compiler.h, os-posix.h and os-win32.h, all of which are doing a
similar job to this file and are under similar constraints."
qapi/error.h doesn't do a similar job, and it doesn't adhere to
similar constraints: it includes qapi-types.h. That's in excess of
100KiB of crap most .c files don't actually need.
Add the typedef to qemu/typedefs.h, and include that instead of
qapi/error.h. Include qapi/error.h in .c files that need it and don't
get it now. Include qapi-types.h in qom/object.h for uint16List.
Update scripts/clean-includes accordingly. Update it further to match
reality: replace config.h by config-target.h, add sysemu/os-posix.h,
sysemu/os-win32.h. Update the list of includes in the qemu/osdep.h
comment quoted above similarly.
This reduces the number of objects depending on qapi/error.h from "all
of them" to less than a third. Unfortunately, the number depending on
qapi-types.h shrinks only a little. More work is needed for that one.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
[Fix compilation without the spice devel packages. - Paolo]
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
When populating ACPI objects for floppy drives one needs to provide the
maximum values for cylinder, sector, and head number the drive supports.
This patch adds a function that iterates through the array of predefined
floppy drive formats and returns the maximum values of c, h, s, out of
those matching the given floppy drive type.
Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
Cc: Igor Mammedov <imammedo@redhat.com>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Marcel Apfelbaum <marcel@redhat.com>
Cc: John Snow <jsnow@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Kevin O'Connor <kevin@koconnor.net>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Coverity noticed that some variables are only used by debug prints, and
called them unused. Always compile the print statements. While we're
here, print to stderr as well.
Bonus: Fix a debug printf I broke in f31937aa8
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[Touched up commit message. --js]
Message-id: 1454971529-14830-1-git-send-email-jsnow@redhat.com
All functions relative to DMA (DMA_*() functions) are stubs on sparc platform.
Disable the DMA in the floppy controller, instead of calling these stubs.
Signed-off-by: Hervé Poussineau <hpoussin@reactos.org>
Message-id: 1453843944-26833-14-git-send-email-hpoussin@reactos.org
Signed-off-by: John Snow <jsnow@redhat.com>
Accidentally, I removed a "feature" where empty drives had geometry
values applied to them, which allows seek on empty drives to work
"by accident," as QEMU actually tries to disallow that.
Seeks on empty drives should work, though, but the easiest thing is to
restore the misfeature where empty drives have non-zero geometries
applied.
Document the hack accordingly.
[Maintainer edit]
This fix corrects a regression introduced in d5d47efc, where
pick_geometry was modified such that it would not operate on empty
drives, and as a result if there is no diskette inserted, QEMU
no longer populates it with geometry bounds. As a result, seek fails
when QEMU denies to move the current track, but reports success anyway.
This can confuse the guest, leading to kernel panics in the guest.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1454106932-17236-1-git-send-email-jsnow@redhat.com
This reverts the changes that commit
2e1280e8ff applied to hw/block/fdc.c;
also, an additional case of drv->media_inserted use has crept in since,
which is replaced by a call to blk_is_inserted().
That commit changed tests/fdc-test.c, too, because after it, one less
TRAY_MOVED event would be emitted when executing 'change' on an empty
drive. However, now, no TRAY_MOVED events will be emitted at all, and
the tray_open status returned by query-block will always be false,
necessitating (different) changes to tests/fdc-test.c and iotest 118,
which is why this patch is not a pure revert of said commit.
Signed-off-by: Max Reitz <mreitz@redhat.com>
Message-id: 1454096953-31773-4-git-send-email-mreitz@redhat.com
Reviewed-by: Eric Blake <eblake@redhat.com>
The 2.88 drive is more suitable as a default because
it can still read 1.44 images correctly, but the reverse
is not true.
Since there exist virtio-win drivers that are shipped on
2.88 floppy images, this patch will allow VMs booted without
a floppy disk inserted to later insert a 2.88MB floppy and
have that work.
This patch has been tested with msdos, freedos, fedora,
windows 8 and windows 10 without issue: if problems do
arise for certain guests being unable to cope with 2.88MB
drives as the default, they are in the minority and can use
type=144 as needed (or insert a proper boot medium and omit
type=144/288 or use type=auto) to obtain different drive types.
As icing, the default will remain auto/144 for any pre-2.6
machine types, hopefully minimizing the impact of this change
in legacy hw to basically zero.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1453495865-9649-13-git-send-email-jsnow@redhat.com
This one is the crazy one.
fd_revalidate currently uses pick_geometry to tell if the diskette
geometry has changed upon an eject/insert event, but it won't allow us
to insert a 1.44MB diskette into a 2.88MB drive. This is inflexible.
The new algorithm applies a new heuristic to guessing disk geometries
that allows us to switch diskette types as long as the physical size
matches before falling back to the old heuristic.
The old one is roughly:
- If the size (sectors) and type matches, choose it.
- Fall back to the first geometry that matched our type.
The new one is:
- If the size (sectors) and type matches, choose it.
- If the size (sectors) and physical size match, choose it.
- Fall back to the first geometry that matched our type.
Signed-off-by: John Snow <jsnow@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Message-id: 1453495865-9649-11-git-send-email-jsnow@redhat.com
2.88MB capable drives can accept 1.44MB floppies,
for instance. To rework the pick_geometry function,
we need to know if our current drive can even accept
the type of disks we're considering.
NB: This allows us to distinguish between all of the
"total sectors" collisions between 1.20MB and 1.44MB
diskette types, by using the physical drive size as a
differentiator.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1453495865-9649-10-git-send-email-jsnow@redhat.com
This patch adds a new explicit Floppy Drive Type option. The existing
behavior in QEMU is to automatically guess a drive type based on the
media inserted, or if a diskette is not present, arbitrarily assign one.
This behavior can be described as "auto." This patch adds the option
to pick an explicit behavior: 120, 144, 288 or none. The new "auto"
option is intended to mimic current behavior, while the other types
pick one explicitly.
Set the type given by the CLI during fd_init. If the type remains the
default (auto), we'll attempt to scan an inserted diskette if present
to determine a type. If auto is selected but no diskette is present,
we fall back to a predetermined default (currently 1.44MB to match
legacy QEMU behavior.)
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1453495865-9649-9-git-send-email-jsnow@redhat.com
Currently, QEMU chooses a drive type automatically based on the inserted
media. If there is no disk inserted, it chooses a 1.44MB drive type.
Change this behavior to be configurable, but leave it defaulted to 1.44.
This is not earnestly intended to be used by a user or a management
library, but rather exists so that pre-2.6 board types can configure it
to be a legacy value.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1453495865-9649-8-git-send-email-jsnow@redhat.com
Split apart pick_geometry by creating a pick_drive routine that will only
ever called during device bring-up instead of relying on pick_geometry to
be used in both cases.
With this change, the drive field is changed to be 'write once'. It is
not altered after the initialization routines exit.
media_validated does not need to be migrated. The target VM
will just revalidate the media on post_load anyway.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1453495865-9649-7-git-send-email-jsnow@redhat.com
pick_geometry is a convoluted function that makes it difficult to tell
at a glance what QEMU's current behavior for choosing a floppy drive
type is when it can't quite identify the diskette.
The code iterates over all entries in the candidate geometry table
("fd_formats") and if our specific drive type matches a row in the table,
then either "match" is set to that entry (an exact match) and the loop
exits, or "first_match" will be non-negative (the first such entry that
shares the same drive type), and the loop continues. If our specific
drive type is NONE, then all drive types in the candidate geometry table
are considered. After iteration, if "match" was not set, we fall back to
"first match".
This means that either "match" was set, or we exited the loop without an
exact match, in which case:
- If drive type is NONE, the default is truly fd_formats[0], a 1.44MB
type, because "first_match" will always get set to the first item.
- If drive type is not NONE, pick_geometry's iteration was fussier and
only looked at rows that matched our drive type. However, since all
possible drive types are represented in the table, we still know that
"first match" was set.
- If drive type is not NONE and the fd_formats table lists no options for
our drive type, we choose fd_formats[1], an incomprehensibly bizarre
choice that can never happen anyway.
Correct this: If first_match is -1, it can ONLY mean we didn't edit our
fd_formats table correctly. Throw an assertion instead.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1453495865-9649-6-git-send-email-jsnow@redhat.com
Currently, 'drive' is used both to represent the current diskette
type as well as the current drive type.
This patch adds a 'disk' field that is updated explicitly to match
the type of the disk.
As of this patch, disk and drive are always the same, but forthcoming
patches to change the behavior of pick_geometry will invalidate this
assumption.
disk does not need to be migrated because it is not user-visible state
nor is it currently used for any calculations. It is purely informative,
and will be rebuilt automatically via fd_revalidate on the new host.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1453495865-9649-5-git-send-email-jsnow@redhat.com
Change the floppy drive type to a QAPI enum type, to allow us to
specify the floppy drive type from the CLI in a forthcoming patch.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1453495865-9649-4-git-send-email-jsnow@redhat.com
Modify this function to operate directly on FDrive objects instead of
unpacking and passing all of those parameters manually. Reduces the
complexity in the caller and reduces the number of args to just one.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1453495865-9649-3-git-send-email-jsnow@redhat.com
Code motion: I want to refactor this function to work with FDrive
directly, so shuffle it below that definition.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: John Snow <jsnow@redhat.com>
Message-id: 1453495865-9649-2-git-send-email-jsnow@redhat.com
Clean up includes so that osdep.h is included first and headers
which it implies are not included manually.
This commit was created with scripts/clean-includes.
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <1449764955-10741-3-git-send-email-armbru@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
The tray of an FDD is open iff there is no medium inserted (there are
only two states for an FDD: "medium inserted" or "no medium inserted").
Signed-off-by: Max Reitz <mreitz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
The i8257 DMA controller uses an idle bottom half, which by default
does not cause the main loop to exit. Therefore, the DMA_schedule
function is there to ensure that the CPU relinquishes the iothread
mutex to the iothread.
However, this is not enough since the iothread will call
aio_compute_timeout() and go to sleep again. In the iothread
world, forcing execution of the idle bottom half is much simpler,
and only requires a call to qemu_notify_event(). Do it, removing
the need for the "cpu_request_exit" pseudo-irq. The next patch
will remove it.
Reviewed-by: Richard Henderson <rth@twiddle.net>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
We create optional sections with this patch. But we already have
optional subsections. Instead of having two mechanism that do the
same, we can just generalize it.
For subsections we just change:
- Add a needed function to VMStateDescription
- Remove VMStateSubsection (after removal of the needed function
it is just a VMStateDescription)
- Adjust the whole tree, moving the needed function to the corresponding
VMStateDescription
Signed-off-by: Juan Quintela <quintela@redhat.com>
The RQM bit in MSR should be set whenever the guest is supposed to
access the FIFO, and it should be cleared in all other cases. This is
important so the guest can't continue writing/reading the FIFO beyond
the length that it's suppossed to access (see CVE-2015-3456).
Commit e9077462 fixed the CVE by adding code that avoids the buffer
overflow; however it doesn't correct the wrong behaviour of the floppy
controller which should already have cleared RQM.
Currently, RQM stays set all the time and during all phases while a
command is being processed. This is error-prone because the command has
to explicitly clear the flag if it doesn't need data (and indeed, the
two buggy commands that are the culprits for the CVE just forgot to do
that).
This patch clears RQM immediately as soon as all bytes that are expected
have been received. If the the FIFO is used in the next phase, the flag
has to be set explicitly there.
It also clear RQM after receiving all bytes even if the phase transition
immediately sets it again. While it's technically not necessary at the
moment because the state between clearing and setting RQM is not
observable by the guest, this is more explicit and matches how real
hardware works. It will actually become necessary in qemu once
asynchronous code paths are introduced.
This alone should have been enough to fix the CVE, but now we have two
lines of defense - even better.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1432214378-31891-8-git-send-email-kwolf@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
This commit makes similar improvements as have already been made to the
write function: Instead of relying on a flag in the MSR to distinguish
controller phases, use the explicit phase that we store now. Assertions
of the right MSR flags are added.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1432214378-31891-7-git-send-email-kwolf@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
Factor out a few common lines of code, reformat, improve comments.
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
Message-id: 1432214378-31891-6-git-send-email-kwolf@redhat.com
Signed-off-by: John Snow <jsnow@redhat.com>
