[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/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 (5)
-
Alex Bennée -
Kevin Wolf -
Michael S. Tsirkin -
Mohamed Mediouni -
Thomas Huth