The following changes since commit dfaecc04c46d298e9ee81bd0ca96d8754f1c27ed:

Merge tag 'pull-riscv-to-apply-20250407-1' of https://github.com/alistair23/qemu into staging (2025-04-07 09:18:33 -0400)

are available in the Git repository at:

https://repo.or.cz/qemu/kevin.git tags/for-upstream

for you to fetch changes up to f8222bfba3409a3ce09c191941127a8cf2c7e623:

test-bdrv-drain: Fix data races (2025-04-08 15:00:01 +0200)

Denis Rastyogin (1):

qemu-img: fix division by zero in bench_cb() for zero-sized images

Kevin Wolf (2):

qcow2: Don't crash qemu-img info with missing crypto header

scsi-disk: Apply error policy for host_status errors again

Vitalii Mordan (1):

test-bdrv-drain: Fix data races

include/qemu/job.h | 3 ++

block/qcow2.c | 4 +-

hw/scsi/scsi-disk.c | 39 +++++++++-----

job.c | 6 +++

qemu-img.c | 6 ++-

tests/unit/test-bdrv-drain.c | 32 +++++++-----

tests/qemu-iotests/tests/qcow2-encryption | 75 +++++++++++++++++++++++++++

tests/qemu-iotests/tests/qcow2-encryption.out | 32 ++++++++++++

8 files changed, 167 insertions(+), 30 deletions(-)

create mode 100755 tests/qemu-iotests/tests/qcow2-encryption

create mode 100644 tests/qemu-iotests/tests/qcow2-encryption.out

From: Denis Rastyogin <gerben@altlinux.org>

This error was discovered by fuzzing qemu-img.

This commit fixes a division by zero error in the bench_cb() function

that occurs when using the bench command with a zero-sized image.

The issue arises because b->image_size can be zero, leading to a

division by zero in the modulo operation (b->offset %= b->image_size).

This patch adds a check for b->image_size == 0 and resets b->offset

to 0 in such cases, preventing the error.

Signed-off-by: Denis Rastyogin <gerben@altlinux.org>

Message-ID: <20250318101933.255617-1-gerben@altlinux.org>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

qemu-img.c | 6 +++++-

1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/qemu-img.c b/qemu-img.c

--- a/qemu-img.c

+++ b/qemu-img.c

@@ -XXX,XX +XXX,XX @@ static void bench_cb(void *opaque, int ret)

*/

b->in_flight++;

b->offset += b->step;

- b->offset %= b->image_size;

+ if (b->image_size == 0) {

+ b->offset = 0;

+ } else {

+ b->offset %= b->image_size;

+ }

if (b->write) {

acb = blk_aio_pwritev(b->blk, offset, b->qiov, 0, bench_cb, b);

} else {

2.49.0

block/qcow2.c | 4 +-

tests/qemu-iotests/tests/qcow2-encryption | 75 +++++++++++++++++++

tests/qemu-iotests/tests/qcow2-encryption.out | 32 ++++++++

3 files changed, 109 insertions(+), 2 deletions(-)

create mode 100755 tests/qemu-iotests/tests/qcow2-encryption

create mode 100644 tests/qemu-iotests/tests/qcow2-encryption.out

diff --git a/block/qcow2.c b/block/qcow2.c

index XXXXXXX..XXXXXXX 100644

--- a/block/qcow2.c

+++ b/block/qcow2.c

@@ -XXX,XX +XXX,XX @@ qcow2_do_open(BlockDriverState *bs, QDict *options, int flags,

ret = -EINVAL;

goto fail;

}

- } else if (!(flags & BDRV_O_NO_IO)) {

+ } else {

error_setg(errp, "Missing CRYPTO header for crypt method %d",

s->crypt_method_header);

ret = -EINVAL;

@@ -XXX,XX +XXX,XX @@ static void qcow2_refresh_limits(BlockDriverState *bs, Error **errp)

{

BDRVQcow2State *s = bs->opaque;

- if (bs->encrypted) {

+ if (s->crypto) {

/* Encryption works on a sector granularity */

bs->bl.request_alignment = qcrypto_block_get_sector_size(s->crypto);

}

diff --git a/tests/qemu-iotests/tests/qcow2-encryption b/tests/qemu-iotests/tests/qcow2-encryption

+++ b/tests/qemu-iotests/tests/qcow2-encryption

+#!/usr/bin/env bash

+# group: rw quick

+# Test case for encryption support in qcow2

+# Copyright (C) 2025 Red Hat, Inc.

+seq="$(basename $0)"

+. ../common.rc

+. ../common.filter

+# This tests qcow2-specific low-level functionality

+_supported_fmt qcow2

+_require_working_luks

+IMG_SIZE=64M

+echo "=== Create an encrypted image ==="

+_make_test_img --object secret,id=sec0,data=123456 -o encrypt.format=luks,encrypt.key-secret=sec0 $IMG_SIZE

+$PYTHON ../qcow2.py "$TEST_IMG" dump-header-exts

+_img_info

+$QEMU_IMG check \

+ --object secret,id=sec0,data=123456 \

+ --image-opts file.filename="$TEST_IMG",encrypt.key-secret=sec0 \

+ | _filter_qemu_img_check

+echo "=== Remove the header extension ==="

+$PYTHON ../qcow2.py "$TEST_IMG" del-header-ext 0x0537be77

+$PYTHON ../qcow2.py "$TEST_IMG" dump-header-exts

+_img_info

+$QEMU_IMG check \

+ --object secret,id=sec0,data=123456 \

+ --image-opts file.filename="$TEST_IMG",encrypt.key-secret=sec0 2>&1 \

+ | _filter_qemu_img_check \

+ | _filter_testdir

diff --git a/tests/qemu-iotests/tests/qcow2-encryption.out b/tests/qemu-iotests/tests/qcow2-encryption.out

+++ b/tests/qemu-iotests/tests/qcow2-encryption.out

+QA output created by qcow2-encryption

+=== Create an encrypted image ===

+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864

+Header extension:

+magic 0x537be77 (Crypto header)

+length 16

+data <binary>

+

+Header extension:

+magic 0x6803f857 (Feature table)

+length 384

+data <binary>

+virtual size: 64 MiB (67108864 bytes)

+encrypted: yes

+cluster_size: 65536

+No errors were found on the image.

+=== Remove the header extension ===

+Header extension:

+magic 0x6803f857 (Feature table)

+length 384

+data <binary>

+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Missing CRYPTO header for crypt method 2

+qemu-img: Could not open 'file.filename=TEST_DIR/t.qcow2,encrypt.key-secret=sec0': Missing CRYPTO header for crypt method 2

2.49.0

Originally, all failed SG_IO requests called scsi_handle_rw_error() to

apply the configured error policy. However, commit f3126d65, which was

supposed to be a mere refactoring for scsi-disk.c, broke this and

accidentally completed the SCSI request without considering the error

policy any more if the error was signalled in the host_status field.

Apart from the commit message not describing the change as intended,

errors indicated in host_status are also obviously backend errors and

not something the guest must deal with independently of the error

policy.

This behaviour means that some recoverable errors (such as a path error

in multipath configurations) were reported to the guest anyway, which

might not expect it and might consider its disk broken.

Make sure that we apply the error policy again for host_status errors,

too. This addresses an existing FIXME comment and allows us to remove

some comments warning that callbacks weren't always called. With this

fix, they are called in all cases again.

The return value passed to the request callback doesn't have more free

values that could be used to indicate host_status errors as well as SAM

status codes and negative errno. Store the value in the host_status

field of the SCSIRequest instead and use -ENODEV as the return value (if

a path hasn't been reachable for a while, blk_aio_ioctl() will return

-ENODEV instead of just setting host_status, so just reuse it here -

it's not necessarily entirely accurate, but it's as good as any errno).

Cc: qemu-stable@nongnu.org

Fixes: f3126d65b393 ('scsi: move host_status handling into SCSI drivers')

Signed-off-by: Kevin Wolf <kwolf@redhat.com>

Message-ID: <20250407155949.44736-1-kwolf@redhat.com>

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>

Reviewed-by: Hanna Czenczek <hreitz@redhat.com>

hw/scsi/scsi-disk.c | 39 +++++++++++++++++++++++++--------------

1 file changed, 25 insertions(+), 14 deletions(-)

diff --git a/hw/scsi/scsi-disk.c b/hw/scsi/scsi-disk.c

--- a/hw/scsi/scsi-disk.c

+++ b/hw/scsi/scsi-disk.c

@@ -XXX,XX +XXX,XX @@ struct SCSIDiskClass {

SCSIDeviceClass parent_class;

/*

* Callbacks receive ret == 0 for success. Errors are represented either as

- * negative errno values, or as positive SAM status codes.

- *

- * Beware: For errors returned in host_status, the function may directly

- * complete the request and never call the callback.

+ * negative errno values, or as positive SAM status codes. For host_status

+ * errors, the function passes ret == -ENODEV and sets the host_status field

+ * of the SCSIRequest.

*/

DMAIOFunc *dma_readv;

DMAIOFunc *dma_writev;

@@ -XXX,XX +XXX,XX @@ static bool scsi_handle_rw_error(SCSIDiskReq *r, int ret, bool acct_failed)

SCSIDiskState *s = DO_UPCAST(SCSIDiskState, qdev, r->req.dev);

SCSIDiskClass *sdc = (SCSIDiskClass *) object_get_class(OBJECT(s));

SCSISense sense = SENSE_CODE(NO_SENSE);

+ int16_t host_status;

int error;

bool req_has_sense = false;

BlockErrorAction action;

int status;

+ /*

+ * host_status should only be set for SG_IO requests that came back with a

+ * host_status error in scsi_block_sgio_complete(). This error path passes

+ * -ENODEV as the return value.

+ *

+ * Reset host_status in the request because we may still want to complete

+ * the request successfully with the 'stop' or 'ignore' error policy.

+ */

+ host_status = r->req.host_status;

+ if (host_status != -1) {

+ assert(ret == -ENODEV);

+ r->req.host_status = -1;

+ }

+

if (ret < 0) {

status = scsi_sense_from_errno(-ret, &sense);

error = -ret;

@@ -XXX,XX +XXX,XX @@ static bool scsi_handle_rw_error(SCSIDiskReq *r, int ret, bool acct_failed)

if (acct_failed) {

block_acct_failed(blk_get_stats(s->qdev.conf.blk), &r->acct);

}

+ if (host_status != -1) {

+ scsi_req_complete_failed(&r->req, host_status);

+ return true;

+ }

if (req_has_sense) {

sdc->update_sense(&r->req);

} else if (status == CHECK_CONDITION) {

@@ -XXX,XX +XXX,XX @@ done:

scsi_req_unref(&r->req);

}

-/* May not be called in all error cases, don't rely on cleanup here */

static void scsi_dma_complete(void *opaque, int ret)

{

SCSIDiskReq *r = (SCSIDiskReq *)opaque;

@@ -XXX,XX +XXX,XX @@ done:

scsi_req_unref(&r->req);

}

-/* May not be called in all error cases, don't rely on cleanup here */

static void scsi_read_complete(void *opaque, int ret)

{

SCSIDiskReq *r = (SCSIDiskReq *)opaque;

@@ -XXX,XX +XXX,XX @@ done:

scsi_req_unref(&r->req);

}

-/* May not be called in all error cases, don't rely on cleanup here */

static void scsi_write_complete(void * opaque, int ret)

{

SCSIDiskReq *r = (SCSIDiskReq *)opaque;

@@ -XXX,XX +XXX,XX @@ static void scsi_block_sgio_complete(void *opaque, int ret)

sg_io_hdr_t *io_hdr = &req->io_header;

if (ret == 0) {

- /* FIXME This skips calling req->cb() and any cleanup in it */

if (io_hdr->host_status != SCSI_HOST_OK) {

- scsi_req_complete_failed(&r->req, io_hdr->host_status);

- scsi_req_unref(&r->req);

- return;

- }

-

- if (io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT) {

+ r->req.host_status = io_hdr->host_status;

+ ret = -ENODEV;

+ } else if (io_hdr->driver_status & SG_ERR_DRIVER_TIMEOUT) {

ret = BUSY;

} else {

ret = io_hdr->status;

2.49.0