[PATCH 0/6] various fixes for 11.2 (gitlab, tests, char-fe, fdc, deprecations)
This is a rather random selection of fixes that didn't quite make it to 11.1 although we have some qemu-stable material in here. The CFLAGS tweak allow another target to be built and tested on MacOS. There are also riscv64 compilers which could be enabled via brew but I don't have time to look at those. The CFLAGS changes are also subsumed into Pierrick's check-tcg meson updates which might hit first. The char-fe clean-up removes a code smell of device models reaching into qemu_chr_fe_get_driver which is poor practice. The fdc fix resolves one of the many security reports for those that still care about floppy controllers in VMs. In my brief excursion into our confidential bugs I did look at fixing virtio-crypto but found it needs some serious love. Therefor I'm taking the other option of deprecating the module. Finally I'm experimenting with using b4's patch series handling so forgive any finger trouble on this series. Alex. Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- Alex Bennée (6): gitlab: use .base_meson_job_template for macOS jobs tests/arm: add explicit CFLAGS for system build gitlab: add arm-softmmu tests to the macOS build char-fe: implement qemu_chr_fe_backend_name hw/block: validate the fdc sector position fits within bounds docs: deprecate virtio-crypto docs/about/deprecated.rst | 14 ++++++++++++++ hw/block/fdc-internal.h | 1 + include/chardev/char-fe.h | 13 ++++++++++++- backends/cryptodev-vhost-user.c | 8 +------- backends/rng-egd.c | 8 +------- backends/vhost-user.c | 8 +------- hw/block/fdc.c | 28 ++++++++++++++++++++++------ hw/char/imx_serial.c | 4 ++-- .gitlab-ci.d/macos.yml | 7 ++++--- tests/tcg/arm/Makefile.softmmu-target | 2 +- 10 files changed, 59 insertions(+), 34 deletions(-) --- base-commit: fa19879df1658f96ac07365fca8835b7decd6995 change-id: 20260819-fixes-for-11-2-f98744b82c12 Best regards, -- Alex Bennée <alex.bennee@linaro.org>
This way we'll get more environmental information which will help with debugging. Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> Message-ID: <20260723200904.224413-2-alex.bennee@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- .gitlab-ci.d/macos.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.d/macos.yml b/.gitlab-ci.d/macos.yml index 641a48b3832..bdb3eae18f0 100644 --- a/.gitlab-ci.d/macos.yml +++ b/.gitlab-ci.d/macos.yml @@ -1,5 +1,5 @@ .macos_job_template: - extends: .base_job_template + extends: .base_meson_job_template stage: build tags: - saas-macos-large-m2pro -- 2.47.3
The brew toolchain on macOS (arm-none-eabi-gcc) is a little different from the default on Debian (arm-linux-gnueabihf-gcc) as a result it defaults to an older architectural baseline (without isb's), assumes softfloat and will get confused by including stdint.h without being told it is compiling freestanding code. Make these all explicit so we can build arm-softmmu tests on macOS. The test-armv6m-undef is unaffected as it has it's very own build stanza. Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> Message-ID: <20260723200904.224413-3-alex.bennee@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- tests/tcg/arm/Makefile.softmmu-target | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tcg/arm/Makefile.softmmu-target b/tests/tcg/arm/Makefile.softmmu-target index b66074b0b43..3e8a9b4073e 100644 --- a/tests/tcg/arm/Makefile.softmmu-target +++ b/tests/tcg/arm/Makefile.softmmu-target @@ -29,7 +29,7 @@ ARM_TESTS+=$(patsubst $(ARM_SRC)/%.c, %, $(ARM_TEST_SRCS)) CRT_PATH=$(ARM_SRC) LINK_SCRIPT=$(ARM_SRC)/kernel.ld LDFLAGS=-Wl,-T$(LINK_SCRIPT) -CFLAGS+=-nostdlib -ggdb -O0 $(MINILIB_INC) +CFLAGS+=-march=armv7-a+fp -ffreestanding -nostdlib -ggdb -O0 $(MINILIB_INC) LDFLAGS+=-static -nostdlib $(CRT_OBJS) $(MINILIB_OBJS) -lgcc # building head blobs -- 2.47.3
Now we have fixed the compiler issues lets have at it. Reviewed-by: Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com> Message-ID: <20260723200904.224413-4-alex.bennee@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- .gitlab-ci.d/macos.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.gitlab-ci.d/macos.yml b/.gitlab-ci.d/macos.yml index bdb3eae18f0..40b9b637b3b 100644 --- a/.gitlab-ci.d/macos.yml +++ b/.gitlab-ci.d/macos.yml @@ -22,7 +22,7 @@ - export PKG_CONFIG_PATH="$PKG_CONFIG_PATH" - brew update - brew install $PKGS - - brew install aarch64-elf-gcc i686-elf-gcc x86_64-elf-gcc + - brew install arm-none-eabi-gcc aarch64-elf-gcc i686-elf-gcc x86_64-elf-gcc - if test -n "$PYPI_PKGS" ; then PYLIB=$($PYTHON -c 'import sysconfig; print(sysconfig.get_path("stdlib"))'); rm -f $PYLIB/EXTERNALLY-MANAGED; $PIP3 install --break-system-packages $PYPI_PKGS ; fi script: - mkdir build @@ -40,8 +40,9 @@ aarch64-macos-15-build: PATH_EXTRA: /opt/homebrew/gettext/bin PKG_CONFIG_PATH: /opt/homebrew/curl/lib/pkgconfig:/opt/homebrew/ncurses/lib/pkgconfig:/opt/homebrew/readline/lib/pkgconfig CONFIGURE_ARGS: - --target-list=aarch64-softmmu,i386-softmmu,x86_64-softmmu + --target-list=aarch64-softmmu,arm-softmmu,i386-softmmu,x86_64-softmmu --cross-prefix-aarch64=aarch64-elf- + --cross-prefix-arm=arm-none-eabi- --cross-prefix-i386=i686-elf- --cross-prefix-x86_64=x86_64-elf- --disable-plugins -- 2.47.3
A number of chardev users where calling qemu_chr_fe_get_driver() which is discouraged as it blocks backend hotswap. As the pattern is common and need for the string is transitory add a helper and keep qemu_chr_fe_get_driver() for those that really need the funkiness of poking around the backend setup. Suggested-by: Peter Maydell <peter.maydell@linaro.org> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> Message-ID: <20260810153503.2652324-1-alex.bennee@linaro.org> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- v2 - checkpatch fixes - r-b --- include/chardev/char-fe.h | 13 ++++++++++++- backends/cryptodev-vhost-user.c | 8 +------- backends/rng-egd.c | 8 +------- backends/vhost-user.c | 8 +------- hw/char/imx_serial.c | 4 ++-- 5 files changed, 17 insertions(+), 24 deletions(-) diff --git a/include/chardev/char-fe.h b/include/chardev/char-fe.h index 5f8a6df17dc..a82894e1d51 100644 --- a/include/chardev/char-fe.h +++ b/include/chardev/char-fe.h @@ -52,7 +52,8 @@ void qemu_chr_fe_deinit(CharFrontend *c, bool del); * associated Chardev. * Note: avoid this function as the driver should never be accessed directly, * especially by the frontends that support chardevice hotswap. - * Consider qemu_chr_fe_backend_connected() to check for driver existence + * Consider qemu_chr_fe_backend_connected() to check for driver + * existence or qemu_chr_fe_backend_name() if you need the name. */ Chardev *qemu_chr_fe_get_driver(CharFrontend *c); @@ -70,6 +71,16 @@ bool qemu_chr_fe_backend_connected(CharFrontend *c); */ bool qemu_chr_fe_backend_open(CharFrontend *c); +/** + * qemu_chr_fe_backend_name: + * + * Returns: caller freeable string or NULL + */ +static inline char *qemu_chr_fe_backend_name(CharFrontend *c) +{ + return (c->chr && c->chr->label) ? g_strdup(c->chr->label) : NULL; +} + /** * qemu_chr_fe_set_handlers_full: * @c: a CharFrontend diff --git a/backends/cryptodev-vhost-user.c b/backends/cryptodev-vhost-user.c index cc478d9902d..3334e280a27 100644 --- a/backends/cryptodev-vhost-user.c +++ b/backends/cryptodev-vhost-user.c @@ -373,13 +373,7 @@ cryptodev_vhost_user_get_chardev(Object *obj, Error **errp) { CryptoDevBackendVhostUser *s = CRYPTODEV_BACKEND_VHOST_USER(obj); - Chardev *chr = qemu_chr_fe_get_driver(&s->chr); - - if (chr && chr->label) { - return g_strdup(chr->label); - } - - return NULL; + return qemu_chr_fe_backend_name(&s->chr); } static void cryptodev_vhost_user_finalize(Object *obj) diff --git a/backends/rng-egd.c b/backends/rng-egd.c index 1d92bd71cf7..c2207f40c5c 100644 --- a/backends/rng-egd.c +++ b/backends/rng-egd.c @@ -126,13 +126,7 @@ static void rng_egd_set_chardev(Object *obj, const char *value, Error **errp) static char *rng_egd_get_chardev(Object *obj, Error **errp) { RngEgd *s = RNG_EGD(obj); - Chardev *chr = qemu_chr_fe_get_driver(&s->chr); - - if (chr && chr->label) { - return g_strdup(chr->label); - } - - return NULL; + return qemu_chr_fe_backend_name(&s->chr); } static void rng_egd_finalize(Object *obj) diff --git a/backends/vhost-user.c b/backends/vhost-user.c index 380d8250233..46dadb74800 100644 --- a/backends/vhost-user.c +++ b/backends/vhost-user.c @@ -152,13 +152,7 @@ static void set_chardev(Object *obj, const char *value, Error **errp) static char *get_chardev(Object *obj, Error **errp) { VhostUserBackend *b = VHOST_USER_BACKEND(obj); - Chardev *chr = qemu_chr_fe_get_driver(&b->chr); - - if (chr && chr->label) { - return g_strdup(chr->label); - } - - return NULL; + return qemu_chr_fe_backend_name(&b->chr); } static void vhost_user_backend_class_init(ObjectClass *oc, const void *data) diff --git a/hw/char/imx_serial.c b/hw/char/imx_serial.c index fb41ee2ac50..8ac4efbb5e9 100644 --- a/hw/char/imx_serial.c +++ b/hw/char/imx_serial.c @@ -278,10 +278,10 @@ static void imx_serial_write(void *opaque, hwaddr offset, uint64_t value, unsigned size) { IMXSerialState *s = (IMXSerialState *)opaque; - Chardev *chr = qemu_chr_fe_get_driver(&s->chr); + g_autofree char *label = qemu_chr_fe_backend_name(&s->chr); unsigned char ch; - trace_imx_serial_write(chr ? chr->label : "NODEV", offset, value); + trace_imx_serial_write(label ? label : "NODEV", offset, value); switch (offset >> 2) { case 0x10: /* UTXD */ -- 2.47.3
Previously the guest controlled the value of last_sec and wasn't particularly careful to check it didn't exceed the media size. We can't re-use drv->last_sec as that changes as we do operations so we set media_last_sect when the geometry is probed. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3800 Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- v2 - checkpatch --- hw/block/fdc-internal.h | 1 + hw/block/fdc.c | 28 ++++++++++++++++++++++------ 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/hw/block/fdc-internal.h b/hw/block/fdc-internal.h index e219623dc7a..3a39ac903c7 100644 --- a/hw/block/fdc-internal.h +++ b/hw/block/fdc-internal.h @@ -83,6 +83,7 @@ typedef struct FDrive { uint8_t max_track; /* Nb of tracks */ uint16_t bps; /* Bytes per sector */ uint8_t ro; /* Is read-only */ + uint8_t media_last_sect; /* Probed sector per track*/ uint8_t media_changed; /* Is media changed */ uint8_t media_rate; /* Data rate of medium */ diff --git a/hw/block/fdc.c b/hw/block/fdc.c index 1178b959a64..18a61f5ce87 100644 --- a/hw/block/fdc.c +++ b/hw/block/fdc.c @@ -192,6 +192,7 @@ static void fd_init(FDrive *drv) drv->max_track = 0; drv->ro = true; drv->media_changed = 1; + drv->media_last_sect = 0; } #define NUM_SIDES(drv) ((drv)->flags & FDISK_DBL_SIDES ? 2 : 1) @@ -373,6 +374,7 @@ static int pick_geometry(FDrive *drv) } drv->max_track = parse->max_track; drv->last_sect = parse->last_sect; + drv->media_last_sect = parse->last_sect; drv->disk = parse->drive; drv->media_rate = parse->rate; return 0; @@ -1905,6 +1907,17 @@ static void fdctrl_handle_partid(FDCtrl *fdctrl, int direction) fdctrl_to_result_phase(fdctrl, 1); } +static bool fd_validate_last_sect(FDrive *drv, uint8_t new_last_sect_val) +{ + if (drv->media_validated && new_last_sect_val > drv->media_last_sect) { + qemu_log_mask(LOG_GUEST_ERROR, + "FDC: Guest attempted to set last_sect to %u, exceeding valid media max of %u\n", + new_last_sect_val, drv->media_last_sect); + return false; + } + return true; +} + static void fdctrl_handle_restore(FDCtrl *fdctrl, int direction) { FDrive *cur_drv = get_cur_drv(fdctrl); @@ -1919,6 +1932,10 @@ static void fdctrl_handle_restore(FDCtrl *fdctrl, int direction) /* timers */ fdctrl->timer0 = fdctrl->fifo[7]; fdctrl->timer1 = fdctrl->fifo[8]; + if (!fd_validate_last_sect(cur_drv, fdctrl->fifo[9])) { + fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_EC, 0x00); + return; + } cur_drv->last_sect = fdctrl->fifo[9]; fdctrl->lock = fdctrl->fifo[10] >> 7; cur_drv->perpendicular = (fdctrl->fifo[10] >> 2) & 0xF; @@ -1983,13 +2000,12 @@ static void fdctrl_handle_format_track(FDCtrl *fdctrl, int direction) fdctrl->data_state &= ~FD_STATE_MULTI; cur_drv->bps = fdctrl->fifo[2] > 7 ? 16384 : 128 << fdctrl->fifo[2]; -#if 0 - cur_drv->last_sect = - cur_drv->flags & FDISK_DBL_SIDES ? fdctrl->fifo[3] : - fdctrl->fifo[3] / 2; -#else + + if (!fd_validate_last_sect(cur_drv, fdctrl->fifo[3])) { + fdctrl_stop_transfer(fdctrl, FD_SR0_ABNTERM, FD_SR1_EC, 0x00); + return; + } cur_drv->last_sect = fdctrl->fifo[3]; -#endif /* TODO: implement format using DMA expected by the Bochs BIOS * and Linux fdformat (read 3 bytes per sector via DMA and fill * the sector with the specified fill byte -- 2.47.3
Am 19.08.2026 um 11:22 hat Alex Bennée geschrieben:
Previously the guest controlled the value of last_sec and wasn't particularly careful to check it didn't exceed the media size. We can't re-use drv->last_sec as that changes as we do operations so we set media_last_sect when the geometry is probed.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3800 Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
If it's not too hard, I would appreciate a qtest case that reproduces the problem in the pre-patch state and shows that the behaviour afterwards is right, including after media change to a floppy of a different size. Kevin
We have outstanding guest exploitable bugs in the code which is fairly complex and needs some serious love to clean it up. As off-CPU cryptography acceleration seems to be waning in popularity lets just deprecate the sub-system with a view to removing it in a couple of cycles. Link: https://gitlab.com/qemu-project/qemu/-/work_items/3625 Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Cc: Gonglei <arei.gonglei@huawei.com> --- docs/about/deprecated.rst | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 0c656a968fc..3c22981fc8a 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -276,6 +276,20 @@ future release. Where no 'dh-params.pem' file is provided, the DH parameters will be automatically negotiated in accordance with RFC7919. +Devices +------- + +``virtio-crypto`` (since 11.2) +'''''''''''''''''''''''''''''' + +The ``virtio-crypto`` device emulation is quite complex code with a +number of known flaws. It has never been migratable so is unlikely to +be used in any serious virtualization setting. With most modern +systems supporting on-CPU cryptography acceleration via their +ISAs sets the need for off-CPU acceleration is also reduced. Add to +that the kernel has recently removed the ability to use off-CPU +acceleration suggests the time for this sort of off-load has passed. + Device options -------------- -- 2.47.3
On 19. Aug 2026, at 11:22, Alex Bennée <alex.bennee@linaro.org> wrote:
We have outstanding guest exploitable bugs in the code which is fairly complex and needs some serious love to clean it up. As off-CPU cryptography acceleration seems to be waning in popularity lets just deprecate the sub-system with a view to removing it in a couple of cycles.
Hi, Still quite powerful compared to on-core acceleration, the kernel isn’t a great example on how to use them well unfortunately. Far better experience in production with DPDK and directly passing through the crypto accelerator to the process that uses it directly. virtio-crypto with its overhead makes things even worse on that front so… In practice, the vmexits did cost so much that it was quicker to do it in software...
Link: https://gitlab.com/qemu-project/qemu/-/work_items/3625 Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Cc: Gonglei <arei.gonglei@huawei.com>
Unfortunate but it wasn’t useful as is, so, Reviewed-by: Mohamed Mediouni <mohamed@unpredictable.fr>
--- docs/about/deprecated.rst | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 0c656a968fc..3c22981fc8a 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -276,6 +276,20 @@ future release. Where no 'dh-params.pem' file is provided, the DH parameters will be automatically negotiated in accordance with RFC7919.
+Devices +------- + +``virtio-crypto`` (since 11.2) +'''''''''''''''''''''''''''''' + +The ``virtio-crypto`` device emulation is quite complex code with a +number of known flaws. It has never been migratable so is unlikely to +be used in any serious virtualization setting. With most modern +systems supporting on-CPU cryptography acceleration via their +ISAs sets the need for off-CPU acceleration is also reduced. Add to +that the kernel has recently removed the ability to use off-CPU +acceleration suggests the time for this sort of off-load has passed. + Device options --------------
-- 2.47.3
On Wed, Aug 19, 2026 at 10:22:06AM +0100, Alex Bennée wrote:
We have outstanding guest exploitable bugs in the code which is fairly complex and needs some serious love to clean it up. As off-CPU cryptography acceleration seems to be waning in popularity lets just deprecate the sub-system with a view to removing it in a couple of cycles.
Link: https://gitlab.com/qemu-project/qemu/-/work_items/3625 Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Cc: Gonglei <arei.gonglei@huawei.com>
Let's see what Gonglei thinks.
--- docs/about/deprecated.rst | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 0c656a968fc..3c22981fc8a 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -276,6 +276,20 @@ future release. Where no 'dh-params.pem' file is provided, the DH parameters will be automatically negotiated in accordance with RFC7919.
+Devices +------- + +``virtio-crypto`` (since 11.2) +'''''''''''''''''''''''''''''' + +The ``virtio-crypto`` device emulation is quite complex code with a +number of known flaws. It has never been migratable so is unlikely to +be used in any serious virtualization setting. With most modern +systems supporting on-CPU cryptography acceleration via their +ISAs sets
ISA sets?
the need for off-CPU acceleration is also reduced. Add to +that the kernel has recently removed the ability to use off-CPU +acceleration
did it really? I thought it's just for af_alg? and putting "recently" into cde will not age well.
suggests the time for this sort of off-load has passed. + Device options --------------
-- 2.47.3
On 19. Aug 2026, at 23:56, Michael S. Tsirkin <mst@redhat.com> wrote:
On Wed, Aug 19, 2026 at 10:22:06AM +0100, Alex Bennée wrote:
We have outstanding guest exploitable bugs in the code which is fairly complex and needs some serious love to clean it up. As off-CPU cryptography acceleration seems to be waning in popularity lets just deprecate the sub-system with a view to removing it in a couple of cycles.
Link: https://gitlab.com/qemu-project/qemu/-/work_items/3625 Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Cc: Gonglei <arei.gonglei@huawei.com>
Let's see what Gonglei thinks.
--- docs/about/deprecated.rst | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 0c656a968fc..3c22981fc8a 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -276,6 +276,20 @@ future release. Where no 'dh-params.pem' file is provided, the DH parameters will be automatically negotiated in accordance with RFC7919.
+Devices +------- + +``virtio-crypto`` (since 11.2) +'''''''''''''''''''''''''''''' + +The ``virtio-crypto`` device emulation is quite complex code with a +number of known flaws. It has never been migratable so is unlikely to +be used in any serious virtualization setting. With most modern +systems supporting on-CPU cryptography acceleration via their +ISAs sets
ISA sets?
Architecturally exposed instead of via PCIe/platform devices
the need for off-CPU acceleration is also reduced. Add to +that the kernel has recently removed the ability to use off-CPU +acceleration
did it really? I thought it's just for af_alg?
The kernel’s own use cases - for disk encryption and co - aren’t supported anymore using offloading - and AF_ALG is deprecated. Drivers exposing a channel directly between hardware and the driver (ConnectX NICs, Intel QAT for ex) remain, and userspace can continue to use hardware crypto offloads through these or kTLS offload which also remains. For the former case though this means using a vendor-specific library in userspace (qatlib for intel)
and putting "recently" into cde will not age well.
suggests the time for this sort of off-load has passed. + Device options --------------
-- 2.47.3
-----Original Message----- From: Michael S. Tsirkin <mst@redhat.com> Sent: Thursday, August 20, 2026 5:56 AM To: Alex Bennée <alex.bennee@linaro.org> Cc: qemu-devel@nongnu.org; Philippe Mathieu-Daudé <philmd@mailo.com>; Pierrick Bouvier <pierrick.bouvier@oss.qualcomm.com>; Peter Maydell <peter.maydell@linaro.org>; qemu-arm@nongnu.org; Gonglei (Arei) <arei.gonglei@huawei.com>; zhenwei pi <zhenwei.pi@linux.dev>; Stefano Garzarella <sgarzare@redhat.com>; Laurent Vivier <lvivier@redhat.com>; Amit Shah <amit@kernel.org>; Marc-André Lureau <marcandre.lureau@redhat.com>; Paolo Bonzini <pbonzini@redhat.com>; John Snow <jsnow@redhat.com>; Kevin Wolf <kwolf@redhat.com>; Hanna Reitz <hreitz@redhat.com>; qemu-block@nongnu.org; devel@lists.libvirt.org Subject: Re: [PATCH 6/6] docs: deprecate virtio-crypto
On Wed, Aug 19, 2026 at 10:22:06AM +0100, Alex Bennée wrote:
We have outstanding guest exploitable bugs in the code which is fairly complex and needs some serious love to clean it up. As off-CPU cryptography acceleration seems to be waning in popularity lets just deprecate the sub-system with a view to removing it in a couple of cycles.
Link: https://gitlab.com/qemu-project/qemu/-/work_items/3625 Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Cc: Gonglei <arei.gonglei@huawei.com>
Let's see what Gonglei thinks.
Hi Alex, Michael, Thanks for the patch. As the virtio-crypto maintainer let me first respond to Michael's three points, then give my formal position on the deprecation - including a scope clarification that I think the community should be aware of. I ACK deprecating virtio-crypto since 11.2, but I want to be explicit that this is effectively a *subsystem-level* deprecation, not just a single device: - In QEMU today, virtio-crypto is the *only* frontend device that consumes the cryptodev backends (cryptodev-builtin, cryptodev-vhost-user, cryptodev-lkcf). There is no standalone vhost-user-crypto device - the vhost-user crypto path is driven through virtio-crypto as well (virtio_crypto_vhost_status -> cryptodev_vhost_start). So removing virtio-crypto orphans the entire backend subsystem, backends/cryptodev-vhost.c, and the vhost-user crypto session protocol support in hw/virtio/vhost-user.c. - I'd therefore like the deprecation to cover that scope explicitly and to plan the cleanup of the backends together with the device, so we don't leave dead, unmaintained objects behind. This includes deciding the fate of the recently added cryptodev-lkcf backend. The basis and commitments behind my ACK: - The emulation has had a string of guest-exploitable issues (CVE-2023-3180, CVE-2024-3446, CVE-2024-8612, CVE-2025-14876) and has been unmigratable since day one (.unmigratable = 1), which indeed undercuts its use in serious virtualization. Maintainer bandwidth is limited. - I will keep landing fixes for the known guest-exploitable bugs during the deprecation window (several are already in flight, e.g. the asym/akcipher size hardening addressing the CVE-2025-14876 class of issues), so the code stays safe while it is being removed. Realistic migration path for any existing users (there is no virtio replacement, so it's honest to say so): - in-guest CPU crypto (AES-NI/VAES etc.) for the common case, and - host-side direct assignment of crypto accelerators via VFIO (e.g. Intel QAT, ConnectX) for the workloads that genuinely need offload, which is also the better-performing path (avoids vmexit overhead, as Mohamed noted). Finally, please let's use the deprecation window to surface any real users before scheduling the actual removal. Proposed replacement text for docs/about/deprecated.rst: The `virtio-crypto` device emulation is quite complex code with a number of known flaws. It has never been migratable, so it is unlikely to be used in any serious virtualization setting. Modern ISAs provide on-CPU cryptography instructions (e.g. AES-NI/VAES, armv8 crypto extensions), and the Linux kernel deprecated AF_ALG and dropped its off-CPU accelerator support in Linux 7.2, removing the primary userspace path for off-CPU crypto acceleration. The time for this sort of off-load has passed.
--- docs/about/deprecated.rst | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 0c656a968fc..3c22981fc8a 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -276,6 +276,20 @@ future release. Where no 'dh-params.pem' file is provided, the DH parameters will be automatically negotiated in accordance with RFC7919.
+Devices +------- + +``virtio-crypto`` (since 11.2) +'''''''''''''''''''''''''''''' + +The ``virtio-crypto`` device emulation is quite complex code with a +number of known flaws. It has never been migratable so is unlikely to +be used in any serious virtualization setting. With most modern +systems supporting on-CPU cryptography acceleration via their ISAs +sets
ISA sets?
Fair catch, that phrasing was sloppy. The point is that modern ISAs expose on-CPU cryptography instructions (x86 AES-NI/VAES, armv8 crypto extensions, RISC-V scalar crypto, s390x KIMD/KM, ...). For the common case there is no longer a performance win in bouncing the request to an off-CPU engine.
the need for off-CPU acceleration is also reduced. Add to +that the kernel has recently removed the ability to use off-CPU +acceleration
did it really? I thought it's just for af_alg?
You are right. To be precise: the virtio-crypto *driver* (CONFIG_CRYPTO_DEV_VIRTIO) is still present in the Linux kernel and is still being maintained (it received fixes for CVE-2024-26753 and, most recently, CVE-2026-23229). What the kernel actually deprecated is AF_ALG, whose off-CPU accelerator support was dropped for Linux 7.2 (Eric Biggers), and AF_ALG is the primary userspace path a guest would use for virtio-crypto (e.g. OpenSSL's afalg engine). The docs should say exactly that, not "the kernel removed the ability to use off-CPU acceleration" in general.
and putting "recently" into cde will not age well.
Agreed. "recently" is time-dependent and has no place in deprecated.rst. I'd reference the concrete kernel change instead (AF_ALG deprecation / offload removal in Linux 7.2), which is version-stable. Regards, -Gonglei
On 19/08/2026 11.22, Alex Bennée wrote:
We have outstanding guest exploitable bugs in the code which is fairly complex and needs some serious love to clean it up. As off-CPU cryptography acceleration seems to be waning in popularity lets just deprecate the sub-system with a view to removing it in a couple of cycles.
Link: https://gitlab.com/qemu-project/qemu/-/work_items/3625 Signed-off-by: Alex Bennée <alex.bennee@linaro.org> Cc: Gonglei <arei.gonglei@huawei.com> --- docs/about/deprecated.rst | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 0c656a968fc..3c22981fc8a 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -276,6 +276,20 @@ future release. Where no 'dh-params.pem' file is provided, the DH parameters will be automatically negotiated in accordance with RFC7919.
+Devices +------- + +``virtio-crypto`` (since 11.2) +'''''''''''''''''''''''''''''' + +The ``virtio-crypto`` device emulation is quite complex code with a +number of known flaws. It has never been migratable so is unlikely to +be used in any serious virtualization setting. With most modern +systems supporting on-CPU cryptography acceleration via their +ISAs sets the need for off-CPU acceleration is also reduced. Add to +that the kernel has recently removed the ability to use off-CPU +acceleration suggests the time for this sort of off-load has passed. Reviewed-by: Thomas Huth <thuth@redhat.com>
participants (6)
-
Alex Bennée -
Gonglei (Arei) -
Kevin Wolf -
Michael S. Tsirkin -
Mohamed Mediouni -
Thomas Huth