a092c513 | 04-Sep-2024 |
Markus Armbruster <armbru@redhat.com> |
qapi/crypto: Rename QCryptoCipherAlgorithm to *Algo, and drop prefix
QAPI's 'prefix' feature can make the connection between enumeration type and its constants less than obvious. It's best used wit
qapi/crypto: Rename QCryptoCipherAlgorithm to *Algo, and drop prefix
QAPI's 'prefix' feature can make the connection between enumeration type and its constants less than obvious. It's best used with restraint.
QCryptoCipherAlgorithm has a 'prefix' that overrides the generated enumeration constants' prefix to QCRYPTO_CIPHER_ALG.
We could simply drop 'prefix', but then the prefix becomes QCRYPTO_CIPHER_ALGORITHM, which is rather long.
We could additionally rename the type to QCryptoCipherAlg, but I think the abbreviation "alg" is less than clear.
Rename the type to QCryptoCipherAlgo instead. The prefix becomes QCRYPTO_CIPHER_ALGO.
Signed-off-by: Markus Armbruster <armbru@redhat.com> Acked-by: Daniel P. Berrangé <berrange@redhat.com> Message-ID: <20240904111836.3273842-13-armbru@redhat.com>
show more ...
|
547c4e50 | 08-Aug-2024 |
Stefano Garzarella <sgarzare@redhat.com> |
block/blkio: use FUA flag on write zeroes only if supported
libblkio supports BLKIO_REQ_FUA with write zeros requests only since version 1.4.0, so let's inform the block layer that the blkio driver
block/blkio: use FUA flag on write zeroes only if supported
libblkio supports BLKIO_REQ_FUA with write zeros requests only since version 1.4.0, so let's inform the block layer that the blkio driver supports it only in this case. Otherwise we can have runtime errors as reported in https://issues.redhat.com/browse/RHEL-32878
Fixes: fd66dbd424 ("blkio: add libblkio block driver") Cc: qemu-stable@nongnu.org Buglink: https://issues.redhat.com/browse/RHEL-32878 Signed-off-by: Stefano Garzarella <sgarzare@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Message-id: 20240808080545.40744-1-sgarzare@redhat.com Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
show more ...
|
c8a76dbd | 06-Aug-2024 |
Eric Blake <eblake@redhat.com> |
nbd/server: CVE-2024-7409: Cap default max-connections to 100
Allowing an unlimited number of clients to any web service is a recipe for a rudimentary denial of service attack: the client merely nee
nbd/server: CVE-2024-7409: Cap default max-connections to 100
Allowing an unlimited number of clients to any web service is a recipe for a rudimentary denial of service attack: the client merely needs to open lots of sockets without closing them, until qemu no longer has any more fds available to allocate.
For qemu-nbd, we default to allowing only 1 connection unless more are explicitly asked for (-e or --shared); this was historically picked as a nice default (without an explicit -t, a non-persistent qemu-nbd goes away after a client disconnects, without needing any additional follow-up commands), and we are not going to change that interface now (besides, someday we want to point people towards qemu-storage-daemon instead of qemu-nbd).
But for qemu proper, and the newer qemu-storage-daemon, the QMP nbd-server-start command has historically had a default of unlimited number of connections, in part because unlike qemu-nbd it is inherently persistent until nbd-server-stop. Allowing multiple client sockets is particularly useful for clients that can take advantage of MULTI_CONN (creating parallel sockets to increase throughput), although known clients that do so (such as libnbd's nbdcopy) typically use only 8 or 16 connections (the benefits of scaling diminish once more sockets are competing for kernel attention). Picking a number large enough for typical use cases, but not unlimited, makes it slightly harder for a malicious client to perform a denial of service merely by opening lots of connections withot progressing through the handshake.
This change does not eliminate CVE-2024-7409 on its own, but reduces the chance for fd exhaustion or unlimited memory usage as an attack surface. On the other hand, by itself, it makes it more obvious that with a finite limit, we have the problem of an unauthenticated client holding 100 fds opened as a way to block out a legitimate client from being able to connect; thus, later patches will further add timeouts to reject clients that are not making progress.
This is an INTENTIONAL change in behavior, and will break any client of nbd-server-start that was not passing an explicit max-connections parameter, yet expects more than 100 simultaneous connections. We are not aware of any such client (as stated above, most clients aware of MULTI_CONN get by just fine on 8 or 16 connections, and probably cope with later connections failing by relying on the earlier connections; libvirt has not yet been passing max-connections, but generally creates NBD servers with the intent for a single client for the sake of live storage migration; meanwhile, the KubeSAN project anticipates a large cluster sharing multiple clients [up to 8 per node, and up to 100 nodes in a cluster], but it currently uses qemu-nbd with an explicit --shared=0 rather than qemu-storage-daemon with nbd-server-start).
We considered using a deprecation period (declare that omitting max-parameters is deprecated, and make it mandatory in 3 releases - then we don't need to pick an arbitrary default); that has zero risk of breaking any apps that accidentally depended on more than 100 connections, and where such breakage might not be noticed under unit testing but only under the larger loads of production usage. But it does not close the denial-of-service hole until far into the future, and requires all apps to change to add the parameter even if 100 was good enough. It also has a drawback that any app (like libvirt) that is accidentally relying on an unlimited default should seriously consider their own CVE now, at which point they are going to change to pass explicit max-connections sooner than waiting for 3 qemu releases. Finally, if our changed default breaks an app, that app can always pass in an explicit max-parameters with a larger value.
It is also intentional that the HMP interface to nbd-server-start is not changed to expose max-connections (any client needing to fine-tune things should be using QMP).
Suggested-by: Daniel P. Berrangé <berrange@redhat.com> Signed-off-by: Eric Blake <eblake@redhat.com> Message-ID: <20240807174943.771624-12-eblake@redhat.com> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> [ericb: Expand commit message to summarize Dan's argument for why we break corner-case back-compat behavior without a deprecation period] Signed-off-by: Eric Blake <eblake@redhat.com>
show more ...
|
5eed3db3 | 20-Jul-2024 |
Amjad Alsharafi <amjadsharafi10@gmail.com> |
vvfat: Fix reading files with non-continuous clusters
When reading with `read_cluster` we get the `mapping` with `find_mapping_for_cluster` and then we call `open_file` for this mapping. The issue a
vvfat: Fix reading files with non-continuous clusters
When reading with `read_cluster` we get the `mapping` with `find_mapping_for_cluster` and then we call `open_file` for this mapping. The issue appear when its the same file, but a second cluster that is not immediately after it, imagine clusters `500 -> 503`, this will give us 2 mappings one has the range `500..501` and another `503..504`, both point to the same file, but different offsets.
When we don't open the file since the path is the same, we won't assign `s->current_mapping` and thus accessing way out of bound of the file.
From our example above, after `open_file` (that didn't open anything) we will get the offset into the file with `s->cluster_size*(cluster_num-s->current_mapping->begin)`, which will give us `0x2000 * (504-500)`, which is out of bound for this mapping and will produce some issues.
Signed-off-by: Amjad Alsharafi <amjadsharafi10@gmail.com> Message-ID: <1f3ea115779abab62ba32c788073cdc99f9ad5dd.1721470238.git.amjadsharafi10@gmail.com> [kwolf: Simplified the patch based on Amjad's analysis and input] Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
f60a6f7e | 20-Jul-2024 |
Amjad Alsharafi <amjadsharafi10@gmail.com> |
vvfat: Fix wrong checks for cluster mappings invariant
How this `abort` was intended to check for was: - if the `mapping->first_mapping_index` is not the same as `first_mapping_index`, which **sho
vvfat: Fix wrong checks for cluster mappings invariant
How this `abort` was intended to check for was: - if the `mapping->first_mapping_index` is not the same as `first_mapping_index`, which **should** happen only in one case, when we are handling the first mapping, in that case `mapping->first_mapping_index == -1`, in all other cases, the other mappings after the first should have the condition `true`. - From above, we know that this is the first mapping, so if the offset is not `0`, then abort, since this is an invalid state.
The issue was that `first_mapping_index` is not set if we are checking from the middle, the variable `first_mapping_index` is only set if we passed through the check `cluster_was_modified` with the first mapping, and in the same function call we checked the other mappings.
One approach is to go into the loop even if `cluster_was_modified` is not true so that we will be able to set `first_mapping_index` for the first mapping, but since `first_mapping_index` is only used here, another approach is to just check manually for the `mapping->first_mapping_index != -1` since we know that this is the value for the only entry where `offset == 0` (i.e. first mapping).
Signed-off-by: Amjad Alsharafi <amjadsharafi10@gmail.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <b0fbca3ee208c565885838f6a7deeaeb23f4f9c2.1721470238.git.amjadsharafi10@gmail.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
21b25a0e | 20-Jul-2024 |
Amjad Alsharafi <amjadsharafi10@gmail.com> |
vvfat: Fix usage of `info.file.offset`
The field is marked as "the offset in the file (in clusters)", but it was being used like this `cluster_size*(nums)+mapping->info.file.offset`, which is incorr
vvfat: Fix usage of `info.file.offset`
The field is marked as "the offset in the file (in clusters)", but it was being used like this `cluster_size*(nums)+mapping->info.file.offset`, which is incorrect.
Signed-off-by: Amjad Alsharafi <amjadsharafi10@gmail.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <72f19a7903886dda1aa78bcae0e17702ee939262.1721470238.git.amjadsharafi10@gmail.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
b881cf00 | 20-Jul-2024 |
Amjad Alsharafi <amjadsharafi10@gmail.com> |
vvfat: Fix bug in writing to middle of file
Before this commit, the behavior when calling `commit_one_file` for example with `offset=0x2000` (second cluster), what will happen is that we won't fetch
vvfat: Fix bug in writing to middle of file
Before this commit, the behavior when calling `commit_one_file` for example with `offset=0x2000` (second cluster), what will happen is that we won't fetch the next cluster from the fat, and instead use the first cluster for the read operation.
This is due to off-by-one error here, where `i=0x2000 !< offset=0x2000`, thus not fetching the next cluster.
Signed-off-by: Amjad Alsharafi <amjadsharafi10@gmail.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Tested-by: Kevin Wolf <kwolf@redhat.com> Message-ID: <b97c1e1f1bc2f776061ae914f95d799d124fcd73.1721470238.git.amjadsharafi10@gmail.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
d656aaa1 | 04-Sep-2023 |
Paolo Bonzini <pbonzini@redhat.com> |
block: rename former bdrv_file_open callbacks
Since there is no bdrv_file_open callback anymore, rename the implementations so that they end with "_open" instead of "_file_open". NFS is the excepti
block: rename former bdrv_file_open callbacks
Since there is no bdrv_file_open callback anymore, rename the implementations so that they end with "_open" instead of "_file_open". NFS is the exception because all the functions are named nfs_file_*.
Suggested-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
show more ...
|
24687abf | 25-Apr-2024 |
Prasad Pandit <pjp@fedoraproject.org> |
linux-aio: add IO_CMD_FDSYNC command support
Libaio defines IO_CMD_FDSYNC command to sync all outstanding asynchronous I/O operations, by flushing out file data to the disk storage. Enable linux-aio
linux-aio: add IO_CMD_FDSYNC command support
Libaio defines IO_CMD_FDSYNC command to sync all outstanding asynchronous I/O operations, by flushing out file data to the disk storage. Enable linux-aio to submit such aio request.
When using aio=native without fdsync() support, QEMU creates pthreads, and destroying these pthreads results in TLB flushes. In a real-time guest environment, TLB flushes cause a latency spike. This patch helps to avoid such spikes.
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com> Signed-off-by: Prasad Pandit <pjp@fedoraproject.org> Message-ID: <20240425070412.37248-1-ppandit@redhat.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
10b1e09e | 29-Apr-2024 |
Fiona Ebner <f.ebner@proxmox.com> |
block/copy-before-write: use uint64_t for timeout in nanoseconds
rather than the uint32_t for which the maximum is slightly more than 4 seconds and larger values would overflow. The QAPI interface a
block/copy-before-write: use uint64_t for timeout in nanoseconds
rather than the uint32_t for which the maximum is slightly more than 4 seconds and larger values would overflow. The QAPI interface allows specifying the number of seconds, so only values 0 to 4 are safe right now, other values lead to a much lower timeout than a user expects.
The block_copy() call where this is used already takes a uint64_t for the timeout, so no change required there.
Fixes: 6db7fd1ca9 ("block/copy-before-write: implement cbw-timeout option") Reported-by: Friedrich Weber <f.weber@proxmox.com> Signed-off-by: Fiona Ebner <f.ebner@proxmox.com> Message-ID: <20240429141934.442154-1-f.ebner@proxmox.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Kevin Wolf <kwolf@redhat.com> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
show more ...
|
0fd05c8d | 13-Mar-2024 |
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> |
qapi: blockdev-backup: add discard-source parameter
Add a parameter that enables discard-after-copy. That is mostly useful in "push backup with fleecing" scheme, when source is snapshot-access forma
qapi: blockdev-backup: add discard-source parameter
Add a parameter that enables discard-after-copy. That is mostly useful in "push backup with fleecing" scheme, when source is snapshot-access format driver node, based on copy-before-write filter snapshot-access API:
[guest] [snapshot-access] ~~ blockdev-backup ~~> [backup target] | | | root | file v v [copy-before-write] | | | file | target v v [active disk] [temp.img]
In this case discard-after-copy does two things:
- discard data in temp.img to save disk space - avoid further copy-before-write operation in discarded area
Note that we have to declare WRITE permission on source in copy-before-write filter, for discard to work. Still we can't take it unconditionally, as it will break normal backup from RO source. So, we have to add a parameter and pass it thorough bdrv_open flags.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Fiona Ebner <f.ebner@proxmox.com> Tested-by: Fiona Ebner <f.ebner@proxmox.com> Acked-by: Markus Armbruster <armbru@redhat.com> Message-Id: <20240313152822.626493-5-vsementsov@yandex-team.ru> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
show more ...
|
006e845b | 13-Mar-2024 |
Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> |
block/copy-before-write: create block_copy bitmap in filter node
Currently block_copy creates copy_bitmap in source node. But that is in bad relation with .independent_close=true of copy-before-writ
block/copy-before-write: create block_copy bitmap in filter node
Currently block_copy creates copy_bitmap in source node. But that is in bad relation with .independent_close=true of copy-before-write filter: source node may be detached and removed before .bdrv_close() handler called, which should call block_copy_state_free(), which in turn should remove copy_bitmap.
That's all not ideal: it would be better if internal bitmap of block-copy object is not attached to any node. But that is not possible now.
The simplest solution is just create copy_bitmap in filter node, where anyway two other bitmaps are created.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru> Reviewed-by: Fiona Ebner <f.ebner@proxmox.com> Tested-by: Fiona Ebner <f.ebner@proxmox.com> Message-Id: <20240313152822.626493-4-vsementsov@yandex-team.ru> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
show more ...
|