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

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

Add a test case for #3800 where we check that the last sector is properly bounded by the physical media that is inserted. Link: https://gitlab.com/qemu-project/qemu/-/issues/3800 AI-used-for: initial draft of test Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- tests/qtest/fdc-test.c | 246 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 241 insertions(+), 5 deletions(-) diff --git a/tests/qtest/fdc-test.c b/tests/qtest/fdc-test.c index 1e1dd8659d0..3f2642fe671 100644 --- a/tests/qtest/fdc-test.c +++ b/tests/qtest/fdc-test.c @@ -31,7 +31,8 @@ #define DRIVE_FLOPPY_BLANK \ "-drive if=floppy,file=null-co://,file.read-zeroes=on,format=raw,size=1440k" -#define TEST_IMAGE_SIZE 1440 * 1024 +#define TEST_IMAGE_1440KB (1440 * 1024) +#define TEST_IMAGE_720KB (720 * 1024) #define FLOPPY_BASE 0x3f0 #define FLOPPY_IRQ 6 @@ -49,8 +50,11 @@ enum { enum { CMD_SENSE_INT = 0x08, CMD_READ_ID = 0x0a, + CMD_FORMAT_TRACK = 0x4d, CMD_SEEK = 0x0f, CMD_VERIFY = 0x16, + CMD_SAVE = 0x2e, + CMD_RESTORE = 0x4e, CMD_READ = 0xe6, CMD_RELATIVE_SEEK_OUT = 0x8f, CMD_RELATIVE_SEEK_IN = 0xcf, @@ -69,9 +73,11 @@ enum { ST0_IC_ABNTERM = 0x40, /* abnormal termination */ ST1_MA = 0x01, /* missing address mark */ + ST1_EC = 0x80, /* end of cylinder / sector past last_sect */ }; static char *test_image; +static char *test_image_720k; #define assert_bit_set(data, mask) g_assert_cmphex((data) & (mask), ==, (mask)) #define assert_bit_clear(data, mask) g_assert_cmphex((data) & (mask), ==, 0) @@ -276,12 +282,17 @@ static void test_cmos(void) g_assert(cmos == 0x40 || cmos == 0x50); } -static void media_insert(void) +static void media_insert_path(const char *path) { qtest_qmp_assert_success(global_qtest, "{'execute':'blockdev-change-medium', 'arguments':{" " 'id':'floppy0', 'filename': %s, 'format': 'raw' }}", - test_image); + path); +} + +static void media_insert(void) +{ + media_insert_path(test_image); } static void media_eject(void) @@ -586,6 +597,222 @@ static void test_verify(void) g_assert(ret == 0); } +/* + * Query cur_drv->last_sect using the SAVE command (CMD_SAVE, 0x2e). + * Byte 8 of the 15 result bytes returned by CMD_SAVE holds last_sect. + */ +static uint8_t get_lastsect(void) +{ + uint8_t res[15]; + int i; + + floppy_send(CMD_SAVE); + for (i = 0; i < 15; i++) { + res[i] = floppy_recv(); + } + return res[8]; +} + +/* + * Attempt to set cur_drv->last_sect directly using the RESTORE command + * (CMD_RESTORE, 0x4e). + * While the 82078 datasheet describes RESTORE for restoring a previously + * saved state, a guest can issue raw RESTORE commands with arbitrary + * parameters without having issued SAVE. Parameter byte 9 is used by the + * controller to restore cur_drv->last_sect. + */ +static void fake_lastsect(uint8_t last_sect) +{ + floppy_send(CMD_RESTORE); + floppy_send(0); /* fifo[1] */ + floppy_send(0); /* fifo[2] */ + floppy_send(0); /* fifo[3]: drv0 track */ + floppy_send(0); /* fifo[4]: drv1 track */ + floppy_send(0); /* fifo[5]: drv2 track */ + floppy_send(0); /* fifo[6]: drv3 track */ + floppy_send(0); /* fifo[7]: timer0 */ + floppy_send(0); /* fifo[8]: timer1 */ + floppy_send(last_sect); /* fifo[9]: last_sect */ + floppy_send(0); /* fifo[10]: lock/perpendicular */ + floppy_send(0); /* fifo[11]: config */ + floppy_send(0); /* fifo[12]: precomp_trk */ + floppy_send(0); /* fifo[13]: pwrd */ + floppy_send(0); /* fifo[14] */ + floppy_send(0); /* fifo[15] */ + floppy_send(0); /* fifo[16] */ + floppy_send(0); /* fifo[17] */ +} + +static void send_format_track(uint8_t drive, uint8_t head, uint8_t last_sect, + uint8_t *st0_out, uint8_t *st1_out) +{ + uint8_t st0, st1; + + floppy_send(CMD_FORMAT_TRACK); + floppy_send((head << 2) | drive); + floppy_send(2); /* 512 bytes per sector */ + floppy_send(last_sect); /* sectors per track */ + floppy_send(0x1b); /* GAP length */ + floppy_send(0x00); /* filler byte */ + + g_assert(get_irq(FLOPPY_IRQ)); + st0 = floppy_recv(); + st1 = floppy_recv(); + floppy_recv(); /* st2 */ + floppy_recv(); /* track */ + floppy_recv(); /* head */ + floppy_recv(); /* sect */ + g_assert(get_irq(FLOPPY_IRQ)); + floppy_recv(); /* sz */ + g_assert(!get_irq(FLOPPY_IRQ)); + + if (st0_out) { + *st0_out = st0; + } + if (st1_out) { + *st1_out = st1; + } +} + +/* + * Test that guest cannot set last_sect beyond the probed media size + * via RESTORE or FORMAT TRACK commands (gitlab issue #3800). + */ +static void test_last_sect_bounds(void) +{ + uint8_t st0, st1; + + /* Start with 1.44 MB media inserted (last_sect = 18) */ + media_insert(); + send_seek(1); + send_seek(0); + + /* Valid last_sect values (<= 18) should succeed */ + fake_lastsect(18); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(get_lastsect(), ==, 18); + + fake_lastsect(9); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(get_lastsect(), ==, 9); + + /* Restoring to the default 18 */ + fake_lastsect(18); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(get_lastsect(), ==, 18); + + /* Invalid last_sect value (> 18) must fail */ + fake_lastsect(19); + g_assert(get_irq(FLOPPY_IRQ)); + st0 = floppy_recv(); + st1 = floppy_recv(); + floppy_recv(); /* st2 */ + floppy_recv(); /* track */ + floppy_recv(); /* head */ + floppy_recv(); /* sect */ + g_assert(get_irq(FLOPPY_IRQ)); + floppy_recv(); /* sz */ + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_EC, ==, ST1_EC); + + /* Verify last_sect was not changed to 19 */ + g_assert_cmpint(get_lastsect(), ==, 18); + + /* FORMAT TRACK with valid last_sect (18) should succeed */ + send_format_track(0, 0, 18, &st0, &st1); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, 0); + g_assert_cmpint(st1, ==, 0); + + /* FORMAT TRACK with invalid last_sect (19) must fail */ + send_format_track(0, 0, 19, &st0, &st1); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_EC, ==, ST1_EC); + + /* Change media to 720 kB floppy (last_sect = 9) */ + media_eject(); + media_insert_path(test_image_720k); + send_seek(1); + send_seek(0); + + /* Probed geometry now has last_sect = 9 */ + fake_lastsect(9); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(get_lastsect(), ==, 9); + + /* Values exceeding 9 (e.g. 10 or 18) must now fail */ + fake_lastsect(10); + g_assert(get_irq(FLOPPY_IRQ)); + st0 = floppy_recv(); + st1 = floppy_recv(); + floppy_recv(); + floppy_recv(); + floppy_recv(); + floppy_recv(); + g_assert(get_irq(FLOPPY_IRQ)); + floppy_recv(); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_EC, ==, ST1_EC); + + fake_lastsect(18); + g_assert(get_irq(FLOPPY_IRQ)); + st0 = floppy_recv(); + st1 = floppy_recv(); + floppy_recv(); + floppy_recv(); + floppy_recv(); + floppy_recv(); + g_assert(get_irq(FLOPPY_IRQ)); + floppy_recv(); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_EC, ==, ST1_EC); + + /* FORMAT TRACK on 720 kB floppy */ + send_format_track(0, 0, 9, &st0, &st1); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, 0); + g_assert_cmpint(st1, ==, 0); + + send_format_track(0, 0, 10, &st0, &st1); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_EC, ==, ST1_EC); + + send_format_track(0, 0, 18, &st0, &st1); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_EC, ==, ST1_EC); + + /* + * Change back to 1.44 MB floppy and verify last_sect = 18 is allowed + * again. + */ + media_eject(); + media_insert(); + send_seek(1); + send_seek(0); + + fake_lastsect(18); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(get_lastsect(), ==, 18); + + fake_lastsect(19); + g_assert(get_irq(FLOPPY_IRQ)); + st0 = floppy_recv(); + st1 = floppy_recv(); + floppy_recv(); + floppy_recv(); + floppy_recv(); + floppy_recv(); + g_assert(get_irq(FLOPPY_IRQ)); + floppy_recv(); + g_assert(!get_irq(FLOPPY_IRQ)); + g_assert_cmpint(st0 & ST0_IC_MASK, ==, ST0_IC_ABNTERM); + g_assert_cmpint(st1 & ST1_EC, ==, ST1_EC); + + /* Leave drive empty */ + media_eject(); +} + /* success if no crash or abort */ static void fuzz_registers(void) { @@ -661,10 +888,16 @@ int main(int argc, char **argv) int fd; int ret; - /* Create a temporary raw image */ + /* Create temporary raw images */ fd = g_file_open_tmp("qtest.XXXXXX", &test_image, NULL); g_assert(fd >= 0); - ret = ftruncate(fd, TEST_IMAGE_SIZE); + ret = ftruncate(fd, TEST_IMAGE_1440KB); + g_assert(ret == 0); + close(fd); + + fd = g_file_open_tmp("qtest720.XXXXXX", &test_image_720k, NULL); + g_assert(fd >= 0); + ret = ftruncate(fd, TEST_IMAGE_720KB); g_assert(ret == 0); close(fd); @@ -686,6 +919,7 @@ int main(int argc, char **argv) qtest_add_func("/fdc/read_no_dma_1", test_read_no_dma_1); qtest_add_func("/fdc/read_no_dma_18", test_read_no_dma_18); qtest_add_func("/fdc/read_no_dma_19", test_read_no_dma_19); + qtest_add_func("/fdc/last_sect_bounds", test_last_sect_bounds); qtest_add_func("/fdc/fuzz-registers", fuzz_registers); qtest_add_func("/fdc/fuzz/cve_2021_20196", test_cve_2021_20196); qtest_add_func("/fdc/fuzz/cve_2021_3507", test_cve_2021_3507); @@ -696,6 +930,8 @@ int main(int argc, char **argv) qtest_end(); unlink(test_image); g_free(test_image); + unlink(test_image_720k); + g_free(test_image_720k); return ret; } -- 2.47.3

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 the deprecate the sub-system with a view to removing it in a couple of cycles. We can also remove the backend code at that point as well. Link: https://gitlab.com/qemu-project/qemu/-/work_items/3625 Reviewed-by: Mohamed Mediouni <mohamed@unpredictable.fr> Reviewed-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Alex Bennée <alex.bennee@linaro.org> --- v2 - mention the backend bits (only used by virtio-crypto) - use Gonglei's modified version of the text --- docs/about/deprecated.rst | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 0c656a968fc..05e4ce8cf16 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -276,6 +276,24 @@ future release. Where no 'dh-params.pem' file is provided, the DH parameters will be automatically negotiated in accordance with RFC7919. +Devices +------- + +``virtio-crypto`` and cryptodev backends (since 11.2) +''''''''''''''''''''''''''''''''''''''''''''''''''''' + +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. + +As this is the only device that uses the cryptodev backends these will +be removed at the same time the ``virtio-crypto`` device is. + Device options -------------- -- 2.47.3