From nobody Tue Aug 25 00:12:08 2026 Delivered-To: importer@patchew.org Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org ARC-Seal: i=1; a=rsa-sha256; t=1779486982; cv=none; d=zohomail.com; s=zohoarc; b=hBbjjNXsXdZeG0UbmV5YfLrrrI7Ge7QwgfCGYgSz5WDue20pUmssF80qhzhNjegYx8c8eyxzEdH2JO1fZgtzZ8KWvCFrKE87GqMfy8BDmsnwTFfE/cKJSn/BTEzJY2x3Oq0xATuHMpIO/4l+luXynD8dSXp4LKJFwOzKw6k5e+0= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1779486982; h=Content-Transfer-Encoding:Cc:Cc:Date:Date:From:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:Subject:To:To:Message-Id:Reply-To; bh=XG/QBUMXO+x5S9OgxaerXGYMSJsNzM/EDXUGasQPzHg=; b=E9GsPawXxm0ovw/Jj+q92sCmyWogj9i9X36meWmcLFALvHxrii+2qM8AItWyKvDMGm3NudR13WfP8hW6iSftEr3G6aVepaBYzx2+p8XahsM3PwcIVeRUbsLsY73vTM1XsVTc0Igc0U2bB7tXjc5hEcQBE9D0x3iDjxtgk9PzFts= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=qemu-devel-bounces+importer=patchew.org@nongnu.org Return-Path: Received: from lists1p.gnu.org (lists1p.gnu.org [209.51.188.17]) by mx.zohomail.com with SMTPS id 1779486982685546.3836578204972; Fri, 22 May 2026 14:56:22 -0700 (PDT) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists1p.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1wQXoz-0002Aj-5P; Fri, 22 May 2026 17:54:21 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists1p.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1wQXoY-0000j8-Hd; Fri, 22 May 2026 17:53:56 -0400 Received: from isrv.corpit.ru ([212.248.84.144]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1wQXoV-0005M4-OC; Fri, 22 May 2026 17:53:53 -0400 Received: from tsrv.corpit.ru (tsrv.tls.msk.ru [192.168.177.2]) by isrv.corpit.ru (Postfix) with ESMTP id 18CEA1AFB89; Sat, 23 May 2026 00:49:22 +0300 (MSK) Received: from think4mjt.tls.msk.ru (mjtthink.wg.tls.msk.ru [192.168.177.146]) by tsrv.corpit.ru (Postfix) with ESMTP id 401693B71D8; Sat, 23 May 2026 00:49:26 +0300 (MSK) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tls.msk.ru; s=202602; t=1779486562; bh=8146qO6KFVKscfnp7ME4DCWZ2VVfJ/m5ZGh/Be/5xEg=; h=From:To:Cc:Subject:Date:In-Reply-To:References; b=GTj+q/ZuNY7NdjLe5S2E017W3+D5Zyfzkotdz5k7T968JBprs8DqrE7+kysMzs5V8 FRF93Sf+0uUKFvZ5NSd83P2GJW+Q5iA9/pF/DOOtPbV2hE1baPPZ+YVtYK+kTmNMeR YXdVXymzaRr5dD6OaK/FupMC995++CymxjnJ/k9j9OhCE3yHkPp7tVOZqIBG1BMDkC VM3+EP/onSEP+2R7xtcytJkqKTu8+bvRw64l7ow8RkSB1+cAmx6sM79MJtwLwd1+Sl rfaOXJxqFt8EfRLYDZbiPotievt1zRBvEq38jPcYeilc2XVGTgNElBbujOO/qQq2iS 0IHOFB0iZoX+Q== From: Michael Tokarev To: qemu-devel@nongnu.org Cc: qemu-stable@nongnu.org, Kevin Wolf , "Denis V. Lunev" , Michael Tokarev Subject: [Stable-10.2.3 147/149] qcow2: Fix corruption on discard during write with COW Date: Sat, 23 May 2026 00:49:19 +0300 Message-ID: <20260522214923.807017-48-mjt@tls.msk.ru> X-Mailer: git-send-email 2.47.3 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Received-SPF: pass (zohomail.com: domain of gnu.org designates 209.51.188.17 as permitted sender) client-ip=209.51.188.17; envelope-from=qemu-devel-bounces+importer=patchew.org@nongnu.org; helo=lists1p.gnu.org; Received-SPF: pass client-ip=212.248.84.144; envelope-from=mjt@tls.msk.ru; helo=isrv.corpit.ru X-Spam_score_int: -19 X-Spam_score: -2.0 X-Spam_bar: -- X-Spam_report: (-2.0 / 5.0 requ) BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, 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: qemu development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+importer=patchew.org@nongnu.org Sender: qemu-devel-bounces+importer=patchew.org@nongnu.org X-ZohoMail-DKIM: pass (identity @tls.msk.ru) X-ZM-MESSAGEID: 1779486984859158500 Content-Type: text/plain; charset="utf-8" From: Kevin Wolf Most code in qcow2 that accesses (and potentially modifies) L2 tables does so while holding s->lock. There is one exception, which is allocating writes. They hold the lock initially while allocating clusters, but drop it for writing the guest payload before taking the lock again for updating the L2 tables. This allows concurrent requests that touch other parts of the image file to continue in parallel and is an important performance optimisation. However, this means that other requests that run while the lock is dropped for writing guest data must synchronise with the list of allocating requests in s->cluster_allocs and wait if they would overlap. For writes, this is done in handle_dependencies(), but discard and write zeros operations neglect to synchronise with s->cluster_allocs. This means that discard can free a cluster whose L2 entry will already be modified in qcow2_alloc_cluster_link_l2() by a previously started write. In the case of a pre-allocated zero cluster that is in the process of being overwritten, this means that discard can lead to a situation where the cluster is still mapped (because the write will restore the L2 entry just without the zero flag), but its refcount has been decreased, resulting in a corrupted image. Add the missing synchronisation to qcow2_cluster_discard() and qcow2_subcluster_zeroize() to fix the problem. Cc: qemu-stable@nongnu.org Reported-by: Denis V. Lunev Signed-off-by: Kevin Wolf Message-ID: <20260427170520.101242-4-kwolf@redhat.com> Reviewed-by: Denis V. Lunev Tested-by: Denis V. Lunev Signed-off-by: Kevin Wolf (cherry picked from commit b8bfb1478d61512f851badd0d912c6661a2efee7) Signed-off-by: Michael Tokarev diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c index c655bf6df4..8b1e80bd0b 100644 --- a/block/qcow2-cluster.c +++ b/block/qcow2-cluster.c @@ -1392,6 +1392,9 @@ count_single_write_clusters(BlockDriverState *bs, int= nb_clusters, * the same cluster. In this case we need to wait until the previous * request has completed and updated the L2 table accordingly. * + * If allow_shortening =3D=3D true, instead of waiting for a dependency, *= cur_bytes + * can be shortened so that the cluster allocations don't overlap. + * * Returns: * 0 if there was no dependency. *cur_bytes indicates the number of * bytes from guest_offset that can be read before the next @@ -1403,7 +1406,9 @@ count_single_write_clusters(BlockDriverState *bs, int= nb_clusters, */ static int coroutine_fn handle_dependencies(BlockDriverState *bs, uint64_t guest_offset, - uint64_t *cur_bytes, QCowL2Met= a **m) + uint64_t *cur_bytes, + bool allow_shortening, + QCowL2Meta **m) { BDRVQcow2State *s =3D bs->opaque; QCowL2Meta *old_alloc; @@ -1434,7 +1439,7 @@ static int coroutine_fn handle_dependencies(BlockDriv= erState *bs, =20 /* Conflict */ =20 - if (start < old_start) { + if (start < old_start && allow_shortening) { /* Stop at the start of a running allocation */ bytes =3D old_start - start; } else { @@ -1469,6 +1474,29 @@ static int coroutine_fn handle_dependencies(BlockDri= verState *bs, return 0; } =20 +static void coroutine_mixed_fn wait_for_dependencies(BlockDriverState *bs, + uint64_t guest_offset, + uint64_t bytes) +{ + BDRVQcow2State *s =3D bs->opaque; + QCowL2Meta *m =3D NULL; + int ret; + + /* + * Discard has some non-coroutine callers (creating internal snapshots= and + * make empty). They are calling from qemu-img or in a drained section= , so + * we know that no writes can be in progress. + */ + if (!qemu_in_coroutine()) { + assert(QLIST_EMPTY(&s->cluster_allocs)); + return; + } + + do { + ret =3D handle_dependencies(bs, guest_offset, &bytes, false, &m); + } while (ret =3D=3D -EAGAIN); +} + /* * Checks how many already allocated clusters that don't require a new * allocation there are at the given guest_offset (up to *bytes). @@ -1840,7 +1868,7 @@ again: * the right synchronisation between the in-flight request= and * the new one. */ - ret =3D handle_dependencies(bs, start, &cur_bytes, m); + ret =3D handle_dependencies(bs, start, &cur_bytes, true, m); if (ret =3D=3D -EAGAIN) { /* Currently handle_dependencies() doesn't yield if we already= had * an allocation. If it did, we would have to clean up the L2M= eta @@ -2000,6 +2028,15 @@ int qcow2_cluster_discard(BlockDriverState *bs, uint= 64_t offset, int64_t cleared; int ret; =20 + /* + * If we're touching a cluster for which allocating writes are in flig= ht, + * wait for them to complete to avoid conflicting metadata updates. + * + * We don't need to allocate a QCowL2Meta for the discard operation be= cause + * s->lock is held for the duration of the whole operation. + */ + wait_for_dependencies(bs, offset, bytes); + /* Caller must pass aligned values, except at image end */ assert(QEMU_IS_ALIGNED(offset, s->cluster_size)); assert(QEMU_IS_ALIGNED(end_offset, s->cluster_size) || @@ -2160,6 +2197,15 @@ int coroutine_fn qcow2_subcluster_zeroize(BlockDrive= rState *bs, uint64_t offset, int64_t cleared; int ret; =20 + /* + * If we're touching a cluster for which allocating writes are in flig= ht, + * wait for them to complete to avoid conflicting metadata updates. + * + * We don't need to allocate a QCowL2Meta for the zeroize operation be= cause + * s->lock is held for the duration of the whole operation. + */ + wait_for_dependencies(bs, offset, bytes); + /* If we have to stay in sync with an external data file, zero out * s->data_file first. */ if (data_file_is_raw(bs)) { --=20 2.47.3