From patchwork Tue Apr 8 13:00:45 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 14043001 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id DD1C7C3600C for ; Tue, 8 Apr 2025 13:02:35 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1u28ab-0006r8-OK; Tue, 08 Apr 2025 09:02:15 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1u28ZZ-0006lT-NS for qemu-devel@nongnu.org; Tue, 08 Apr 2025 09:01:10 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1u28ZX-0005bg-O6 for qemu-devel@nongnu.org; Tue, 08 Apr 2025 09:01:01 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1744117258; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=3rX++C6y7c2kTkmHdJ5btTzWRmxB6hJRYTB6HAm2FhE=; b=ZCTkAFAuCLcm8kqSW1Wzql55ob9BhPA8j0eOnr3OTDFdXPSw9whT2aBLB4ZDSeTaen38HF iInvVPAGRqbBfcMhV8ALAm+goJqzWZpsh6uuN6q4jHRyi0nlbQnxy6sQPchS2088A5ljnI tqCzkFWOnhWCf+3O8xuKg0kdRHFFxKI= Received: from mx-prod-mc-01.mail-002.prod.us-west-2.aws.redhat.com (ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-333-u_drAbzINzOnWPJIaF9-fA-1; Tue, 08 Apr 2025 09:00:56 -0400 X-MC-Unique: u_drAbzINzOnWPJIaF9-fA-1 X-Mimecast-MFC-AGG-ID: u_drAbzINzOnWPJIaF9-fA_1744117255 Received: from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.40]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 9E3141956053; Tue, 8 Apr 2025 13:00:55 +0000 (UTC) Received: from merkur.redhat.com (unknown [10.44.33.56]) by mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 6AA211955BF1; Tue, 8 Apr 2025 13:00:52 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Cc: kwolf@redhat.com, qemu-devel@nongnu.org Subject: [PULL 1/4] qemu-img: fix division by zero in bench_cb() for zero-sized images Date: Tue, 8 Apr 2025 15:00:45 +0200 Message-ID: <20250408130048.283364-2-kwolf@redhat.com> In-Reply-To: <20250408130048.283364-1-kwolf@redhat.com> References: <20250408130048.283364-1-kwolf@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.40 Received-SPF: pass client-ip=170.10.129.124; envelope-from=kwolf@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -28 X-Spam_score: -2.9 X-Spam_bar: -- X-Spam_report: (-2.9 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.845, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_VALIDITY_CERTIFIED_BLOCKED=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org From: Denis Rastyogin 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 Message-ID: <20250318101933.255617-1-gerben@altlinux.org> Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- qemu-img.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/qemu-img.c b/qemu-img.c index 89c93c1eb5..2044c22a4c 100644 --- a/qemu-img.c +++ b/qemu-img.c @@ -4488,7 +4488,11 @@ 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 { From patchwork Tue Apr 8 13:00:46 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 14043000 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 9C435C3600C for ; Tue, 8 Apr 2025 13:02:31 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1u28a4-0006qe-4n; Tue, 08 Apr 2025 09:01:32 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1u28Zd-0006lm-Gv for qemu-devel@nongnu.org; Tue, 08 Apr 2025 09:01:10 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1u28Za-0005bs-5T for qemu-devel@nongnu.org; Tue, 08 Apr 2025 09:01:04 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1744117260; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=dzyk8p1cAmH+C/2a12zOzfh/vQzAs48dsHc/xpXbFws=; b=FoFEPuMy3RWQeIsNVb9//tgL8Bx/xic5CV6E4DWynIIYulJucF3ndgNEwCIkww/SyzuKu3 W8LZ3KNVt93IgWaLOJfEbW8tg3sItBJtgsIYpiVYOP2Ta1qNXv2xb7Nso/zE1PrafP1oGo 2gNuKFz8GKL+x2uEsJUyKZv4vCylYNQ= Received: from mx-prod-mc-06.mail-002.prod.us-west-2.aws.redhat.com (ec2-35-165-154-97.us-west-2.compute.amazonaws.com [35.165.154.97]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-597-1XBBp1BTMlKjaJTm3yhqUQ-1; Tue, 08 Apr 2025 09:00:58 -0400 X-MC-Unique: 1XBBp1BTMlKjaJTm3yhqUQ-1 X-Mimecast-MFC-AGG-ID: 1XBBp1BTMlKjaJTm3yhqUQ_1744117257 Received: from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.40]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-06.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 75C4E18001FA; Tue, 8 Apr 2025 13:00:57 +0000 (UTC) Received: from merkur.redhat.com (unknown [10.44.33.56]) by mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 37B0E1955BF1; Tue, 8 Apr 2025 13:00:55 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Cc: kwolf@redhat.com, qemu-devel@nongnu.org Subject: [PULL 2/4] qcow2: Don't crash qemu-img info with missing crypto header Date: Tue, 8 Apr 2025 15:00:46 +0200 Message-ID: <20250408130048.283364-3-kwolf@redhat.com> In-Reply-To: <20250408130048.283364-1-kwolf@redhat.com> References: <20250408130048.283364-1-kwolf@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.40 Received-SPF: pass client-ip=170.10.129.124; envelope-from=kwolf@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -28 X-Spam_score: -2.9 X-Spam_bar: -- X-Spam_report: (-2.9 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.845, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_VALIDITY_CERTIFIED_BLOCKED=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org qcow2_refresh_limits() assumes that s->crypto is non-NULL whenever bs->encrypted is true. This is actually not the case: qcow2_do_open() allows to open an image with a missing crypto header for BDRV_O_NO_IO, and then bs->encrypted is true, but s->crypto is still NULL. It doesn't make sense to open an invalid image, so remove the exception for BDRV_O_NO_IO. This catches the problem early and any code that makes the same assumption is safe now. At the same time, in the name of defensive programming, we shouldn't make the assumption in the first place. Let qcow2_refresh_limits() check s->crypto rather than bs->encrypted. If s->crypto is NULL, it also can't make any requirement on request alignment. Finally, start a qcow2-encryption test case that only serves as a regression test for this crash for now. Reported-by: Leonid Reviakin Reported-by: Denis Rastyogin Signed-off-by: Kevin Wolf Message-ID: <20250318201143.70657-1-kwolf@redhat.com> Reviewed-by: Daniel P. Berrangé Signed-off-by: Kevin Wolf --- 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 dd6bcafbd8..7774e7f090 100644 --- a/block/qcow2.c +++ b/block/qcow2.c @@ -1721,7 +1721,7 @@ 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; @@ -1976,7 +1976,7 @@ 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 new file mode 100755 index 0000000000..95f6195ab8 --- /dev/null +++ b/tests/qemu-iotests/tests/qcow2-encryption @@ -0,0 +1,75 @@ +#!/usr/bin/env bash +# group: rw quick +# +# Test case for encryption support in qcow2 +# +# Copyright (C) 2025 Red Hat, Inc. +# +# This program is free software; you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +# creator +owner=kwolf@redhat.com + +seq="$(basename $0)" +echo "QA output created by $seq" + +status=1 # failure is the default! + +_cleanup() +{ + _cleanup_test_img +} +trap "_cleanup; exit \$status" 0 1 2 3 15 + +# get standard environment, filters and checks +. ../common.rc +. ../common.filter + +# This tests qcow2-specific low-level functionality +_supported_fmt qcow2 +_supported_proto file +_require_working_luks + +IMG_SIZE=64M + +echo +echo "=== Create an encrypted image ===" +echo + +_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 +echo "=== Remove the header extension ===" +echo + +$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 + +# success, all done +echo "*** done" +rm -f $seq.full +status=0 diff --git a/tests/qemu-iotests/tests/qcow2-encryption.out b/tests/qemu-iotests/tests/qcow2-encryption.out new file mode 100644 index 0000000000..9b549dc2ab --- /dev/null +++ b/tests/qemu-iotests/tests/qcow2-encryption.out @@ -0,0 +1,32 @@ +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 + +Header extension: +magic 0x6803f857 (Feature table) +length 384 +data + +image: TEST_DIR/t.IMGFMT +file format: IMGFMT +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 + +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 +*** done From patchwork Tue Apr 8 13:00:47 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 14043003 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 3A06AC3600C for ; Tue, 8 Apr 2025 13:03:07 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1u28at-0007Ag-Oc; Tue, 08 Apr 2025 09:02:23 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1u28Ze-0006ly-Qg for qemu-devel@nongnu.org; Tue, 08 Apr 2025 09:01:10 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.133.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1u28Zc-0005cU-5x for qemu-devel@nongnu.org; Tue, 08 Apr 2025 09:01:06 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1744117263; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=h7O1+jeMjpt4PGSabZHa12EThqfaHw3Rj3mPvJFT2p0=; b=KHUr2YCD3RJMIHMEOCpLeRatiMaBL1nBEh+0Z1+/jI9CHZXtxJfQd4mzJzKrLLpD2ckMfY IkTHlOMCu+Ylqho+6WnF2QCjRedjkY5uR5XgRadj0EiYRJBdF5rIY/ETTaduYUd2lsMpYq nBHRrOzP9eoRRjKldzz+4A4SWNautKk= Received: from mx-prod-mc-06.mail-002.prod.us-west-2.aws.redhat.com (ec2-35-165-154-97.us-west-2.compute.amazonaws.com [35.165.154.97]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-116-89pWzGi1O6udpRPD99MjZA-1; Tue, 08 Apr 2025 09:01:00 -0400 X-MC-Unique: 89pWzGi1O6udpRPD99MjZA-1 X-Mimecast-MFC-AGG-ID: 89pWzGi1O6udpRPD99MjZA_1744117259 Received: from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.40]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-06.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 1B1FA180AF65; Tue, 8 Apr 2025 13:00:59 +0000 (UTC) Received: from merkur.redhat.com (unknown [10.44.33.56]) by mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id E87C71955BEF; Tue, 8 Apr 2025 13:00:57 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Cc: kwolf@redhat.com, qemu-devel@nongnu.org Subject: [PULL 3/4] scsi-disk: Apply error policy for host_status errors again Date: Tue, 8 Apr 2025 15:00:47 +0200 Message-ID: <20250408130048.283364-4-kwolf@redhat.com> In-Reply-To: <20250408130048.283364-1-kwolf@redhat.com> References: <20250408130048.283364-1-kwolf@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.40 Received-SPF: pass client-ip=170.10.133.124; envelope-from=kwolf@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -28 X-Spam_score: -2.9 X-Spam_bar: -- X-Spam_report: (-2.9 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.845, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H5=0.001, RCVD_IN_MSPIKE_WL=0.001, RCVD_IN_VALIDITY_CERTIFIED_BLOCKED=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org 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 Message-ID: <20250407155949.44736-1-kwolf@redhat.com> Reviewed-by: Stefan Hajnoczi Reviewed-by: Hanna Czenczek Signed-off-by: Kevin Wolf --- 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 index 8da1d5a77c..e59632e9b1 100644 --- a/hw/scsi/scsi-disk.c +++ b/hw/scsi/scsi-disk.c @@ -68,10 +68,9 @@ 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; @@ -225,11 +224,26 @@ 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; @@ -289,6 +303,10 @@ 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) { @@ -409,7 +427,6 @@ 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; @@ -448,7 +465,6 @@ 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; @@ -585,7 +601,6 @@ 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; @@ -2846,14 +2861,10 @@ 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; From patchwork Tue Apr 8 13:00:48 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kevin Wolf X-Patchwork-Id: 14043004 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id BDE82C3600C for ; Tue, 8 Apr 2025 13:03:46 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1u28as-00078P-VJ; Tue, 08 Apr 2025 09:02:23 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1u28Zk-0006mL-WC for qemu-devel@nongnu.org; Tue, 08 Apr 2025 09:01:20 -0400 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1u28Zi-0005dE-Qv for qemu-devel@nongnu.org; Tue, 08 Apr 2025 09:01:12 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1744117268; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ahykKS5380HRIk59tI1Zc8zyJMxoC+VGjiLPBK0Cc6c=; b=Nq+1YHhLsX1OuCl5s9L/qMTbgEyROuvConQli7MDITLBF6KyKdhH03PK0p+obkufUJQAAQ lQBgl7QHaePQ1PXYUKmZPW/nLAKHwksaz0AfagH6+oOJ/XRZoLaKSlug34jOfY+5OrvOcN aeRAfdb+mDTzP6GtuEhZRe6G+JlPh54= Received: from mx-prod-mc-06.mail-002.prod.us-west-2.aws.redhat.com (ec2-35-165-154-97.us-west-2.compute.amazonaws.com [35.165.154.97]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-687-umJ8wUPONJukArKaEsORBQ-1; Tue, 08 Apr 2025 09:01:02 -0400 X-MC-Unique: umJ8wUPONJukArKaEsORBQ-1 X-Mimecast-MFC-AGG-ID: umJ8wUPONJukArKaEsORBQ_1744117260 Received: from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.40]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-06.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id A6AE418011ED; Tue, 8 Apr 2025 13:01:00 +0000 (UTC) Received: from merkur.redhat.com (unknown [10.44.33.56]) by mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTP id 83B5019560AD; Tue, 8 Apr 2025 13:00:59 +0000 (UTC) From: Kevin Wolf To: qemu-block@nongnu.org Cc: kwolf@redhat.com, qemu-devel@nongnu.org Subject: [PULL 4/4] test-bdrv-drain: Fix data races Date: Tue, 8 Apr 2025 15:00:48 +0200 Message-ID: <20250408130048.283364-5-kwolf@redhat.com> In-Reply-To: <20250408130048.283364-1-kwolf@redhat.com> References: <20250408130048.283364-1-kwolf@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.40 Received-SPF: pass client-ip=170.10.129.124; envelope-from=kwolf@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -28 X-Spam_score: -2.9 X-Spam_bar: -- X-Spam_report: (-2.9 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.845, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_VALIDITY_CERTIFIED_BLOCKED=0.001, RCVD_IN_VALIDITY_RPBL_BLOCKED=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org From: Vitalii Mordan This patch addresses potential data races involving access to Job fields in the test-bdrv-drain test. Fixes: 7253220de4 ("test-bdrv-drain: Test drain vs. block jobs") Resolves: https://gitlab.com/qemu-project/qemu/-/issues/2900 Signed-off-by: Vitalii Mordan Message-ID: <20250402102119.3345626-1-mordan@ispras.ru> [kwolf: Fixed up coding style and one missing atomic access] Reviewed-by: Kevin Wolf Signed-off-by: Kevin Wolf --- include/qemu/job.h | 3 +++ job.c | 6 ++++++ tests/unit/test-bdrv-drain.c | 32 +++++++++++++++++++------------- 3 files changed, 28 insertions(+), 13 deletions(-) diff --git a/include/qemu/job.h b/include/qemu/job.h index 2b873f2576..a5a04155ea 100644 --- a/include/qemu/job.h +++ b/include/qemu/job.h @@ -545,6 +545,9 @@ bool job_is_ready(Job *job); /* Same as job_is_ready(), but called with job lock held. */ bool job_is_ready_locked(Job *job); +/** Returns whether the job is paused. Called with job_mutex *not* held. */ +bool job_is_paused(Job *job); + /** * Request @job to pause at the next pause point. Must be paired with * job_resume(). If the job is supposed to be resumed by user action, call diff --git a/job.c b/job.c index 660ce22c56..0653bc2ba6 100644 --- a/job.c +++ b/job.c @@ -251,6 +251,12 @@ bool job_is_cancelled_locked(Job *job) return job->force_cancel; } +bool job_is_paused(Job *job) +{ + JOB_LOCK_GUARD(); + return job->paused; +} + bool job_is_cancelled(Job *job) { JOB_LOCK_GUARD(); diff --git a/tests/unit/test-bdrv-drain.c b/tests/unit/test-bdrv-drain.c index 7410e6f352..290cd2a70e 100644 --- a/tests/unit/test-bdrv-drain.c +++ b/tests/unit/test-bdrv-drain.c @@ -632,6 +632,8 @@ typedef struct TestBlockJob { BlockDriverState *bs; int run_ret; int prepare_ret; + + /* Accessed with atomics */ bool running; bool should_complete; } TestBlockJob; @@ -667,10 +669,10 @@ static int coroutine_fn test_job_run(Job *job, Error **errp) /* We are running the actual job code past the pause point in * job_co_entry(). */ - s->running = true; + qatomic_set(&s->running, true); job_transition_to_ready(&s->common.job); - while (!s->should_complete) { + while (!qatomic_read(&s->should_complete)) { /* Avoid job_sleep_ns() because it marks the job as !busy. We want to * emulate some actual activity (probably some I/O) here so that drain * has to wait for this activity to stop. */ @@ -685,7 +687,7 @@ static int coroutine_fn test_job_run(Job *job, Error **errp) static void test_job_complete(Job *job, Error **errp) { TestBlockJob *s = container_of(job, TestBlockJob, common.job); - s->should_complete = true; + qatomic_set(&s->should_complete, true); } BlockJobDriver test_job_driver = { @@ -791,7 +793,7 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type, /* job_co_entry() is run in the I/O thread, wait for the actual job * code to start (we don't want to catch the job in the pause point in * job_co_entry(). */ - while (!tjob->running) { + while (!qatomic_read(&tjob->running)) { aio_poll(qemu_get_aio_context(), false); } } @@ -799,7 +801,7 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type, WITH_JOB_LOCK_GUARD() { g_assert_cmpint(job->job.pause_count, ==, 0); g_assert_false(job->job.paused); - g_assert_true(tjob->running); + g_assert_true(qatomic_read(&tjob->running)); g_assert_true(job->job.busy); /* We're in qemu_co_sleep_ns() */ } @@ -825,7 +827,7 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type, * * paused is reset in the I/O thread, wait for it */ - while (job->job.paused) { + while (job_is_paused(&job->job)) { aio_poll(qemu_get_aio_context(), false); } } @@ -858,7 +860,7 @@ static void test_blockjob_common_drain_node(enum drain_type drain_type, * * paused is reset in the I/O thread, wait for it */ - while (job->job.paused) { + while (job_is_paused(&job->job)) { aio_poll(qemu_get_aio_context(), false); } } @@ -1411,10 +1413,12 @@ static void test_set_aio_context(void) typedef struct TestDropBackingBlockJob { BlockJob common; - bool should_complete; bool *did_complete; BlockDriverState *detach_also; BlockDriverState *bs; + + /* Accessed with atomics */ + bool should_complete; } TestDropBackingBlockJob; static int coroutine_fn test_drop_backing_job_run(Job *job, Error **errp) @@ -1422,7 +1426,7 @@ static int coroutine_fn test_drop_backing_job_run(Job *job, Error **errp) TestDropBackingBlockJob *s = container_of(job, TestDropBackingBlockJob, common.job); - while (!s->should_complete) { + while (!qatomic_read(&s->should_complete)) { job_sleep_ns(job, 0); } @@ -1541,7 +1545,7 @@ static void test_blockjob_commit_by_drained_end(void) job_start(&job->common.job); - job->should_complete = true; + qatomic_set(&job->should_complete, true); bdrv_drained_begin(bs_child); g_assert(!job_has_completed); bdrv_drained_end(bs_child); @@ -1557,15 +1561,17 @@ static void test_blockjob_commit_by_drained_end(void) typedef struct TestSimpleBlockJob { BlockJob common; - bool should_complete; bool *did_complete; + + /* Accessed with atomics */ + bool should_complete; } TestSimpleBlockJob; static int coroutine_fn test_simple_job_run(Job *job, Error **errp) { TestSimpleBlockJob *s = container_of(job, TestSimpleBlockJob, common.job); - while (!s->should_complete) { + while (!qatomic_read(&s->should_complete)) { job_sleep_ns(job, 0); } @@ -1700,7 +1706,7 @@ static void test_drop_intermediate_poll(void) job->did_complete = &job_has_completed; job_start(&job->common.job); - job->should_complete = true; + qatomic_set(&job->should_complete, true); g_assert(!job_has_completed); ret = bdrv_drop_intermediate(chain[1], chain[0], NULL, false);